helix-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From h...@apache.org
Subject [helix] branch master updated: Fix PreFetch annotation in ZkClient and CallbackHandler (#1332)
Date Tue, 01 Sep 2020 02:03:38 GMT
This is an automated email from the ASF dual-hosted git repository.

hzlu 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 8320f91  Fix PreFetch annotation in ZkClient and CallbackHandler (#1332)
8320f91 is described below

commit 8320f916fa60a58dbc545c84f42eaad688ce6047
Author: Huizhi Lu <ihuizhi.lu@gmail.com>
AuthorDate: Mon Aug 31 19:03:26 2020 -0700

    Fix PreFetch annotation in ZkClient and CallbackHandler (#1332)
    
    PreFetch annotation in class CallbackHandler is using org.apache.helix.api.listeners.PreFetch.
    So ZkClient still prefetches the data for callback handler. Because in ZkClient, the annotation
org.apache.helix.zookeeper.zkclient.annotation.PreFetch is used.
    
    This commit also changes the existing annotation name in zookeeper-api PreFetch to PreFetchChangedData.
---
 .../org/apache/helix/api/listeners/PreFetch.java   |   4 +-
 .../apache/helix/manager/zk/CallbackHandler.java   |   5 +-
 .../apache/helix/zookeeper/zkclient/ZkClient.java  |   6 +-
 .../{PreFetch.java => PreFetchChangedData.java}    |  21 +++-
 .../zkclient/TestPrefetchChangedData.java          | 109 +++++++++++++++++++++
 5 files changed, 135 insertions(+), 10 deletions(-)

diff --git a/helix-core/src/main/java/org/apache/helix/api/listeners/PreFetch.java b/helix-core/src/main/java/org/apache/helix/api/listeners/PreFetch.java
index ff24cec..9c46652 100644
--- a/helix-core/src/main/java/org/apache/helix/api/listeners/PreFetch.java
+++ b/helix-core/src/main/java/org/apache/helix/api/listeners/PreFetch.java
@@ -23,9 +23,9 @@ import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 
 /**
- * This annotation has been deprecated. Use PreFetch in zookeeper-api module instead.
+ * An annotation used to prefetch data for listeners (eg. LiveInstanceChangeListener)
+ * in callback handler.
  */
-@Deprecated
 @Retention(RetentionPolicy.RUNTIME)
 public @interface PreFetch {
   boolean enabled() default true;
diff --git a/helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java b/helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java
index 0b41bb6..5ec7e91 100644
--- a/helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java
+++ b/helix-core/src/main/java/org/apache/helix/manager/zk/CallbackHandler.java
@@ -74,11 +74,11 @@ import org.apache.helix.model.Message;
 import org.apache.helix.model.ResourceConfig;
 import org.apache.helix.monitoring.mbeans.HelixCallbackMonitor;
 import org.apache.helix.zookeeper.api.client.ChildrenSubscribeResult;
-import org.apache.helix.zookeeper.api.client.HelixZkClient;
 import org.apache.helix.zookeeper.api.client.RealmAwareZkClient;
 import org.apache.helix.zookeeper.datamodel.ZNRecord;
 import org.apache.helix.zookeeper.zkclient.IZkChildListener;
 import org.apache.helix.zookeeper.zkclient.IZkDataListener;
+import org.apache.helix.zookeeper.zkclient.annotation.PreFetchChangedData;
 import org.apache.helix.zookeeper.zkclient.exception.ZkNoNodeException;
 import org.apache.zookeeper.Watcher.Event.EventType;
 import org.slf4j.Logger;
@@ -102,8 +102,7 @@ import static org.apache.helix.HelixConstants.ChangeType.MESSAGES_CONTROLLER;
 import static org.apache.helix.HelixConstants.ChangeType.RESOURCE_CONFIG;
 import static org.apache.helix.HelixConstants.ChangeType.TARGET_EXTERNAL_VIEW;
 
-
-@PreFetch(enabled = false)
+@PreFetchChangedData(enabled = false)
 public class CallbackHandler implements IZkChildListener, IZkDataListener {
   private static Logger logger = LoggerFactory.getLogger(CallbackHandler.class);
 
diff --git a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
index a926a8d..40332a3 100644
--- a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
+++ b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/ZkClient.java
@@ -38,7 +38,7 @@ import org.apache.helix.zookeeper.api.client.ChildrenSubscribeResult;
 import org.apache.helix.zookeeper.constant.ZkSystemPropertyKeys;
 import org.apache.helix.zookeeper.datamodel.ZNRecord;
 import org.apache.helix.zookeeper.exception.ZkClientException;
-import org.apache.helix.zookeeper.zkclient.annotation.PreFetch;
+import org.apache.helix.zookeeper.zkclient.annotation.PreFetchChangedData;
 import org.apache.helix.zookeeper.zkclient.callback.ZkAsyncCallMonitorContext;
 import org.apache.helix.zookeeper.zkclient.callback.ZkAsyncCallbacks;
 import org.apache.helix.zookeeper.zkclient.callback.ZkAsyncRetryCallContext;
@@ -319,7 +319,7 @@ public class ZkClient implements Watcher {
   }
 
   private boolean isPrefetchEnabled(IZkDataListener dataListener) {
-    PreFetch preFetch = dataListener.getClass().getAnnotation(PreFetch.class);
+    PreFetchChangedData preFetch = dataListener.getClass().getAnnotation(PreFetchChangedData.class);
     if (preFetch != null) {
       return preFetch.enabled();
     }
@@ -328,7 +328,7 @@ public class ZkClient implements Watcher {
     try {
       Method method = dataListener.getClass()
           .getMethod(callbackMethod.getName(), callbackMethod.getParameterTypes());
-      PreFetch preFetchInMethod = method.getAnnotation(PreFetch.class);
+      PreFetchChangedData preFetchInMethod = method.getAnnotation(PreFetchChangedData.class);
       if (preFetchInMethod != null) {
         return preFetchInMethod.enabled();
       }
diff --git a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/annotation/PreFetch.java
b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/annotation/PreFetchChangedData.java
similarity index 54%
rename from zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/annotation/PreFetch.java
rename to zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/annotation/PreFetchChangedData.java
index 3bfe6b7..e6db00e 100644
--- a/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/annotation/PreFetch.java
+++ b/zookeeper-api/src/main/java/org/apache/helix/zookeeper/zkclient/annotation/PreFetchChangedData.java
@@ -22,8 +22,25 @@ package org.apache.helix.zookeeper.zkclient.annotation;
 import java.lang.annotation.Retention;
 import java.lang.annotation.RetentionPolicy;
 
-
+/**
+ * An annotation used to prefetch changed data for
+ * {@link org.apache.helix.zookeeper.zkclient.IZkDataListener} in ZkClient.
+ * By default, prefetch is enabled: when ZkClient handles a data change event,
+ * ZkClient will read data and pass data object to
+ * {@link org.apache.helix.zookeeper.zkclient.IZkDataListener#handleDataChange(String, Object)}.
+ * If disabled({@code false}): ZkClient will not read data, so data object is passed as null.
+ * <p>
+ * Example:
+ * <pre>
+ * {@code @PreFetch(enabled = false)}
+ *  public class MyCallback implements IZkDataListener
+ * </pre>
+ *
+ * {@code @PreFetch(enabled = false)} means data will not be prefetched in ZkClient before
+ * handling data change: data is null in
+ * {@link org.apache.helix.zookeeper.zkclient.IZkDataListener#handleDataChange(String, Object)}
+ */
 @Retention(RetentionPolicy.RUNTIME)
-public @interface PreFetch {
+public @interface PreFetchChangedData {
   boolean enabled() default true;
 }
diff --git a/zookeeper-api/src/test/java/org/apache/helix/zookeeper/zkclient/TestPrefetchChangedData.java
b/zookeeper-api/src/test/java/org/apache/helix/zookeeper/zkclient/TestPrefetchChangedData.java
new file mode 100644
index 0000000..2c2f8a2
--- /dev/null
+++ b/zookeeper-api/src/test/java/org/apache/helix/zookeeper/zkclient/TestPrefetchChangedData.java
@@ -0,0 +1,109 @@
+package org.apache.helix.zookeeper.zkclient;
+
+/*
+ * 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.concurrent.CountDownLatch;
+import java.util.concurrent.TimeUnit;
+
+import org.apache.helix.zookeeper.impl.TestHelper;
+import org.apache.helix.zookeeper.impl.ZkTestBase;
+import org.apache.helix.zookeeper.impl.client.ZkClient;
+import org.apache.helix.zookeeper.zkclient.annotation.PreFetchChangedData;
+import org.testng.Assert;
+import org.testng.annotations.Test;
+
+public class TestPrefetchChangedData extends ZkTestBase {
+  @Test
+  public void testPrefetchChangedDataEnabled() throws InterruptedException {
+    String path = "/" + TestHelper.getTestMethodName();
+    ZkClient zkClient = new ZkClient(ZkTestBase.ZK_ADDR);
+
+    try {
+      zkClient.createPersistent(path, "v1");
+
+      CountDownLatch countDownLatch = new CountDownLatch(1);
+      PreFetchZkDataListener dataListener = new PreFetchZkDataListener(countDownLatch);
+      zkClient.subscribeDataChanges(path, dataListener);
+      zkClient.writeData(path, "v2");
+
+      Assert.assertTrue(countDownLatch.await(3L, TimeUnit.SECONDS));
+
+      Assert.assertTrue(dataListener.isDataPreFetched());
+    } finally {
+      zkClient.unsubscribeAll();
+      zkClient.delete(path);
+      zkClient.close();
+    }
+  }
+
+  @Test
+  public void testPrefetchChangedDataDisabled() throws InterruptedException {
+    String path = "/" + TestHelper.getTestMethodName();
+    ZkClient zkClient = new ZkClient(ZkTestBase.ZK_ADDR);
+
+    try {
+      zkClient.createPersistent(path, "v1");
+
+      CountDownLatch countDownLatch = new CountDownLatch(1);
+      PreFetchZkDataListener dataListener = new PreFetchDisabledZkDataListener(countDownLatch);
+      zkClient.subscribeDataChanges(path, dataListener);
+      zkClient.writeData(path, "v2");
+
+      Assert.assertTrue(countDownLatch.await(3L, TimeUnit.SECONDS));
+
+      Assert.assertFalse(dataListener.isDataPreFetched());
+    } finally {
+      zkClient.unsubscribeAll();
+      zkClient.delete(path);
+      zkClient.close();
+    }
+  }
+
+  private static class PreFetchZkDataListener implements IZkDataListener {
+    private boolean isDataPreFetched;
+    private CountDownLatch countDownLatch;
+
+    public PreFetchZkDataListener(CountDownLatch countDownLatch) {
+      this.countDownLatch = countDownLatch;
+    }
+
+    @Override
+    public void handleDataChange(String dataPath, Object data) throws Exception {
+      isDataPreFetched = (data != null);
+      countDownLatch.countDown();
+    }
+
+    @Override
+    public void handleDataDeleted(String dataPath) throws Exception {
+      // Not implemented
+    }
+
+    public boolean isDataPreFetched() {
+      return isDataPreFetched;
+    }
+  }
+
+  @PreFetchChangedData(enabled = false)
+  private static class PreFetchDisabledZkDataListener extends PreFetchZkDataListener {
+    public PreFetchDisabledZkDataListener(CountDownLatch countDownLatch) {
+      super(countDownLatch);
+    }
+  }
+}


Mime
View raw message