PawasChhokra commented on a change in pull request #1446:
URL: https://github.com/apache/samza/pull/1446#discussion_r547138145



##########
File path: 
samza-core/src/main/java/org/apache/samza/clustermanager/StandbyContainerManager.java
##########
@@ -168,11 +173,13 @@ public void handleContainerStopFail(String containerID, 
String resourceID,
    * @param host The hostname of the active container
    * @return the set of racks on which this active container's standby can be 
scheduled
    */
-  public Set<FaultDomain> 
getAllowedFaultDomainsForSchedulingStandbyContainer(String host) {
-    Set<FaultDomain> activeContainerRack = 
faultDomainManager.getFaultDomainOfHost(host);
-    Set<FaultDomain> standbyRacks = faultDomainManager.getAllFaultDomains();
-    standbyRacks.removeAll(activeContainerRack);
-    return standbyRacks;
+  public Set<FaultDomain> 
getAllowedFaultDomainsForSchedulingStandbyContainer(Optional<String> host) {
+    Set<FaultDomain> standbyFaultDomain = 
faultDomainManager.getAllFaultDomains();
+    if (host.isPresent()) {
+      Set<FaultDomain> activeContainerFaultDomain = 
faultDomainManager.getFaultDomainsForHost(host.get());
+      standbyFaultDomain.removeAll(activeContainerFaultDomain);
+    }
+    return standbyFaultDomain;

Review comment:
       I've extracted out the simplified part of the method in a separate 
method (`getAllowedFaultDomainsForStandbyContainerGivenActiveContainerHost`), 
and am doing extra overloaded functionalities in the parent method 
(`getAllowedFaultDomainsForStandbyContainerGivenHostToExclude`). This way, the 
caller can decide which method to invoke. Also, I've removed Optional from the 
argument. 




----------------------------------------------------------------
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:
us...@infra.apache.org


Reply via email to