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