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



##########
File path: helix-core/src/main/java/org/apache/helix/task/TaskDriver.java
##########
@@ -746,13 +705,11 @@ public void deleteAndWaitForCompletion(String workflow, 
long timeout)
     BaseDataAccessor baseDataAccessor = _accessor.getBaseDataAccessor();
     PropertyKey.Builder keyBuilder = _accessor.keyBuilder();
 
-    String idealStatePath = keyBuilder.idealStates(workflow).getPath();
     String workflowConfigPath = keyBuilder.resourceConfig(workflow).getPath();
     String workflowContextPath = 
keyBuilder.workflowContext(workflow).getPath();
 
     while (System.currentTimeMillis() <= endTime) {
-      if (baseDataAccessor.exists(idealStatePath, AccessOption.PERSISTENT)
-          || baseDataAccessor.exists(workflowConfigPath, 
AccessOption.PERSISTENT)
+      if (baseDataAccessor.exists(workflowConfigPath, AccessOption.PERSISTENT)

Review comment:
       So this section you commented on is only used for testing purpose, as 
far as I know. 
   
   However, you raised a good point here: 
   1. Workflows relying on the current pipeline logic should be fine during the 
switch, because they are guaranteed to have workflow configs.
   2. Workflows relying on the old pipeline logic (no config) wouldn't be 
managed by the controller after this change. Is that a concern?
   
   In general, keeping the IS clean up has only one advantage, which is to 
clean up the leftover IS from pre-existing workflows. However I don't think 
that is an huge issue. 




----------------------------------------------------------------
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:
[email protected]



---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to