SOLR-18298: ZkController.OnReconnect Is Trigger Unnecessarily

[linxiaokun528]

Description

Curator's RECONNECTED event is different from the previous RECONNECTED event. Before Solr10, the OnReconnect is only triggered after a reconnection from a session expiration. Check the following

solr/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ConnectionManager.java

Line 165 in fdb5314

"Our previous ZooKeeper session was expired. Attempting to reconnect to recover relationship with ZooKeeper...");

solr/solr/solrj-zookeeper/src/java/org/apache/solr/common/cloud/ConnectionManager.java

Line 199 in fdb5314

onReconnect.command();

But Curator's RECONNECTED event is triggered every time a Solr node is disconnected from a ZooKeeper instance and reconnected to another ZooKeeper instance.

Therefore, currently ZkController.onReconnect is invoked every time a Solr node reconnects to a ZooKeeper instance without a session expiration, which is a huge overhead, especially when we need to rolling-restart a ZooKeeper Cluster. It can take more than 10 minutes for a small Solr cluster to level out.

Similiarly, now ZkController.onDisconnect is triggered just after a disconnection from a Zookeeper instance. It should only be triggered after a session expiration.

Solution

I created a SolrCuratorEvent class as a Bridge. Now it only has two event types: EXPIRED_RECONNECTION and SESSION_EXPIRATION.
EXPIRED_RECONNECTION: Will only be triggered when Solr reconnects Zookeeper from a session expiration
SESSION_EXPIRATION: Will be triggered when a session expiration happens (The same to Curator's LOST event.)

We should use SolrCuratorEvent instead of Curator's event and we can add more types into it whenever needed.

I also renamed some methods to make them clearer:
ZkController.onReconnect -> ZkController.onExpiredReconnection
ZkController.onDisconnect -> ZkController.onSessionExpiration

Tests

In ZkControllerTest.java,

1. I added a method testZkDisconnectionEvents to make sure when there is a session expiration, ZkController.onSessionExpiration is triggered.
2. I added a method testZkReconnectionEvents to make sure when Solr reconnects to ZooKeeper from a session expiration, ZkController.onExpiredReconnection is triggered.

Checklist

Please review the following and check all that apply:

• I have reviewed the guidelines for How to Contribute and my code conforms to the standards described there to the best of my ability.
• I have created a Jira issue and added the issue ID to my pull request title.
• I have given Solr maintainers access to contribute to my PR branch. (optional but recommended, not available for branches on forks living under an organisation)
• I have developed this patch against the main branch.
• I have run ./gradlew check.
• I have added tests for my changes.
• I have added documentation for the Reference Guide (This change restores the original behavior (a performance fix), and does not introduce any new user-facing features or configuration changes. Therefore, I believe no Reference Guide update is required.)
• I have added a changelog entry for my change

[ercsonusharma]

Thanks for tracking this down. It's fine to keep the current direction if you want the clean removal, but flagging that the same fix is achievable with much less surface change IMO.

SolrCuratorEvent is more machinery than needed. It's an enum + abstract of() + a new EventAction type that's really just Runnable renamed. The same behavior can live directly on the existing OnReconnect/OnDisconnect callbacks and no new type.
The AtomicBoolean that makes it correct just lives in the returned closure.

FWIW, The public OnReconnect/OnDisconnect interfaces published in solrj-zookeeper, so removing them is a breaking change. The bug is purely in the detection logic which is fixable while keeping the interfaces intact.