HBASE-30058 Eliminate unnecessary connection creation in snapshot operations

[mini666]

Summary

Each snapshot operation created two short-lived connections in SnapshotDescriptionUtils.validate():

1. isSecurityAvailable(conf) — created a Connection + Admin just to check hbase:acl table existence
2. writeAclToSnapshotDescription() — created another Connection to read ACL data from hbase:acl

In Kerberos environments with the default ZKConnectionRegistry, each connection triggered a new ZK session with GSSAPI authentication and a TGS request to the KDC. During batch snapshot operations, this caused excessive KDC load that could lead to IP blocking.

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

Zero behavioral change — the same checks are performed, the same data is read, the same ACL is written to snapshots.

See HBASE-30058 for detailed root cause analysis including design ambiguity discussion.

[junegunn]

Two suggestions:

Keep the old method signatures and overload

Could we add overloads that take Connection rather than changing existing signatures? It would minimize code changes and keep the existing API working. For example:

@Deprecated
public static SnapshotDescription validate(SnapshotDescription snapshot, Configuration conf)
    throws IOException {
  try (Connection conn = ConnectionFactory.createConnection(conf)) {
    return validate(conn, snapshot, conf);
  }
}

public static SnapshotDescription validate(Connection conn, SnapshotDescription snapshot,
    Configuration conf) throws IOException {
  // ... new implementation
}

Same idea for isSecurityAvailable.

Commit message

Please drop the Signed-off-by: JeongMin Ju <mini666@apache.org> line from the commit message. In the HBase project, Signed-off-by is used to attribute reviewers of the PR, not the author.

[mini666]

@junegunn Thanks for the review! Updated:

1. Overloads: Kept the original validate(SnapshotDescription, Configuration) and isSecurityAvailable(Configuration) signatures as @Deprecated overloads that delegate to the new Connection-based methods. Reverted the test file changes since they no longer need the null Connection workaround.
2. Commit message: Dropped the Signed-off-by line.

Diff is now down to 5 production files (no test changes). Please take another look when you get a chance.

[junegunn]

Do we need to deprecate the original ones? We're still using them in multiple places. I think it's enough to mention that the new ones are for avoiding connection overhead.

[mini666]

@junegunn Good point — agreed. Dropped @Deprecated from the original validate(SnapshotDescription, Configuration) and isSecurityAvailable(Configuration) overloads, and added a Javadoc note on the new Connection-based overloads explaining they're preferred when the caller already holds a Connection (to avoid the per-call connection setup cost). Updated the commit message accordingly.

[junegunn]

The existing one without t can call this:

  public static ListMultimap<String, UserPermission> getTablePermissions(Configuration conf,
    TableName tableName) throws IOException {
    return getTablePermissions(conf, tableName, null);
  }

[mini666]

Good catch — done. The 2-arg overload now delegates to the 3-arg one with t=null to avoid duplicating the getPermissions(...) call.