You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by ji...@apache.org on 2020/04/23 19:28:03 UTC

[helix] 17/23: Replace customized view cache with property cache (#869)

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

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

commit 9c40583cdc411a1731bb19c8d941a45b584039e8
Author: Ali Reza Zamani Zadeh Najari <an...@linkedin.com>
AuthorDate: Mon Mar 30 16:47:13 2020 -0700

    Replace customized view cache with property cache (#869)
    
    In this commit, the custom implementation of customized view cache
    has been replaced with property cache implementation.
---
 .../helix/common/caches/CustomizedViewCache.java   | 116 +++++----------------
 .../spectator/TestRoutingTableProvider.java        |  69 ++++++++++--
 2 files changed, 86 insertions(+), 99 deletions(-)

diff --git a/helix-core/src/main/java/org/apache/helix/common/caches/CustomizedViewCache.java b/helix-core/src/main/java/org/apache/helix/common/caches/CustomizedViewCache.java
index d46be64..cdbc0f8 100644
--- a/helix-core/src/main/java/org/apache/helix/common/caches/CustomizedViewCache.java
+++ b/helix-core/src/main/java/org/apache/helix/common/caches/CustomizedViewCache.java
@@ -19,23 +19,16 @@ package org.apache.helix.common.caches;
  * under the License.
  */
 
-import java.util.ArrayList;
 import java.util.Collections;
-import java.util.HashMap;
-import java.util.HashSet;
-import java.util.List;
 import java.util.Map;
-import java.util.Set;
 
 import org.apache.helix.HelixDataAccessor;
-import org.apache.helix.HelixException;
 import org.apache.helix.PropertyKey;
 import org.apache.helix.PropertyType;
 import org.apache.helix.model.CustomizedView;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.collect.Maps;
 
 /**
  * Cache to hold all CustomizedView of a specific type.
@@ -43,8 +36,7 @@ import com.google.common.collect.Maps;
 public class CustomizedViewCache extends AbstractDataCache<CustomizedView> {
   private static final Logger LOG = LoggerFactory.getLogger(CustomizedViewCache.class.getName());
 
-  protected Map<String, CustomizedView> _customizedViewMap;
-  protected Map<String, CustomizedView> _customizedViewCache;
+  private final PropertyCache<CustomizedView> _customizedViewCache;
   protected String _clusterName;
   private PropertyType _propertyType;
   private String _customizedStateType;
@@ -54,13 +46,27 @@ public class CustomizedViewCache extends AbstractDataCache<CustomizedView> {
   }
 
   protected CustomizedViewCache(String clusterName, PropertyType propertyType, String customizedStateType) {
-    super(createDefaultControlContextProvider(clusterName));
-    _clusterName = clusterName;
-    _customizedViewMap = Collections.emptyMap();
-    _customizedViewCache = Collections.emptyMap();
-    _propertyType = propertyType;
-    _customizedStateType = customizedStateType;
-  }
+      super(createDefaultControlContextProvider(clusterName));
+      _clusterName = clusterName;
+      _propertyType = propertyType;
+      _customizedStateType = customizedStateType;
+      _customizedViewCache = new PropertyCache<>(AbstractDataCache.createDefaultControlContextProvider(clusterName), "CustomizedView", new PropertyCache.PropertyCacheKeyFuncs<CustomizedView>() {
+        @Override
+        public PropertyKey getRootKey(HelixDataAccessor accessor) {
+          return accessor.keyBuilder().customizedView(_customizedStateType);
+        }
+
+        @Override
+        public PropertyKey getObjPropertyKey(HelixDataAccessor accessor, String objName) {
+          return accessor.keyBuilder().customizedView(_customizedStateType, objName);
+        }
+
+        @Override
+        public String getObjName(CustomizedView obj) {
+          return obj.getResourceName();
+        }
+      }, true);
+    }
 
 
   /**
@@ -70,65 +76,7 @@ public class CustomizedViewCache extends AbstractDataCache<CustomizedView> {
    * @return
    */
   public void refresh(HelixDataAccessor accessor) {
-    long startTime = System.currentTimeMillis();
-    PropertyKey.Builder keyBuilder = accessor.keyBuilder();
-    Set<PropertyKey> currentPropertyKeys = new HashSet<>();
-
-    List<String> resources = accessor.getChildNames(customizedViewsKey(keyBuilder));
-
-    for (String resource : resources) {
-      currentPropertyKeys.add(customizedViewKey(keyBuilder, resource));
-    }
-
-    Set<PropertyKey> cachedKeys = new HashSet<>();
-    Map<PropertyKey, CustomizedView> cachedCustomizedViewMap = Maps.newHashMap();
-    for (String resource : _customizedViewCache.keySet()) {
-      PropertyKey key = customizedViewKey(keyBuilder, resource);
-      cachedKeys.add(key);
-      cachedCustomizedViewMap.put(key, _customizedViewCache.get(resource));
-    }
-    cachedKeys.retainAll(currentPropertyKeys);
-
-    Set<PropertyKey> reloadKeys = new HashSet<>(currentPropertyKeys);
-    reloadKeys.removeAll(cachedKeys);
-
-    Map<PropertyKey, CustomizedView> updatedMap =
-        refreshProperties(accessor, reloadKeys, new ArrayList<>(cachedKeys),
-            cachedCustomizedViewMap, new HashSet<>());
-
-    Map<String, CustomizedView> newCustomizedViewMap = Maps.newHashMap();
-    for (CustomizedView customizedView : updatedMap.values()) {
-      newCustomizedViewMap.put(customizedView.getResourceName(), customizedView);
-    }
-
-    _customizedViewCache = new HashMap<>(newCustomizedViewMap);
-    _customizedViewMap = new HashMap<>(newCustomizedViewMap);
-
-    long endTime = System.currentTimeMillis();
-    LOG.info("Refresh " + _customizedViewMap.size() + " CustomizedViews of type " + _customizedStateType
-        + " for cluster " + _clusterName + ", took " + (endTime - startTime) + " ms");
-  }
-
-  private PropertyKey customizedViewsKey(PropertyKey.Builder keyBuilder) {
-    PropertyKey customizedViewPropertyKey;
-    if (_propertyType.equals(PropertyType.CUSTOMIZEDVIEW)){
-      customizedViewPropertyKey = keyBuilder.customizedView(_customizedStateType);
-    } else {
-      throw new HelixException(
-          "Failed to refresh CustomizedViewCache, Wrong property type " + _propertyType + "!");
-    }
-    return customizedViewPropertyKey;
-  }
-
-  private PropertyKey customizedViewKey(PropertyKey.Builder keyBuilder, String resource) {
-    PropertyKey customizedViewPropertyKey;
-    if (_propertyType.equals(PropertyType.CUSTOMIZEDVIEW)) {
-      customizedViewPropertyKey = keyBuilder.customizedView(_customizedStateType, resource);
-    } else {
-      throw new HelixException(
-          "Failed to refresh CustomizedViewCache, Wrong property type " + _propertyType + "!");
-    }
-    return customizedViewPropertyKey;
+    _customizedViewCache.refresh(accessor);
   }
 
   /**
@@ -136,22 +84,6 @@ public class CustomizedViewCache extends AbstractDataCache<CustomizedView> {
    * @return
    */
   public Map<String, CustomizedView> getCustomizedViewMap() {
-    return Collections.unmodifiableMap(_customizedViewMap);
-  }
-
-  /**
-   * Remove dead customized views from map
-   * @param resourceNames
-   */
-
-  public synchronized void removeCustomizedView(List<String> resourceNames) {
-    for (String resourceName : resourceNames) {
-      _customizedViewCache.remove(resourceName);
-    }
-  }
-
-  public void clear() {
-    _customizedViewCache.clear();
-    _customizedViewMap.clear();
+    return Collections.unmodifiableMap(_customizedViewCache.getPropertyMap());
   }
 }
diff --git a/helix-core/src/test/java/org/apache/helix/integration/spectator/TestRoutingTableProvider.java b/helix-core/src/test/java/org/apache/helix/integration/spectator/TestRoutingTableProvider.java
index d1a2b4c..43ccdc9 100644
--- a/helix-core/src/test/java/org/apache/helix/integration/spectator/TestRoutingTableProvider.java
+++ b/helix-core/src/test/java/org/apache/helix/integration/spectator/TestRoutingTableProvider.java
@@ -1,5 +1,24 @@
 package org.apache.helix.integration.spectator;
 
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -47,6 +66,7 @@ public class TestRoutingTableProvider extends ZkTestBase {
   static final String CLUSTER_NAME = CLUSTER_PREFIX + "_" + CLASS_NAME;
   static final int PARTICIPANT_NUMBER = 3;
   static final int PARTICIPANT_START_PORT = 12918;
+  static final long WAIT_DURATION = 5 * 1000L; // 5 seconds
 
   static final int PARTITION_NUMBER = 20;
   static final int REPLICA_NUMBER = 3;
@@ -269,7 +289,7 @@ public class TestRoutingTableProvider extends ZkTestBase {
         customizedView.getRecord(), AccessOption.PERSISTENT);
 
     boolean onCustomizedViewChangeCalled =
-        TestHelper.verify(() -> customizedViewChangeCalled.get(), TestHelper.WAIT_DURATION);
+        TestHelper.verify(() -> customizedViewChangeCalled.get(), WAIT_DURATION);
     Assert.assertTrue(onCustomizedViewChangeCalled);
 
     _spectator.getHelixDataAccessor().getBaseDataAccessor().remove(customizedViewPath,
@@ -278,7 +298,7 @@ public class TestRoutingTableProvider extends ZkTestBase {
   }
 
   @Test(dependsOnMethods = "testCustomizedViewCorrectConstructor")
-  public void testGetRoutingTableSnapshot() {
+  public void testGetRoutingTableSnapshot() throws Exception {
     Map<PropertyType, List<String>> sourceDataTypes = new HashMap<>();
     sourceDataTypes.put(PropertyType.CUSTOMIZEDVIEW, Arrays.asList("typeA", "typeB"));
     sourceDataTypes.put(PropertyType.EXTERNALVIEW, Collections.emptyList());
@@ -308,19 +328,54 @@ public class TestRoutingTableProvider extends ZkTestBase {
         routingTableProvider.getRoutingTableSnapshot(PropertyType.CUSTOMIZEDVIEW, "typeA");
     Assert.assertEquals(routingTableSnapshot.getPropertyType(), PropertyType.CUSTOMIZEDVIEW);
     Assert.assertEquals(routingTableSnapshot.getCustomizedStateType(), "typeA");
+
     routingTableSnapshot =
         routingTableProvider.getRoutingTableSnapshot(PropertyType.CUSTOMIZEDVIEW, "typeB");
     Assert.assertEquals(routingTableSnapshot.getPropertyType(), PropertyType.CUSTOMIZEDVIEW);
     Assert.assertEquals(routingTableSnapshot.getCustomizedStateType(), "typeB");
 
-    Map<String, Map<String, RoutingTableSnapshot>> routingTableSnapshots =
-        routingTableProvider.getRoutingTableSnapshots();
-    Assert.assertEquals(routingTableSnapshots.size(), 2);
-    Assert.assertEquals(routingTableSnapshots.get(PropertyType.CUSTOMIZEDVIEW.name()).size(), 2);
+    // Make sure snapshot information is correct
+    // Check resources are in a correct state
+    boolean isRoutingTableUpdatedProperly = TestHelper.verify(() -> {
+      Map<String, Map<String, RoutingTableSnapshot>> routingTableSnapshots =
+          routingTableProvider.getRoutingTableSnapshots();
+      RoutingTableSnapshot routingTableSnapshotTypeA =
+          routingTableSnapshots.get(PropertyType.CUSTOMIZEDVIEW.name()).get("typeA");
+      RoutingTableSnapshot routingTableSnapshotTypeB =
+          routingTableSnapshots.get(PropertyType.CUSTOMIZEDVIEW.name()).get("typeB");
+      String typeAp1h1 = "noState";
+      String typeAp1h2 = "noState";
+      String typeAp2h1 = "noState";
+      String typeAp3h2 = "noState";
+      String typeBp1h2 = "noState";
+      String typeBp1h4 = "noState";
+      try {
+        typeAp1h1 = routingTableSnapshotTypeA.getCustomizeViews().iterator().next()
+            .getStateMap("p1").get("h1");
+        typeAp1h2 = routingTableSnapshotTypeA.getCustomizeViews().iterator().next()
+            .getStateMap("p1").get("h2");
+        typeAp2h1 = routingTableSnapshotTypeA.getCustomizeViews().iterator().next()
+            .getStateMap("p2").get("h1");
+        typeAp3h2 = routingTableSnapshotTypeA.getCustomizeViews().iterator().next()
+            .getStateMap("p3").get("h2");
+        typeBp1h2 = routingTableSnapshotTypeB.getCustomizeViews().iterator().next()
+            .getStateMap("p1").get("h3");
+        typeBp1h4 = routingTableSnapshotTypeB.getCustomizeViews().iterator().next()
+            .getStateMap("p1").get("h4");
+      } catch (Exception e) {
+        // ok because RoutingTable has not been updated yet
+        return false;
+      }
+      return (routingTableSnapshots.size() == 2
+          && routingTableSnapshots.get(PropertyType.CUSTOMIZEDVIEW.name()).size() == 2
+          && typeAp1h1.equals("testState1") && typeAp1h2.equals("testState1")
+          && typeAp2h1.equals("testState2") && typeAp3h2.equals("testState3")
+          && typeBp1h2.equals("testState3") && typeBp1h4.equals("testState2"));
+    }, WAIT_DURATION);
+    Assert.assertTrue(isRoutingTableUpdatedProperly, "RoutingTable has been updated properly");
     routingTableProvider.shutdown();
   }
 
-
   private void validateRoutingTable(RoutingTableProvider routingTableProvider,
       Set<String> masterNodes, Set<String> slaveNodes) {
     IdealState is =