homatthew commented on code in PR #3692:
URL: https://github.com/apache/gobblin/pull/3692#discussion_r1198417782
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java:
##########
@@ -473,6 +475,17 @@ public synchronized boolean
requestTargetNumberOfContainers(YarnContainerRequest
return false;
}
+ //Correct the containerMap first as there is cases that
handleContainerCompletion() is called before onContainersAllocated()
+ for (ContainerId removedId :this.removedContainerID.keySet()) {
+ ContainerInfo containerInfo = this.containerMap.remove(removedId);
+ if (containerInfo != null) {
+ String helixTag = containerInfo.getHelixTag();
+ allocatedContainerCountMap.putIfAbsent(helixTag, new AtomicInteger(0));
+ this.allocatedContainerCountMap.get(helixTag).decrementAndGet();
Review Comment:
> When Yarn allocates "ghost containers" without calling the
onContainerAllocated() method and when the container is eventually released,
onContainersCompleted() is called, container numbers mismatches can occur.
In the onContainerAllocated() method, we add the container to the
containerMap using the container ID as the key, and increase the count for the
specific tag.
This from the description makes it sound like it is not guaranteed to be
called. Either way, I think if we do need to put this line, we should put if
absent 1 and then decrement
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
