jiajunwang commented on a change in pull request #1149:
URL: https://github.com/apache/helix/pull/1149#discussion_r455977980
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java
##########
@@ -866,6 +858,83 @@ public void disconnect() {
}
}
+ /**
+ * The callback handler cleanup operations that require an active ZkClient
connection.
+ * If ZkClient is not connected, Helix Manager shall skip the cleanup.
+ *
+ * @return true if the cleanup has been done successfully.
+ */
+ private boolean cleanupCallbackHandlers() {
+ AtomicBoolean cleanupDone = new AtomicBoolean(false);
+
+ if (_zkclient.waitUntilConnected(_waitForConnectedTimeout,
TimeUnit.MILLISECONDS)) {
+ // Create a separate thread for executing cleanup task to avoid forever
retry.
+ Thread cleanupThread = new Thread(String
+ .format("Cleanup thread for %s-%s-%s", _clusterName, _instanceName,
_instanceType)) {
+ @Override
+ public void run() {
+ // TODO reset user defined handlers only
+ resetHandlers(true);
+
+ if (_leaderElectionHandler != null) {
+ _leaderElectionHandler.reset(true);
+ }
+
+ ParticipantManager participantManager = _participantManager;
+ if (participantManager != null) {
+ participantManager.disconnect();
+ }
+
+ cleanupDone.set(true);
+ }
+ };
+
+ // Define the state listener to terminate the cleanup thread when the
ZkConnection breaks.
+ IZkStateListener stateListener = new IZkStateListener() {
Review comment:
`just have an exist call for the cluster since it is in HelixManager?`
This is already done.
However, this won't prevent if the connection breaks during the cleanup
process.
For example, we have 10 callback handlers to reset, the connection breaks at
the 5th one. Then the check before this cleanup work won't work. To achieve the
goal, an alternative way is keeping checking the connection state in a loop.
But that is a bad design compare of using the listener.
##########
File path:
helix-core/src/main/java/org/apache/helix/manager/zk/ZKHelixManager.java
##########
@@ -866,6 +858,83 @@ public void disconnect() {
}
}
+ /**
+ * The callback handler cleanup operations that require an active ZkClient
connection.
+ * If ZkClient is not connected, Helix Manager shall skip the cleanup.
+ *
+ * @return true if the cleanup has been done successfully.
+ */
+ private boolean cleanupCallbackHandlers() {
+ AtomicBoolean cleanupDone = new AtomicBoolean(false);
+
+ if (_zkclient.waitUntilConnected(_waitForConnectedTimeout,
TimeUnit.MILLISECONDS)) {
+ // Create a separate thread for executing cleanup task to avoid forever
retry.
+ Thread cleanupThread = new Thread(String
+ .format("Cleanup thread for %s-%s-%s", _clusterName, _instanceName,
_instanceType)) {
+ @Override
+ public void run() {
+ // TODO reset user defined handlers only
+ resetHandlers(true);
+
+ if (_leaderElectionHandler != null) {
+ _leaderElectionHandler.reset(true);
+ }
+
+ ParticipantManager participantManager = _participantManager;
+ if (participantManager != null) {
+ participantManager.disconnect();
+ }
+
+ cleanupDone.set(true);
+ }
+ };
+
+ // Define the state listener to terminate the cleanup thread when the
ZkConnection breaks.
+ IZkStateListener stateListener = new IZkStateListener() {
Review comment:
`just have an exist call for the cluster since it is in HelixManager?`
This is already done.
However, this won't prevent if the connection breaks during the cleanup
process.
For example, we have 10 callback handlers to reset, the connection breaks at
the 5th one. Then the check before this cleanup work won't work. To achieve the
goal, an alternative way is keeping checking the connection state in a loop.
But that is a bad design compare to using the listener.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
