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 <ihuizhi.lu@gmail.com>
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);
}
}
|