[helix] branch master updated: Remove workflow garbage collection (#803)

[hu...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

hulee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/helix.git


The following commit(s) were added to refs/heads/master by this push:
     new 19811ea  Remove workflow garbage collection (#803)
19811ea is described below

commit 19811eaa32e532a4ddf8c4168391ebc28ba11e75
Author: Ali Reza Zamani Zadeh Najari <an...@linkedin.com>
AuthorDate: Mon Feb 24 19:11:58 2020 -0800

    Remove workflow garbage collection (#803)
    
    Remove the workflow garbage collection function and its corresponding test
    
    This is because the garbage collection caused removal of active workflows and workflows that shouldn't be garbage collected, which is a critical bug.
---
 .../stages/TaskGarbageCollectionStage.java         |  1 -
 .../main/java/org/apache/helix/task/TaskUtil.java  | 60 -------------------
 .../task/TestWorkflowContextWithoutConfig.java     | 67 ----------------------
 3 files changed, 128 deletions(-)

diff --git a/helix-core/src/main/java/org/apache/helix/controller/stages/TaskGarbageCollectionStage.java b/helix-core/src/main/java/org/apache/helix/controller/stages/TaskGarbageCollectionStage.java
index 9aca3ef..3f8e744 100644
--- a/helix-core/src/main/java/org/apache/helix/controller/stages/TaskGarbageCollectionStage.java
+++ b/helix-core/src/main/java/org/apache/helix/controller/stages/TaskGarbageCollectionStage.java
@@ -52,6 +52,5 @@ public class TaskGarbageCollectionStage extends AbstractAsyncBaseStage {
       }
     }
 
-    TaskUtil.workflowGarbageCollection(dataProvider, manager);
   }
 }
diff --git a/helix-core/src/main/java/org/apache/helix/task/TaskUtil.java b/helix-core/src/main/java/org/apache/helix/task/TaskUtil.java
index 5ae2f07..917a69b 100644
--- a/helix-core/src/main/java/org/apache/helix/task/TaskUtil.java
+++ b/helix-core/src/main/java/org/apache/helix/task/TaskUtil.java
@@ -1037,66 +1037,6 @@ public class TaskUtil {
     setNextJobPurgeTime(workflow, currentTime, purgeInterval, rebalanceScheduler, manager);
   }
 
-  /**
-   * The function that loops through the all existing workflow contexts and removes IdealState and
-   * workflow context of the workflow whose workflow config does not exist.
-   * @param dataProvider
-   * @param manager
-   */
-  public static void workflowGarbageCollection(final WorkflowControllerDataProvider dataProvider,
-      final HelixManager manager) {
-    // Garbage collections for conditions where workflow context exists but config is missing.
-
-    Set<String> existingContexts;
-    /*
-     * Here try-catch is used to avoid concurrent modification exception while doing deep copy.
-     * Map.keySet() can produce concurrent modification exception.
-     * Reason: If the map is modified while an iteration over the set is in progress, concurrent
-     * modification exception will be thrown.
-     */
-    try {
-      existingContexts = new HashSet<>(dataProvider.getContexts().keySet());
-    } catch (Exception e) {
-      LOG.warn(
-          "Exception occurred while creating a list of all workflow/job context names!",
-          e);
-      return;
-    }
-
-    // toBeDeletedWorkflows is a set that contains the name of the workflows that their contexts
-    // should be deleted.
-    Set<String> toBeDeletedWorkflows = new HashSet<>();
-    for (String entry : existingContexts) {
-      WorkflowConfig cfg = dataProvider.getWorkflowConfig(entry);
-      WorkflowContext ctx = dataProvider.getWorkflowContext(entry);
-      if (ctx != null && ctx.getId().equals(TaskUtil.WORKFLOW_CONTEXT_KW) && cfg == null) {
-        toBeDeletedWorkflows.add(entry);
-      }
-    }
-
-    HelixDataAccessor accessor = manager.getHelixDataAccessor();
-    HelixPropertyStore<ZNRecord> propertyStore = manager.getHelixPropertyStore();
-
-    for (String workflowName : toBeDeletedWorkflows) {
-      LOG.warn(String.format(
-          "WorkflowContext exists for workflow %s. However, Workflow Config is missing! Deleting the WorkflowConfig and IdealState!!",
-          workflowName));
-
-      if (!cleanupWorkflowIdealStateExtView(accessor, workflowName)) {
-        LOG.warn(String.format(
-            "Error occurred while trying to remove workflow idealstate/externalview for %s.",
-            workflowName));
-        continue;
-      }
-
-      if (!removeWorkflowContext(propertyStore, workflowName)) {
-        LOG.warn(String.format("Error occurred while trying to remove workflow context for %s.",
-            workflowName));
-        continue;
-      }
-    }
-  }
-
   private static void setNextJobPurgeTime(String workflow, long currentTime, long purgeInterval,
       RebalanceScheduler rebalanceScheduler, HelixManager manager) {
     long nextPurgeTime = currentTime + purgeInterval;
diff --git a/helix-core/src/test/java/org/apache/helix/integration/task/TestWorkflowContextWithoutConfig.java b/helix-core/src/test/java/org/apache/helix/integration/task/TestWorkflowContextWithoutConfig.java
index d1102e1..6c29f3a 100644
--- a/helix-core/src/test/java/org/apache/helix/integration/task/TestWorkflowContextWithoutConfig.java
+++ b/helix-core/src/test/java/org/apache/helix/integration/task/TestWorkflowContextWithoutConfig.java
@@ -103,73 +103,6 @@ public class TestWorkflowContextWithoutConfig extends TaskTestBase {
     Assert.assertTrue(workflowContextNotCreated);
   }
 
-  @Test
-  public void testWorkflowContextGarbageCollection() throws Exception {
-    String workflowName = TestHelper.getTestMethodName();
-    Workflow.Builder builder1 = createSimpleWorkflowBuilder(workflowName);
-    _driver.start(builder1.build());
-
-    // Wait until workflow is created and IN_PROGRESS state.
-    _driver.pollForWorkflowState(workflowName, TaskState.IN_PROGRESS);
-
-    // Check that WorkflowConfig, WorkflowContext, and IdealState are indeed created for this
-    // workflow
-    Assert.assertNotNull(_driver.getWorkflowConfig(workflowName));
-    Assert.assertNotNull(_driver.getWorkflowContext(workflowName));
-    Assert.assertNotNull(_admin.getResourceIdealState(CLUSTER_NAME, workflowName));
-
-    String workflowContextPath =
-        "/" + CLUSTER_NAME + "/PROPERTYSTORE/TaskRebalancer/" + workflowName + "/Context";
-
-    ZNRecord record = _manager.getHelixDataAccessor().getBaseDataAccessor().get(workflowContextPath,
-        null, AccessOption.PERSISTENT);
-    Assert.assertNotNull(record);
-
-    // Wait until workflow is completed.
-    _driver.pollForWorkflowState(workflowName, TaskState.COMPLETED);
-
-    // Verify that WorkflowConfig, WorkflowContext, and IdealState are removed after workflow got
-    // expired.
-    boolean workflowExpired = TestHelper.verify(() -> {
-      WorkflowContext wCtx = _driver.getWorkflowContext(workflowName);
-      WorkflowConfig wCfg = _driver.getWorkflowConfig(workflowName);
-      IdealState idealState = _admin.getResourceIdealState(CLUSTER_NAME, workflowName);
-      return (wCtx == null && wCfg == null && idealState == null);
-    }, TestHelper.WAIT_DURATION);
-    Assert.assertTrue(workflowExpired);
-
-
-    _controller.syncStop();
-
-    // Write workflow context to ZooKeeper
-    _manager.getHelixDataAccessor().getBaseDataAccessor().set(workflowContextPath, record,
-        AccessOption.PERSISTENT);
-
-    // Verify context is written back to ZK.
-    record = _manager.getHelixDataAccessor().getBaseDataAccessor().get(workflowContextPath,
-        null, AccessOption.PERSISTENT);
-    Assert.assertNotNull(record);
-
-    // start controller
-    String controllerName = CONTROLLER_PREFIX + "_0";
-    _controller = new ClusterControllerManager(ZK_ADDR, CLUSTER_NAME, controllerName);
-    _controller.syncStart();
-
-    // Create and start new workflow just to make sure controller is running and new workflow is
-    // scheduled successfully.
-    String workflowName2 = TestHelper.getTestMethodName() + "_2";
-    Workflow.Builder builder2 = createSimpleWorkflowBuilder(workflowName2);
-    _driver.start(builder2.build());
-    _driver.pollForWorkflowState(workflowName2, TaskState.COMPLETED);
-
-    // Verify that WorkflowContext will be deleted
-    boolean contextDeleted = TestHelper.verify(() -> {
-      WorkflowContext wCtx = _driver.getWorkflowContext(workflowName);
-      return (wCtx == null);
-    }, TestHelper.WAIT_DURATION);
-    Assert.assertTrue(contextDeleted);
-  }
-
   private Workflow.Builder createSimpleWorkflowBuilder(String workflowName) {
     final long expiryTime = 5000L;
     Workflow.Builder builder = new Workflow.Builder(workflowName);