[GitHub] [gobblin] Will-Lo commented on a change in pull request #3481: [GOBBLIN-1624] Refactor quota management, fix various bugs in accounting of running …

[GitBox <gi...@apache.org>]

Will-Lo commented on a change in pull request #3481:
URL: https://github.com/apache/gobblin/pull/3481#discussion_r836681064



##########
File path: gobblin-service/src/main/java/org/apache/gobblin/service/modules/orchestration/DagManager.java
##########
@@ -1086,15 +1008,17 @@ private int getQuotaForUser(String user) {
       ExecutionStatus jobStatus = DagManagerUtils.getExecutionStatus(dagNode);
       log.info("Job {} of Dag {} has finished with status {}", jobName, dagId, jobStatus.name());
 
-      releaseQuota(dagNode);
-
-      if (this.metricContext != null) {
-        getRunningJobsCounter(dagNode).dec();
-        getRunningJobsCounterForUser(dagNode).forEach(ContextAwareCounter::dec);
+      // Only decrement counters and quota for jobs that actually ran on the executor, not from a GaaS side failure/skip event
+      if (this.runningDags.contains(dagId)) {
+        quotaManager.releaseQuota(dagNode);
+        runningDags.remove(dagId);

Review comment:
       Yes only one thread would work with the same dagId, even if there are concurrent executions with the same flowname and flowgroup it will have a unique flowexecutionId that's generated from the timestamp. I'll add a comment for this




-- 
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