[GitHub] [hbase] Apache9 commented on a diff in pull request #4865: HBASE-27469 IllegalArgumentException is thrown by SnapshotScannerHDFS…

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

Apache9 commented on code in PR #4865:
URL: https://github.com/apache/hbase/pull/4865#discussion_r1015534952


##########
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclController.java:
##########
@@ -610,7 +609,8 @@ private Set<String> filterUsersToRemoveNsAccessAcl(Table aclTable, TableName tab
           PermissionStorage.isGlobalEntry(entry)
             || (PermissionStorage.isNamespaceEntry(entry)
               && Bytes.equals(PermissionStorage.fromNamespaceEntry(entry), namespace))
-            || (!Bytes.equals(tableName.getName(), entry)
+            || (PermissionStorage.isTableEntry(entry)

Review Comment:
   So this is the problem we want to fix? Mind explaining more?



##########
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/SnapshotScannerHDFSAclController.java:
##########
@@ -132,8 +132,7 @@ public void preMasterInitialization(ObserverContext<MasterCoprocessorEnvironment
       userProvider = UserProvider.instantiate(c.getEnvironment().getConfiguration());
     } else {
       LOG.warn("Try to initialize the coprocessor SnapshotScannerHDFSAclController but failure "
-        + "because the config " + SnapshotScannerHDFSAclHelper.ACL_SYNC_TO_HDFS_ENABLE
-        + " is false.");
+        + "because the config {} is false.", SnapshotScannerHDFSAclHelper.ACL_SYNC_TO_HDFS_ENABLE);

Review Comment:
   Since SnapshotScannerHDFSAclHelper.ACL_SYNC_TO_HDFS_ENABLE is a constant, I think the old way can have better performance?



-- 
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.

To unsubscribe, e-mail: issues-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org