[GitHub] [gobblin] Will-Lo commented on a diff in pull request #3638: [GOBBLIN-1781] Helix offline instance purging is not thread safe in the yarn service

["Will-Lo (via GitHub)" <gi...@apache.org>]

Will-Lo commented on code in PR #3638:
URL: https://github.com/apache/gobblin/pull/3638#discussion_r1100589492


##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java:
##########
@@ -463,9 +465,14 @@ private EventSubmitter buildEventSubmitter() {
    * @param yarnContainerRequestBundle the desired containers information, including numbers, resource and helix tag
    * @param inUseInstances  a set of in use instances
    */

Review Comment:
   Update the javadoc for what the return value means in this case



##########
gobblin-yarn/src/main/java/org/apache/gobblin/yarn/YarnService.java:
##########
@@ -463,9 +465,14 @@ private EventSubmitter buildEventSubmitter() {
    * @param yarnContainerRequestBundle the desired containers information, including numbers, resource and helix tag
    * @param inUseInstances  a set of in use instances
    */
-  public synchronized void requestTargetNumberOfContainers(YarnContainerRequestBundle yarnContainerRequestBundle, Set<String> inUseInstances) {
+  public synchronized boolean requestTargetNumberOfContainers(YarnContainerRequestBundle yarnContainerRequestBundle, Set<String> inUseInstances) {
     LOGGER.info("Trying to set numTargetContainers={}, in-use helix instances count is {}, container map size is {}",
         yarnContainerRequestBundle.getTotalContainers(), inUseInstances.size(), this.containerMap.size());
+    if (startupInProgress) {
+      LOGGER.info("YarnService is still starting up. Unable to request containers from yarn until YarnService is finished starting up.");

Review Comment:
   Nit: This should be a warn log



-- 
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: dev-unsubscribe@gobblin.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org