You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@helix.apache.org by hz...@apache.org on 2020/09/01 02:03:38 UTC
[helix] branch master updated: Fix PreFetch annotation in ZkClient
and CallbackHandler (#1332)
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 <ih...@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);
+ }
+ }
+}