You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by jx...@apache.org on 2018/04/02 18:22:20 UTC

[2/2] helix git commit: Avoid to read data from ZK for ExternalViewStage

Avoid to read data from ZK for ExternalViewStage

Helix Controller is trying to external view back to compare with ZK data, which is not necessary since we can cache the data and compare them in memory.

RB=1244812
BUG=HELIX-844
G=helix-reviewers
A=jjwang


Project: http://git-wip-us.apache.org/repos/asf/helix/repo
Commit: http://git-wip-us.apache.org/repos/asf/helix/commit/f9e43c02
Tree: http://git-wip-us.apache.org/repos/asf/helix/tree/f9e43c02
Diff: http://git-wip-us.apache.org/repos/asf/helix/diff/f9e43c02

Branch: refs/heads/master
Commit: f9e43c0229be20ea1f34cb5aa7f26f3452b99486
Parents: 6b6ba3c
Author: Junkai Xue <jx...@linkedin.com>
Authored: Wed Mar 7 16:55:30 2018 -0800
Committer: Junkai Xue <jx...@linkedin.com>
Committed: Mon Apr 2 11:21:59 2018 -0700

----------------------------------------------------------------------
 .../controller/stages/ClusterDataCache.java     | 33 ++++++++++
 .../stages/ExternalViewComputeStage.java        | 13 ++--
 .../stages/TestExternalViewStage.java           | 65 ++++++++++++++++++++
 3 files changed, 107 insertions(+), 4 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/helix/blob/f9e43c02/helix-core/src/main/java/org/apache/helix/controller/stages/ClusterDataCache.java
