aditya1105 commented on code in PR #3932:
URL: https://github.com/apache/gobblin/pull/3932#discussion_r1575772969
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java:
##########
@@ -335,13 +350,15 @@ public void
handleContainerReleaseRequest(ContainerReleaseRequest containerRelea
for (Container container : containerReleaseRequest.getContainers()) {
LOGGER.info(String.format("Releasing container %s running on %s",
container.getId(), container.getNodeId()));
- // Record that this container was explicitly released so that a new one
is not spawned to replace it
- // Put the container id in the releasedContainerCache before releasing
it so that handleContainerCompletion()
- // can check for the container id and skip spawning a replacement
container.
- // Note that this is the best effort since these are asynchronous
operations and a container may abort concurrently
- // with the release call. So in some cases a replacement container may
have already been spawned before
- // the container is put into the black list.
- this.releasedContainerCache.put(container.getId(), "");
+ if(!containerReleaseRequest.isSpinUpReplacementContainers()) {
+ // Record that this container was explicitly released so that a new
one is not spawned to replace it
+ // Put the container id in the releasedContainerCache before releasing
it so that handleContainerCompletion()
+ // can check for the container id and skip spawning a replacement
container.
+ // Note that this is the best effort since these are asynchronous
operations and a container may abort concurrently
+ // with the release call. So in some cases a replacement container may
have already been spawned before
+ // the container is put into the black list.
+ this.releasedContainerCache.put(container.getId(), "");
Review Comment:
Require some explanation on this part.
1) What is the utility of this cache?
2) Why we do we check "isSpinUpReplacementContainers" and then only put in
cache? while we log and release assigned containers without this check? I think
1st should be the answer to this question.
--
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]
