[ 
https://issues.apache.org/jira/browse/GOBBLIN-2189?focusedWorklogId=957698&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-957698
 ]

ASF GitHub Bot logged work on GOBBLIN-2189:
-------------------------------------------

                Author: ASF GitHub Bot
            Created on: 19/Feb/25 10:53
            Start Date: 19/Feb/25 10:53
    Worklog Time Spent: 10m 
      Work Description: khandelwal-prateek commented on code in PR #4092:
URL: https://github.com/apache/gobblin/pull/4092#discussion_r1961358840


##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/yarn/AbstractDynamicScalingYarnServiceManager.java:
##########
@@ -110,9 +110,13 @@ public void run() {
         List<ScalingDirective> scalingDirectives = 
scalingDirectiveSource.getScalingDirectives();
         if (CollectionUtils.isNotEmpty(scalingDirectives)) {
           
dynamicScalingYarnService.reviseWorkforcePlanAndRequestNewContainers(scalingDirectives);
+        } else {
+          dynamicScalingYarnService.calcDeltasAndRequestContainers();
         }
       } catch (FileNotFoundException fnfe) {

Review Comment:
   when is this exception thrown? there is already a check for empty 
scalingDirective. Also, we should not be using exception as part of the control 
flow 



##########
gobblin-temporal/src/main/java/org/apache/gobblin/temporal/yarn/AbstractDynamicScalingYarnServiceManager.java:
##########
@@ -110,9 +110,13 @@ public void run() {
         List<ScalingDirective> scalingDirectives = 
scalingDirectiveSource.getScalingDirectives();
         if (CollectionUtils.isNotEmpty(scalingDirectives)) {
           
dynamicScalingYarnService.reviseWorkforcePlanAndRequestNewContainers(scalingDirectives);
+        } else {
+          dynamicScalingYarnService.calcDeltasAndRequestContainers();
         }
       } catch (FileNotFoundException fnfe) {
-        log.warn("Failed to get scaling directives - " + fnfe.getMessage()); 
// important message, but no need for a stack trace
+        log.debug("Failed to get scaling directives - " + fnfe.getMessage()); 
// important message, but no need for a stack trace

Review Comment:
   I think this would be useful for debugging, any reason to change this to 
`debug` log level





Issue Time Tracking
-------------------

    Worklog Id:     (was: 957698)
    Time Spent: 3h 50m  (was: 3h 40m)

> Implement ContainerCompletion callback in DynamicScalingYarnService
> -------------------------------------------------------------------
>
>                 Key: GOBBLIN-2189
>                 URL: https://issues.apache.org/jira/browse/GOBBLIN-2189
>             Project: Apache Gobblin
>          Issue Type: Improvement
>          Components: gobblin-core
>            Reporter: Vivek Rai
>            Assignee: Abhishek Tiwari
>            Priority: Major
>          Time Spent: 3h 50m
>  Remaining Estimate: 0h
>
> DynamicScalingYarnService currently doesn't handle scaling down containers 
> and neither does anything if container is killed abruptly or goes OOM. So to 
> handle this scenario containerCompletion callback should be implemented to 
> launch the replacement containers and also scaling down handling should be 
> done.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to