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 b0b9911 Reduce unnecessary getChildren call in subscribeForChanges
(#1344)
b0b9911 is described below
commit b0b9911d803227c2595d19a0c1d894f996848ce6
Author: Huizhi Lu <[email protected]>
AuthorDate: Thu Sep 24 23:40:41 2020 -0700
Reduce unnecessary getChildren call in subscribeForChanges (#1344)
In CallbackHanlder's subscribeForChanges(), children names need to be
fetched. subscribeChildChange() does actually have the children list, but it
doesn't return, and later getChildren() is called again.
It wastes one zk call: not only wasting time in handling a callback, but
also adds more read pressure to zk server.
This commit uses the return result from subscribeChildChange() and saves
one getChildren() call.
---
.../apache/helix/manager/zk/CallbackHandler.java | 39 +++++++++++++---------
1 file changed, 23 insertions(+), 16 deletions(-)
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 2e8726d..dece3de 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
@@ -538,7 +538,12 @@ public class CallbackHandler implements IZkChildListener,
IZkDataListener {
}
}
- private void subscribeChildChange(String path, NotificationContext.Type
callbackType) {
+ /*
+ * If callback type is INIT or CALLBACK, subscribes child change listener to
the path
+ * and returns the path's children names. The children list might be null
when the path
+ * doesn't exist or callback type is INIT/CALLBACK.
+ */
+ private List<String> subscribeChildChange(String path,
NotificationContext.Type callbackType) {
if (callbackType == NotificationContext.Type.INIT
|| callbackType == NotificationContext.Type.CALLBACK) {
if (logger.isDebugEnabled()) {
@@ -551,20 +556,26 @@ public class CallbackHandler implements IZkChildListener,
IZkDataListener {
// Later, CALLBACK type, the CallbackHandler already registered the
watch and knows the
// path was created. Here, to avoid leaking path in ZooKeeper server, we
would not let
// CallbackHandler to install exists watch, namely watch for path not
existing.
- // Note when path is removed, the CallbackHanler would remove itself
from ZkHelixManager too
+ // Note when path is removed, the CallbackHandler would remove itself
from ZkHelixManager too
// to avoid leaking a CallbackHandler.
- ChildrenSubscribeResult childrenSubscribeResult =
_zkClient.subscribeChildChanges(path, this, callbackType != Type.INIT);
+ ChildrenSubscribeResult childrenSubscribeResult =
+ _zkClient.subscribeChildChanges(path, this, callbackType !=
Type.INIT);
logger.debug("CallbackHandler {} subscribe data path {} result {}",
_uid, path,
childrenSubscribeResult.isInstalled());
if (!childrenSubscribeResult.isInstalled()) {
logger.info("CallbackHandler {} subscribe data path {} failed!", _uid,
path);
}
+ // getChildren() might be null: when path doesn't exist.
+ return childrenSubscribeResult.getChildren();
} else if (callbackType == NotificationContext.Type.FINALIZE) {
logger.info("CallbackHandler{}, {} unsubscribe child-change. path: {},
listener: {}",
_uid ,_manager.getInstanceName(), path, _listener);
_zkClient.unsubscribeChildChanges(path, this);
}
+
+ // List of children could be empty, but won't be null.
+ return _zkClient.getChildren(path);
}
private void subscribeDataChange(String path, NotificationContext.Type
callbackType) {
@@ -597,23 +608,21 @@ public class CallbackHandler implements IZkChildListener,
IZkDataListener {
private void subscribeForChanges(NotificationContext.Type callbackType,
String path,
boolean watchChild) {
- logger.info("CallbackHandler {} Subscribing changes listener to path: {},
type: {}, listener: {}",
- _uid, path, callbackType, _listener);
+ logger.info("CallbackHandler {} subscribing changes listener to path: {},
callback type: {}, "
+ + "event types: {}, listener: {}, watchChild: {}",
+ _uid, path, callbackType, _eventTypes, _listener, watchChild);
long start = System.currentTimeMillis();
if (_eventTypes.contains(EventType.NodeDataChanged)
|| _eventTypes.contains(EventType.NodeCreated)
|| _eventTypes.contains(EventType.NodeDeleted)) {
- logger.info("CallbackHandler{} Subscribing data change listener to path:
{}", _uid, path);
+ logger.info("CallbackHandler {} subscribing data change listener to
path: {}", _uid, path);
subscribeDataChange(path, callbackType);
}
if (_eventTypes.contains(EventType.NodeChildrenChanged)) {
- logger.info("CallbackHandler{}, Subscribing child change listener to
path: {}", _uid, path);
- subscribeChildChange(path, callbackType);
+ List<String> children = subscribeChildChange(path, callbackType);
if (watchChild) {
- logger.info("CallbackHandler{}, Subscribing data change listener to
all children for path: {}", _uid, path);
-
try {
switch (_changeType) {
case CURRENT_STATE:
@@ -633,11 +642,10 @@ public class CallbackHandler implements IZkChildListener,
IZkDataListener {
if (bucketSize > 0) {
// subscribe both data-change and child-change on bucketized
parent node
// data-change gives a delete-callback which is used to
remove watch
- subscribeChildChange(childPath, callbackType);
+ List<String> bucketizedChildNames =
subscribeChildChange(childPath, callbackType);
subscribeDataChange(childPath, callbackType);
// subscribe data-change on bucketized child
- List<String> bucketizedChildNames =
_zkClient.getChildren(childPath);
if (bucketizedChildNames != null) {
for (String bucketizedChildName : bucketizedChildNames) {
String bucketizedChildPath = childPath + "/" +
bucketizedChildName;
@@ -651,10 +659,9 @@ public class CallbackHandler implements IZkChildListener,
IZkDataListener {
break;
}
default: {
- List<String> childNames = _zkClient.getChildren(path);
- if (childNames != null) {
- for (String childName : childNames) {
- String childPath = path + "/" + childName;
+ if (children != null) {
+ for (String child : children) {
+ String childPath = path + "/" + child;
subscribeDataChange(childPath, callbackType);
}
}
