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_r267633751
##########
File path:
samza-core/src/main/java/org/apache/samza/clustermanager/StandbyContainerManager.java
##########
@@ -161,54 +161,72 @@ private void handleStandbyContainerStop(String
standbyContainerID, String resour
}
}
- /** Method to handle failover for an active container.
- * We try to find a standby for the active container, and issue a stop on
it.
- * If we do not find a standby container, we simply issue an anyhost
request to place it.
+ /** Method to handle standby-aware allocation for an active container.
+ * We try to find a standby host for the active container, and issue a stop
on any standby-containers running on it,
+ * request resource to place the active on the standby's host, and one to
place the standby elsewhere.
*
- * @param containerID the samzaContainerID of the active-container
+ * @param activeContainerID the samzaContainerID of the active-container
* @param resourceID the samza-resource-ID of the container when it failed
(used to index failover-state)
*/
- private void initiateActiveContainerFailover(String containerID, String
resourceID,
+ private void initiateStandbyAwareAllocation(String activeContainerID, String
resourceID,
AbstractContainerAllocator containerAllocator) {
- Optional<Entry<String, SamzaResource>> standbyContainer =
this.selectStandby(containerID, resourceID);
+ String standbyHost = this.selectStandbyHost(activeContainerID, resourceID);
- // If we find a standbyContainer, we initiate a failover
- if (standbyContainer.isPresent()) {
+ // Check if there is a running standby-container on that host that needs
to be stopped
+ List<String> standbyContainers =
this.standbyContainerConstraints.get(activeContainerID);
+ Map<String, SamzaResource> runningStandbyContainersOnHost =
this.samzaApplicationState.runningContainers.entrySet().stream().filter(x ->
standbyContainers.contains(x.getKey()))
Review comment:
Personally, I think it's cleaner to restructure this as follows:
```
runningContainers.forEach((samzaContainerId, samzaResource) -> {
if (samzaContainerId in standbySamzaContainerIds and samzaResource.host ==
standByHost) {
// update the map
}
}
```
Alternately, if you still prefer using Java 8 streams, I'd suggest to
consolidating the two filter operators
----------------------------------------------------------------
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
