khandelwal-prateek commented on code in PR #4092: URL: https://github.com/apache/gobblin/pull/4092#discussion_r1945994951
########## gobblin-temporal/src/main/java/org/apache/gobblin/temporal/yarn/DynamicScalingYarnService.java: ########## @@ -42,18 +54,27 @@ */ @Slf4j public class DynamicScalingYarnService extends YarnService { + private static final String DEFAULT_REPLACEMENT_CONTAINER_WORKER_PROFILE_NAME_PREFIX = "replacementWorkerProfile"; + private static final int LAUNCH_CONTAINER_FAILED_EXIT_CODE = 1; + private static final int GENERAL_OOM_EXIT_STATUS_CODE = 137; + private static final int DEFAULT_REPLACEMENT_CONTAINER_MEMORY_MULTIPLIER = 2; + private static final int MAX_REPLACEMENT_CONTAINER_MEMORY_MBS = 65536; // 64GB /** this holds the current count of containers already requested for each worker profile */ private final WorkforceStaffing actualWorkforceStaffing; /** this holds the current total workforce plan as per latest received scaling directives */ private final WorkforcePlan workforcePlan; + private final Set<ContainerId> removedContainerIds; + private final AtomicLong profileNameSuffixGenerator; public DynamicScalingYarnService(Config config, String applicationName, String applicationId, YarnConfiguration yarnConfiguration, FileSystem fs, EventBus eventBus) throws Exception { super(config, applicationName, applicationId, yarnConfiguration, fs, eventBus); this.actualWorkforceStaffing = WorkforceStaffing.initialize(0); this.workforcePlan = new WorkforcePlan(this.config, this.config.getInt(GobblinYarnConfigurationKeys.INITIAL_CONTAINERS_KEY)); + this.removedContainerIds = ConcurrentHashMap.newKeySet(); Review Comment: update to use ConcurrentLinkedQueue, as `ConcurrentHashMap.newKeySet()` works better for membership checks but not for ordered or frequent removals. ``` private final Queue<ContainerId> removedContainerIds; removedContainerIds = new ConcurrentLinkedQueue<>(); ``` Also, if another thread adds/removes an element while iterating, the iterator may skip those modifications during iteration as these may not be immediately visible in ConcurrentHashMap. -- 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-unsubscr...@gobblin.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org