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: Reduce unnecessary getChildren call in subscribeForChanges (#1344)
Date Fri, 25 Sep 2020 06:44:27 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 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);
                 }
               }


Mime
View raw message