[GitHub] [helix] NealSun96 commented on a change in pull request #1508: Fix ondemand rebalance flooding and log flooding caused by dangling jobs

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

NealSun96 commented on a change in pull request #1508:
URL: https://github.com/apache/helix/pull/1508#discussion_r516415341



##########
File path: helix-core/src/main/java/org/apache/helix/common/caches/TaskDataCache.java
##########
@@ -116,33 +116,6 @@ public synchronized boolean refresh(HelixDataAccessor accessor,
       }
     }
 
-    // The following 3 blocks is for finding a list of workflows whose JobDAGs have been changed

Review comment:
       Why this section is removed: this section is refresh runtime dags based on JobConfigs. For every new job config, its workflow is added; for ever job config that doesn't exist in the jobqueue's dag (seems to be duplicate with the first part of logic), its workflow is added. 
   
   With dangling jobs, their jobConfigs are never deleted, so this section is activated all the time and the workflow, if exists, will have its DAG refreshed all the time. The problem is detailed in PR description. 
   
   I don't think the extra logic for new job config or for the race condition makes much sense, given that a new logic which compares WorkflowConfig version is in place - if a new job is added, eventually the DAG will be updated, which will cause a version change and a runtime dag refresh. 




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@helix.apache.org
For additional commands, e-mail: reviews-help@helix.apache.org