[helix] branch master updated: Use ZnRecordSerializer in ConfigAccessor (#666)

[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 d6ba1f3  Use ZnRecordSerializer in ConfigAccessor (#666)
d6ba1f3 is described below

commit d6ba1f3e792651e250d60b3df1fc6b537934f69e
Author: Hunter Lee <hu...@linkedin.com>
AuthorDate: Thu Dec 19 14:22:56 2019 -0800

    Use ZnRecordSerializer in ConfigAccessor (#666)
    
    ConfigAccessor is a Helix API used for CRUD access of Helix's data model such as ResourceConfig, InstanceConfig, etc.. These records are represented as ZNRecord and therefore needs to be serialized and deserialized with ZnRecordSerializer. This diff fixes ConfigAccessor so that it explicitly uses ZnRecordSerializer by default.
---
 .../src/main/java/org/apache/helix/ConfigAccessor.java     | 14 +++++++++-----
 .../src/test/java/org/apache/helix/TestConfigAccessor.java | 10 ++++++++++
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java b/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java
index 96fc9ec..018743d 100644
--- a/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java
+++ b/helix-core/src/main/java/org/apache/helix/ConfigAccessor.java
@@ -28,6 +28,7 @@ import java.util.Map;
 import java.util.TreeMap;
 
 import org.apache.helix.manager.zk.ZKUtil;
+import org.apache.helix.manager.zk.ZNRecordSerializer;
 import org.apache.helix.manager.zk.client.HelixZkClient;
 import org.apache.helix.manager.zk.client.SharedZkClientFactory;
 import org.apache.helix.model.ClusterConfig;
@@ -75,7 +76,8 @@ public class ConfigAccessor {
 
   /**
    * Initialize an accessor with a Zookeeper client
-   * Note: it is recommended to use the other constructor instead to avoid having to create a HelixZkClient.
+   * Note: it is recommended to use the other constructor instead to avoid having to create a
+   * HelixZkClient.
    * @param zkClient
    */
   @Deprecated
@@ -85,13 +87,15 @@ public class ConfigAccessor {
   }
 
   /**
-   * Initialize a ConfigAccessor with a ZooKeeper connect string. It will use a SharedZkClient with default settings.
+   * Initialize a ConfigAccessor with a ZooKeeper connect string. It will use a SharedZkClient with
+   * default settings. Note that ZNRecordSerializer will be used for the internal ZkClient since
+   * ConfigAccessor only deals with Helix's data models like ResourceConfig.
    * @param zkAddress
    */
   public ConfigAccessor(String zkAddress) {
-    _zkClient = SharedZkClientFactory.getInstance()
-        .buildZkClient(new HelixZkClient.ZkConnectionConfig(zkAddress),
-            new HelixZkClient.ZkClientConfig());
+    _zkClient = SharedZkClientFactory.getInstance().buildZkClient(
+        new HelixZkClient.ZkConnectionConfig(zkAddress),
+        new HelixZkClient.ZkClientConfig().setZkSerializer(new ZNRecordSerializer()));
     _usesExternalZkClient = false;
   }
 
diff --git a/helix-core/src/test/java/org/apache/helix/TestConfigAccessor.java b/helix-core/src/test/java/org/apache/helix/TestConfigAccessor.java
index 9d1c069..0fa05c3 100644
--- a/helix-core/src/test/java/org/apache/helix/TestConfigAccessor.java
+++ b/helix-core/src/test/java/org/apache/helix/TestConfigAccessor.java
@@ -43,15 +43,22 @@ public class TestConfigAccessor extends ZkUnitTestBase {
         "MasterSlave", true);
 
     ConfigAccessor configAccessor = new ConfigAccessor(_gZkClient);
+    ConfigAccessor configAccessorZkAddr = new ConfigAccessor(ZK_ADDR);
     ConfigScope clusterScope = new ConfigScopeBuilder().forCluster(clusterName).build();
 
     // cluster scope config
     String clusterConfigValue = configAccessor.get(clusterScope, "clusterConfigKey");
     Assert.assertNull(clusterConfigValue);
+    // also test with ConfigAccessor created with ZkAddr
+    clusterConfigValue = configAccessorZkAddr.get(clusterScope, "clusterConfigKey");
+    Assert.assertNull(clusterConfigValue);
 
     configAccessor.set(clusterScope, "clusterConfigKey", "clusterConfigValue");
     clusterConfigValue = configAccessor.get(clusterScope, "clusterConfigKey");
     Assert.assertEquals(clusterConfigValue, "clusterConfigValue");
+    configAccessorZkAddr.set(clusterScope, "clusterConfigKey", "clusterConfigValue");
+    clusterConfigValue = configAccessorZkAddr.get(clusterScope, "clusterConfigKey");
+    Assert.assertEquals(clusterConfigValue, "clusterConfigValue");
 
     // resource scope config
     ConfigScope resourceScope =
@@ -153,6 +160,8 @@ public class TestConfigAccessor extends ZkUnitTestBase {
 
     TestHelper.dropCluster(clusterName, _gZkClient);
 
+    configAccessor.close();
+    configAccessorZkAddr.close();
     System.out.println("END " + clusterName + " at " + new Date(System.currentTimeMillis()));
 
   }
@@ -192,6 +201,7 @@ public class TestConfigAccessor extends ZkUnitTestBase {
     Assert.assertEquals(participantConfigValue, "participantConfigValue");
 
     admin.dropCluster(clusterName);
+    configAccessor.close();
     System.out.println("END " + clusterName + " at " + new Date(System.currentTimeMillis()));
   }
 }