[GitHub] [hbase] virajjasani commented on a change in pull request #1546: HBASE-24211: Create table is slow in large cluster when AccessController is enabled.

[GitBox <gi...@apache.org>]

virajjasani commented on a change in pull request #1546: HBASE-24211: Create table is slow in large cluster when AccessController is enabled.
URL: https://github.com/apache/hbase/pull/1546#discussion_r410693565
 
 

 ##########
 File path: hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/ZKPermissionWatcher.java
 ##########
 @@ -185,25 +185,27 @@ public void run() {
   public void nodeChildrenChanged(final String path) {
     waitUntilStarted();
     if (path.equals(aclZNode)) {
-      try {
-        final List<ZKUtil.NodeAndData> nodeList =
-            ZKUtil.getChildDataAndWatchForNewChildren(watcher, aclZNode);
-        // preempt any existing nodeChildrenChanged event processing
-        if (childrenChangedFuture != null && !childrenChangedFuture.isDone()) {
-          boolean cancelled = childrenChangedFuture.cancel(true);
-          if (!cancelled) {
-            // task may have finished between our check and attempted cancel, this is fine.
-            if (! childrenChangedFuture.isDone()) {
-              LOG.warn("Could not cancel processing node children changed event, " +
-                  "please file a JIRA and attach logs if possible.");
-            }
+      // preempt any existing nodeChildrenChanged event processing
+      if (childrenChangedFuture != null && !childrenChangedFuture.isDone()) {
+        boolean cancelled = childrenChangedFuture.cancel(true);
+        if (!cancelled) {
+          // task may have finished between our check and attempted cancel, this is fine.
+          if (!childrenChangedFuture.isDone()) {
+            LOG.warn("Could not cancel processing node children changed event, "
+              + "please file a JIRA and attach logs if possible.");
           }
         }
-        childrenChangedFuture = asyncProcessNodeUpdate(() -> refreshNodes(nodeList));
-      } catch (KeeperException ke) {
-        LOG.error("Error reading data from zookeeper for path "+path, ke);
-        watcher.abort("ZooKeeper error get node children for path "+path, ke);
       }
+      childrenChangedFuture = asyncProcessNodeUpdate(() -> {
+        try {
+          final List<ZKUtil.NodeAndData> nodeList =
+            ZKUtil.getChildDataAndWatchForNewChildren(watcher, aclZNode);
+          refreshNodes(nodeList);
+        } catch (KeeperException ke) {
+          LOG.error("Error reading data from zookeeper for path " + path, ke);
 
 Review comment:
   Or better create a string variable and use it for both LOG.error and abort message.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services