----------------------------------------------------------------------
diff --git a/helix-core/src/main/java/org/apache/helix/controller/stages/ClusterDataCache.java b/helix-core/src/main/java/org/apache/helix/controller/stages/ClusterDataCache.java
index 3582480..734b7f7 100644
--- a/helix-core/src/main/java/org/apache/helix/controller/stages/ClusterDataCache.java
+++ b/helix-core/src/main/java/org/apache/helix/controller/stages/ClusterDataCache.java
@@ -83,6 +83,7 @@ public class ClusterDataCache {
   private Map<String, Map<String, String>> _idealStateRuleMap;
   private Map<String, Map<String, Long>> _missingTopStateMap = new HashMap<>();
   private Map<String, ExternalView> _targetExternalViewMap = new HashMap<>();
+  private Map<String, ExternalView> _externalViewMap = new HashMap<>();
   private Map<String, Map<String, Set<String>>> _disabledInstanceForPartitionMap = new HashMap<>();
   private Set<String> _disabledInstanceSet = new HashSet<>();
 
@@ -713,6 +714,38 @@ public class ClusterDataCache {
   }
 
   /**
+   * Get local cached external view map
+   *
+   * @return
+   */
+  public Map<String, ExternalView> getExternalViews() {
+    return Collections.unmodifiableMap(_externalViewMap);
+  }
+
+  /**
+   * Update the cached external view map
+   *
+   * @param externalViews
+   */
+  public void updateExternalViews(List<ExternalView> externalViews) {
+    for (ExternalView externalView : externalViews) {
+      _externalViewMap.put(externalView.getResourceName(), externalView);
+    }
+  }
+
+  /**
+   * Remove dead external views from map
+   *
+   * @param resourceNames
+   */
+
+  public void removeExternalViews(List<String> resourceNames) {
+    for (String externalView : resourceNames) {
+      _externalViewMap.remove(externalView);
+    }
+  }
+
+  /**
    * Indicate that a full read should be done on the next refresh
    */
   public synchronized void requireFullRefresh() {

http://git-wip-us.apache.org/repos/asf/helix/blob/f9e43c02/helix-core/src/main/java/org/apache/helix/controller/stages/ExternalViewComputeStage.java
----------------------------------------------------------------------
diff --git a/helix-core/src/main/java/org/apache/helix/controller/stages/ExternalViewComputeStage.java b/helix-core/src/main/java/org/apache/helix/controller/stages/ExternalViewComputeStage.java
index 9e91ba5..e2bd2a9 100644
--- a/helix-core/src/main/java/org/apache/helix/controller/stages/ExternalViewComputeStage.java
+++ b/helix-core/src/main/java/org/apache/helix/controller/stages/ExternalViewComputeStage.java
@@ -53,10 +53,9 @@ public class ExternalViewComputeStage extends AbstractBaseStage {
     CurrentStateOutput currentStateOutput =
         event.getAttribute(AttributeName.CURRENT_STATE.name());
 
-    List<ExternalView> newExtViews = new ArrayList<ExternalView>();
+    List<ExternalView> newExtViews = new ArrayList<>();
 
-    Map<String, ExternalView> curExtViews =
-        dataAccessor.getChildValuesMap(keyBuilder.externalViews());
+    Map<String, ExternalView> curExtViews = cache.getExternalViews();
 
     for (String resourceName : resourceMap.keySet()) {
       ExternalView view = new ExternalView(resourceName);
@@ -130,12 +129,14 @@ public class ExternalViewComputeStage extends AbstractBaseStage {
         }
       }
     }
+
+    List<String> externalviewsToRemove = new ArrayList<>();
     // TODO: consider not setting the externalview of SCHEDULER_TASK_QUEUE at all.
     // Are there any entity that will be interested in its change?
 
     // For the resource with DisableExternalView option turned on in IdealState
     // We will not actually create or write the externalView to ZooKeeper.
-    List<PropertyKey> keys = new ArrayList<PropertyKey>();
+    List<PropertyKey> keys = new ArrayList<>();
     for(Iterator<ExternalView> it = newExtViews.iterator(); it.hasNext(); ) {
       ExternalView view = it.next();
       String resourceName = view.getResourceName();
@@ -146,6 +147,7 @@ public class ExternalViewComputeStage extends AbstractBaseStage {
         if (curExtViews.containsKey(resourceName)) {
           LOG.info("Remove externalView for resource: " + resourceName);
           dataAccessor.removeProperty(keyBuilder.externalView(resourceName));
+          externalviewsToRemove.add(resourceName);
         }
       } else {
         keys.add(keyBuilder.externalView(resourceName));
@@ -155,6 +157,7 @@ public class ExternalViewComputeStage extends AbstractBaseStage {
     // add/update external-views
     if (newExtViews.size() > 0) {
       dataAccessor.setChildren(keys, newExtViews);
+      cache.updateExternalViews(newExtViews);
     }
 
     // remove dead external-views
@@ -162,8 +165,10 @@ public class ExternalViewComputeStage extends AbstractBaseStage {
       if (!resourceMap.keySet().contains(resourceName)) {
         LOG.info("Remove externalView for resource: " + resourceName);
         dataAccessor.removeProperty(keyBuilder.externalView(resourceName));
+        externalviewsToRemove.add(resourceName);
       }
     }
+    cache.removeExternalViews(externalviewsToRemove);
   }
 
   private void updateScheduledTaskStatus(ExternalView ev, HelixManager manager,

http://git-wip-us.apache.org/repos/asf/helix/blob/f9e43c02/helix-core/src/test/java/org/apache/helix/controller/stages/TestExternalViewStage.java
----------------------------------------------------------------------
diff --git a/helix-core/src/test/java/org/apache/helix/controller/stages/TestExternalViewStage.java b/helix-core/src/test/java/org/apache/helix/controller/stages/TestExternalViewStage.java
new file mode 100644
index 0000000..f29ae19
--- /dev/null
+++ b/helix-core/src/test/java/org/apache/helix/controller/stages/TestExternalViewStage.java
@@ -0,0 +1,65 @@
+package org.apache.helix.controller.stages;
+
+import java.util.List;
+import org.apache.helix.HelixDataAccessor;
+import org.apache.helix.HelixManager;
+import org.apache.helix.TestHelper;
+import org.apache.helix.ZNRecord;
+import org.apache.helix.ZkUnitTestBase;
+import org.apache.helix.controller.pipeline.Pipeline;
+import org.apache.helix.manager.zk.ZKHelixDataAccessor;
+import org.apache.helix.manager.zk.ZkBaseDataAccessor;
+import org.apache.helix.model.ExternalView;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+public class TestExternalViewStage extends ZkUnitTestBase {
+  @Test
+  public void testCachedExternalViews() throws Exception {
+    String clusterName = "CLUSTER_" + TestHelper.getTestMethodName();
+
+    HelixDataAccessor accessor =
+        new ZKHelixDataAccessor(clusterName, new ZkBaseDataAccessor<ZNRecord>(_gZkClient));
+    HelixManager manager = new DummyClusterManager(clusterName, accessor);
+
+    // ideal state: node0 is MASTER, node1 is SLAVE
+    // replica=2 means 1 master and 1 slave
+    setupIdealState(clusterName, new int[] {
+        0, 1
+    }, new String[] {
+        "TestDB"
+    }, 1, 2);
+    setupLiveInstances(clusterName, new int[] {
+        0, 1
+    });
+    setupStateModel(clusterName);
+
+    ClusterEvent event = new ClusterEvent(ClusterEventType.Unknown);
+    ClusterDataCache cache = new ClusterDataCache(clusterName);
+    event.addAttribute(AttributeName.helixmanager.name(), manager);
+    event.addAttribute(AttributeName.ClusterDataCache.name(), cache);
+
+    ExternalViewComputeStage externalViewComputeStage = new ExternalViewComputeStage();
+    Pipeline dataRefresh = new Pipeline();
+    dataRefresh.addStage(new ReadClusterDataStage());
+    runPipeline(event, dataRefresh);
+    runStage(event, new ResourceComputationStage());
+    runStage(event, new CurrentStateComputationStage());
+    runStage(event, externalViewComputeStage);
+    Assert.assertEquals(cache.getExternalViews().values(),
+        accessor.getChildValues(accessor.keyBuilder().externalViews()));
+
+    // Assure there is no external got updated
+    List<ExternalView> oldExternalViews =
+        accessor.getChildValues(accessor.keyBuilder().externalViews());
+    runStage(event, externalViewComputeStage);
+    List<ExternalView> newExternalViews =
+        accessor.getChildValues(accessor.keyBuilder().externalViews());
+    Assert.assertEquals(oldExternalViews, newExternalViews);
+    for (int i = 0; i < oldExternalViews.size(); i++) {
+      Assert.assertEquals(oldExternalViews.get(i).getStat().getVersion(),
+          newExternalViews.get(i).getStat().getVersion());
+    }
+  }
+
+}