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



##########
File path: 
helix-core/src/main/java/org/apache/helix/controller/dataproviders/BaseControllerDataProvider.java
##########
@@ -428,13 +463,32 @@ public synchronized void setIdealStates(List<IdealState> 
idealStates) {
   }
 
   /**
-   * Returns the LiveInstances for each of the instances that are curretnly up 
and running
+   * Returns the LiveInstances for each of the instances that are currently up 
and running
    * @return
    */
   public Map<String, LiveInstance> getLiveInstances() {

Review comment:
       In short: everywhere else that isn't critical to "timing out" instances. 
   
   The new `getLiveInstances(boolean)` is minimized to only one place: 
`MessageGenerationPhase`. This approach minimizes any impact to existing code 
while also making the "timed-out" instances not reachable from the controller; 
effectively, those instances will have no operations done on them. This 
decision is made after @jiajunwang 's comment that we shouldn't directly alter 
the live instance cache; we should only alter the getter. 
   
   Please let me know if this change makes sense. I believe only changing 
`MessageGenerationPhase` is sufficient. 




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