homatthew commented on code in PR #3586:
URL: https://github.com/apache/gobblin/pull/3586#discussion_r1001069688
##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java:
##########
@@ -845,8 +877,6 @@ public void onContainersAllocated(List<Container>
containers) {
instanceName = null;
}
}
- allocatedContainerCountMap.put(containerHelixTag,
Review Comment:
But then again the comment above says
```
Ensure that updates to unusedHelixInstanceNames are visible to other threads
that might concurrently
invoke the callback on container allocation.
```
Which implies there could be multiple callers to this single object. In that
case, why don't we have a lock for oncomplete method which decrements this
count?
Either way, the code is so painful to reason about when we have all these
concurrent maps + explicit locks... Gives a sense of "safety" but still is very
error prone IMO
--
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]
