vjagadish1989 commented on a change in pull request #952: Improved 
standby-aware container allocation for active-containers on job redeploys
URL: https://github.com/apache/samza/pull/952#discussion_r268927712
 
 

 ##########
 File path: 
samza-core/src/main/java/org/apache/samza/clustermanager/StandbyContainerManager.java
 ##########
 @@ -224,17 +257,40 @@ private void initiateActiveContainerFailover(String 
containerID, String resource
         SamzaResource standbyContainerResource = 
samzaApplicationState.runningContainers.get(standbyContainerID);
 
         // use this standby if there was no previous failover for which this 
standbyResource was used
-        if (!(failoverMetadata.isPresent() && 
failoverMetadata.get().isStandbyResourceUsed(standbyContainerResource.getResourceID())))
 {
+        if (!(failoverMetadata.isPresent() && failoverMetadata.get()
+            .isStandbyResourceUsed(standbyContainerResource.getResourceID()))) 
{
 
-          log.info("Returning standby container {} in running state for active 
container {}", standbyContainerID,
-              activeContainerID);
-          return Optional.of(new Entry<>(standbyContainerID, 
standbyContainerResource));
+          log.info("Returning standby container {} in running state on host {} 
for active container {}",
+              standbyContainerID, standbyContainerResource.getHost(), 
activeContainerID);
+          return standbyContainerResource.getHost();
         }
       }
     }
-
     log.info("Did not find any running standby container for active container 
{}", activeContainerID);
-    return Optional.empty();
+
+    // We iterate over the list of last-known standbyHosts to check if anyone 
of them has not already been tried
+    for (String standbyContainerID : 
this.standbyContainerConstraints.get(activeContainerID)) {
+
+      String standbyHost = 
this.samzaApplicationState.jobModelManager.jobModel().
+          getContainerToHostValue(standbyContainerID, 
SetContainerHostMapping.HOST_KEY);
+
+      if (standbyHost == null) {
+        log.info("No last known standbyHost for container {}", 
standbyContainerID);
+
+      } else if (standbyHost != null && failoverMetadata.isPresent() && 
failoverMetadata.get()
 
 Review comment:
   by definition, the condition `standbyHost != null` is true at L:280. Can we 
remove it

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


With regards,
Apache Git Services

Reply via email to