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. Let me know if this makes sense to you.
----------------------------------------------------------------
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]
