This is an automated email from the ASF dual-hosted git repository.
junegunn pushed a commit to branch branch-3
in repository https://gitbox.apache.org/repos/asf/hbase.git
The following commit(s) were added to refs/heads/branch-3 by this push:
new d3379ccaaab HBASE-30058 Eliminate unnecessary connection creation in
snapshot operations (#8030) (#8312)
d3379ccaaab is described below
commit d3379ccaaabb0af0e2ec1366fe89c1e568cc4763
Author: Junegunn Choi <[email protected]>
AuthorDate: Sat Jun 6 09:20:39 2026 +0900
HBASE-30058 Eliminate unnecessary connection creation in snapshot
operations (#8030) (#8312)
Each snapshot operation created two short-lived connections in
SnapshotDescriptionUtils.validate() — one for isSecurityAvailable()
to check hbase:acl table existence, and another in
writeAclToSnapshotDescription() to read ACL data. In Kerberos
environments with ZKConnectionRegistry, each connection triggered
a new ZK session with GSSAPI authentication and a TGS request to
the KDC, causing excessive KDC load during batch snapshot operations.
This patch reuses the caller's existing connection instead of
creating new ones:
- isSecurityAvailable() now accepts a Connection parameter
- writeAclToSnapshotDescription() passes the shared connection's
Table to PermissionStorage.getTablePermissions()
- All callers (MasterRpcServices, RestoreSnapshotProcedure,
CloneSnapshotProcedure) pass through their available connection
The original validate(SnapshotDescription, Configuration) and
isSecurityAvailable(Configuration) signatures are preserved as
overloads that delegate to the new Connection-based methods, so
existing callers (including tests) keep working unchanged. The
new Connection-based overloads are documented as preferred for
callers that already hold a Connection, to avoid the per-call
connection setup cost.
Signed-off-by: Junegunn Choi <[email protected]>
Co-authored-by: Jeongmin Ju <[email protected]>
---
.../hadoop/hbase/master/MasterRpcServices.java | 4 +-
.../master/procedure/CloneSnapshotProcedure.java | 2 +-
.../master/procedure/RestoreSnapshotProcedure.java | 2 +-
.../hbase/security/access/PermissionStorage.java | 9 +++-
.../hbase/snapshot/SnapshotDescriptionUtils.java | 55 ++++++++++++++++++----
5 files changed, 58 insertions(+), 14 deletions(-)
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
index 08b6587047c..790ff28d28e 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterRpcServices.java
@@ -1788,8 +1788,8 @@ public class MasterRpcServices extends
HBaseRpcServicesBase<HMaster>
LOG.info(server.getClientIdAuditPrefix() + " snapshot request for:"
+ ClientSnapshotDescriptionUtils.toString(request.getSnapshot()));
// get the snapshot information
- SnapshotDescription snapshot =
- SnapshotDescriptionUtils.validate(request.getSnapshot(),
server.getConfiguration());
+ SnapshotDescription snapshot =
SnapshotDescriptionUtils.validate(server.getConnection(),
+ request.getSnapshot(), server.getConfiguration());
// send back the max amount of time the client should wait for the
snapshot to complete
long waitTime =
SnapshotDescriptionUtils.getMaxMasterTimeout(server.getConfiguration(),
snapshot.getType(), SnapshotDescriptionUtils.DEFAULT_MAX_WAIT_TIME);
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java
index 19f5d9db41d..3799dd1daf0 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/CloneSnapshotProcedure.java
@@ -133,7 +133,7 @@ public class CloneSnapshotProcedure extends
AbstractStateMachineTableProcedure<C
Configuration conf = env.getMasterServices().getConfiguration();
if (
restoreAcl && snapshot.hasUsersAndPermissions() &&
snapshot.getUsersAndPermissions() != null
- && SnapshotDescriptionUtils.isSecurityAvailable(conf)
+ &&
SnapshotDescriptionUtils.isSecurityAvailable(env.getMasterServices().getConnection())
) {
RestoreSnapshotHelper.restoreSnapshotAcl(snapshot,
tableDescriptor.getTableName(), conf);
}
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java
index e16b3374106..bb7a5582f4d 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/RestoreSnapshotProcedure.java
@@ -556,7 +556,7 @@ public class RestoreSnapshotProcedure
private void restoreSnapshotAcl(final MasterProcedureEnv env) throws
IOException {
if (
restoreAcl && snapshot.hasUsersAndPermissions() &&
snapshot.getUsersAndPermissions() != null
- &&
SnapshotDescriptionUtils.isSecurityAvailable(env.getMasterServices().getConfiguration())
+ &&
SnapshotDescriptionUtils.isSecurityAvailable(env.getMasterServices().getConnection())
) {
// restore acl of snapshot to table.
RestoreSnapshotHelper.restoreSnapshotAcl(snapshot,
TableName.valueOf(snapshot.getTable()),
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/PermissionStorage.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/PermissionStorage.java
index b66c0ed0b09..5fa1e114aad 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/PermissionStorage.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/PermissionStorage.java
@@ -482,8 +482,13 @@ public final class PermissionStorage {
public static ListMultimap<String, UserPermission>
getTablePermissions(Configuration conf,
TableName tableName) throws IOException {
- return getPermissions(conf, tableName != null ? tableName.getName() :
null, null, null, null,
- null, false);
+ return getTablePermissions(conf, tableName, null);
+ }
+
+ public static ListMultimap<String, UserPermission>
getTablePermissions(Configuration conf,
+ TableName tableName, Table t) throws IOException {
+ return getPermissions(conf, tableName != null ? tableName.getName() :
null, t, null, null, null,
+ false);
}
public static ListMultimap<String, UserPermission>
getNamespacePermissions(Configuration conf,
diff --git
a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java
index 689cd89259b..7269ba65b4d 100644
---
a/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java
+++
b/hbase-server/src/main/java/org/apache/hadoop/hbase/snapshot/SnapshotDescriptionUtils.java
@@ -34,6 +34,7 @@ import org.apache.hadoop.hbase.TableName;
import org.apache.hadoop.hbase.client.Admin;
import org.apache.hadoop.hbase.client.Connection;
import org.apache.hadoop.hbase.client.ConnectionFactory;
+import org.apache.hadoop.hbase.client.Table;
import org.apache.hadoop.hbase.ipc.RpcServer;
import org.apache.hadoop.hbase.security.User;
import org.apache.hadoop.hbase.security.access.AccessChecker;
@@ -302,10 +303,36 @@ public final class SnapshotDescriptionUtils {
*/
public static SnapshotDescription validate(SnapshotDescription snapshot,
Configuration conf)
throws IllegalArgumentException, IOException {
+ requireHasTable(snapshot);
+ try (Connection conn = ConnectionFactory.createConnection(conf)) {
+ return validate(conn, snapshot, conf);
+ }
+ }
+
+ private static void requireHasTable(SnapshotDescription snapshot) {
if (!snapshot.hasTable()) {
throw new IllegalArgumentException(
"Descriptor doesn't apply to a table, so we can't build it.");
}
+ }
+
+ /**
+ * Convert the passed snapshot description into a 'full' snapshot
description based on default
+ * parameters, if none have been supplied. This resolves any 'optional'
parameters that aren't
+ * supplied to their default values. Prefer this overload over
+ * {@link #validate(SnapshotDescription, Configuration)} when the caller
already holds a
+ * {@link Connection}, to avoid the cost of creating a fresh connection
(which involves a ZK
+ * session and, in Kerberos environments, a TGS request to the KDC) for each
call.
+ * @param conn connection to use for reading ACL information when
security is enabled
+ * @param snapshot general snapshot descriptor
+ * @param conf Configuration to read configured snapshot defaults if
snapshot is not complete
+ * @return a valid snapshot description
+ * @throws IllegalArgumentException if the {@link SnapshotDescription} is
not a complete
+ * {@link SnapshotDescription}.
+ */
+ public static SnapshotDescription validate(Connection conn,
SnapshotDescription snapshot,
+ Configuration conf) throws IllegalArgumentException, IOException {
+ requireHasTable(snapshot);
SnapshotDescription.Builder builder = snapshot.toBuilder();
@@ -350,8 +377,8 @@ public final class SnapshotDescriptionUtils {
snapshot = builder.build();
// set the acl to snapshot if security feature is enabled.
- if (isSecurityAvailable(conf)) {
- snapshot = writeAclToSnapshotDescription(snapshot, conf);
+ if (isSecurityAvailable(conn)) {
+ snapshot = writeAclToSnapshotDescription(conn, snapshot, conf);
}
return snapshot;
}
@@ -475,20 +502,32 @@ public final class SnapshotDescriptionUtils {
}
public static boolean isSecurityAvailable(Configuration conf) throws
IOException {
- try (Connection conn = ConnectionFactory.createConnection(conf);
- Admin admin = conn.getAdmin()) {
+ try (Connection conn = ConnectionFactory.createConnection(conf)) {
+ return isSecurityAvailable(conn);
+ }
+ }
+
+ /**
+ * Prefer this overload over {@link #isSecurityAvailable(Configuration)}
when the caller already
+ * holds a {@link Connection}, to avoid the cost of creating a fresh
connection (which involves a
+ * ZK session and, in Kerberos environments, a TGS request to the KDC) for
each call.
+ */
+ public static boolean isSecurityAvailable(Connection conn) throws
IOException {
+ try (Admin admin = conn.getAdmin()) {
return admin.tableExists(PermissionStorage.ACL_TABLE_NAME);
}
}
- private static SnapshotDescription
writeAclToSnapshotDescription(SnapshotDescription snapshot,
- Configuration conf) throws IOException {
+ private static SnapshotDescription writeAclToSnapshotDescription(Connection
conn,
+ SnapshotDescription snapshot, Configuration conf) throws IOException {
ListMultimap<String, UserPermission> perms =
User.runAsLoginUser(new PrivilegedExceptionAction<ListMultimap<String,
UserPermission>>() {
@Override
public ListMultimap<String, UserPermission> run() throws Exception {
- return PermissionStorage.getTablePermissions(conf,
- TableName.valueOf(snapshot.getTable()));
+ try (Table aclTable =
conn.getTable(PermissionStorage.ACL_TABLE_NAME)) {
+ return PermissionStorage.getTablePermissions(conf,
+ TableName.valueOf(snapshot.getTable()), aclTable);
+ }
}
});
return snapshot.toBuilder()
