NealSun96 commented on a change in pull request #1413:
URL: https://github.com/apache/helix/pull/1413#discussion_r500471814



##########
File path: 
helix-core/src/main/java/org/apache/helix/controller/dataproviders/BaseControllerDataProvider.java
##########
@@ -299,6 +306,33 @@ private void updateMaintenanceInfo(final HelixDataAccessor 
accessor) {
     // The following flag is to guarantee that there's only one update per 
pineline run because we
     // check for whether maintenance recovery could happen twice every pipeline
     _hasMaintenanceSignalChanged = false;
+
+    // If maintenance mode has exited, clear cached timed-out nodes
+    if (!_isMaintenanceModeEnabled) {
+      _timedOutInstanceDuringMaintenance.clear();
+    }
+  }
+
+  private void timeoutNodesDuringMaintenance(final HelixDataAccessor accessor) 
{
+    // If maintenance mode is enabled and timeout window is specified, filter 
'new' live nodes
+    // for timed-out nodes
+    long timeOutWindow = -1;
+    if (_clusterConfig != null) {
+      timeOutWindow = _clusterConfig.getOfflineNodeTimeOutForMaintenanceMode();
+    }
+    if (timeOutWindow >= 0 && isMaintenanceModeEnabled()) {
+      for (String instance : _liveInstanceCache.getPropertyMap().keySet()) {
+        // 1. Check timed-out cache and don't do repeated work;
+        // 2. Check for nodes that didn't exist in the last iteration, because 
it has been checked;
+        // 3. For all other nodes, check if it's timed-out.
+        // When maintenance mode is first entered, all nodes will be checked 
as a result.
+        if (!_timedOutInstanceDuringMaintenance.contains(instance)
+            && !_liveInstanceSnapshotForMaintenance.containsKey(instance)
+            && isInstanceTimedOutDuringMaintenance(accessor, instance, 
timeOutWindow)) {
+          _timedOutInstanceDuringMaintenance.add(instance);

Review comment:
       No, `_liveInstanceSnapshotForMaintenance` is equal to 
`getLiveInstances()` from the **last** pipeline. If a long-offline instance 
coms back after the first pipeline and before the next pipeline, it will 
**not** be in the snapshot obtained during the next pipeline, and will 
therefore be checked by the timeout logic. Whether this instance fails the 
check, it will be stored either in the snapshot or in the "timed-out cache", 
and it will not be checked by the pipeline after. 




----------------------------------------------------------------
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]

Reply via email to