[helix] branch master updated: Fix ConcurrentModification exception in Workflow Garbage Collection (#741)

[hulee]

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 12f11a3  Fix ConcurrentModification exception in Workflow Garbage 
Collection (#741)
12f11a3 is described below

commit 12f11a357d4ee8859a408357c57b1cad854ed841
Author: Ali Reza Zamani Zadeh Najari <anaj...@linkedin.com>
AuthorDate: Wed Feb 12 11:44:53 2020 -0800

    Fix ConcurrentModification exception in Workflow Garbage Collection (#741)
    
    In workflow Garbage collection, there is possibility that we encounter
    ConcurrentMod exception while looping through the workflow contexts.
    This commit fixes this issue by adding a try-catch.
---
 .../main/java/org/apache/helix/task/TaskUtil.java  | 37 ++++++++++++++++------
 .../task/TestWorkflowContextWithoutConfig.java     |  2 +-
 2 files changed, 28 insertions(+), 11 deletions(-)

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 6b36db4..5ae2f07 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
@@ -1043,23 +1043,40 @@ public class TaskUtil {
    * @param dataProvider
    * @param manager
    */
-  public static void workflowGarbageCollection(WorkflowControllerDataProvider 
dataProvider,
+  public static void workflowGarbageCollection(final 
WorkflowControllerDataProvider dataProvider,
       final HelixManager manager) {
     // Garbage collections for conditions where workflow context exists but 
config is missing.
-    Map<String, ZNRecord> contexts = dataProvider.getContexts();
-    HelixDataAccessor accessor = manager.getHelixDataAccessor();
-    HelixPropertyStore<ZNRecord> propertyStore = 
manager.getHelixPropertyStore();
 
+    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 (Map.Entry<String, ZNRecord> entry : contexts.entrySet()) {
-      if (entry.getValue() != null
-          && entry.getValue().getId().equals(TaskUtil.WORKFLOW_CONTEXT_KW)) {
-        if (dataProvider.getWorkflowConfig(entry.getKey()) == null) {
-          toBeDeletedWorkflows.add(entry.getKey());
-        }
+    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!!",
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 b7d6016..d1102e1 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
@@ -170,7 +170,7 @@ public class TestWorkflowContextWithoutConfig extends 
TaskTestBase {
     Assert.assertTrue(contextDeleted);
   }
 
-  Workflow.Builder createSimpleWorkflowBuilder(String workflowName) {
+  private Workflow.Builder createSimpleWorkflowBuilder(String workflowName) {
     final long expiryTime = 5000L;
     Workflow.Builder builder = new Workflow.Builder(workflowName);