From 4d376744ae610c131e355a2818f06229f431ec7f Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Thu, 5 Mar 2026 20:53:41 +0530 Subject: [PATCH 01/16] HBASE-29958 Improve log messages --- .../main/java/org/apache/hadoop/hbase/TableName.java | 2 +- .../apache/hadoop/hbase/master/MasterFileSystem.java | 10 +++++----- .../java/org/apache/hadoop/hbase/util/FSUtils.java | 7 ++++--- 3 files changed, 10 insertions(+), 9 deletions(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java index 73008c7ad5fd..e86274bc8f26 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java @@ -92,7 +92,7 @@ public static TableName getDefaultNameOfMetaForReplica() { public static TableName initializeHbaseMetaTableName(Configuration conf) { String suffix_val = conf.get(HConstants.HBASE_META_TABLE_SUFFIX, HConstants.HBASE_META_TABLE_SUFFIX_DEFAULT_VALUE); - LOG.info("Meta table suffix value: {}", suffix_val); + LOG.info("Read Replica Cluster suffix value: {}", suffix_val); if (Strings.isNullOrEmpty(suffix_val)) { return valueOf(NamespaceDescriptor.SYSTEM_NAMESPACE_NAME_STR, "meta"); } else { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java index 1734f03a2738..63e65d163569 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java @@ -405,14 +405,14 @@ private void negotiateActiveClusterSuffixFile(long wait) throws IOException { this.activeClusterSuffix = acs; } else { // throw error - LOG.info("rootdir {} : Active Cluster File Suffix {} ", rootdir, acs); + LOG.info("rootdir {} : Read replica active cluster file suffix {} ", rootdir, acs); throw new IOException("Cannot start master, because another cluster is running in active " - + "(read-write) mode on this storage location. Active Cluster Id: {} " + acs - + " This cluster Id: " + getClusterId()); + + "(read-write) mode on this storage location. Active Cluster Id: " + acs + + ", This cluster Id: " + getClusterId()); } LOG.info( - "This is the active cluster on this storage location, " + "File Suffix {} : Suffix {} : ", - acs, getActiveClusterSuffix()); + "Read Replica Cluster: This is the active cluster on this storage location with cluster id: {}", + getClusterId()); } catch (FileNotFoundException fnfe) { // this is the active cluster, create active cluster suffix file if it does not exist FSUtils.setActiveClusterSuffix(fs, rootdir, computeAndSetSuffixFileDataToWrite(), wait); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java index adc5e39fdcd3..c59b1c5c9f31 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java @@ -642,14 +642,14 @@ public static ActiveClusterSuffix getActiveClusterSuffix(FileSystem fs, Path roo data = in.readUTF(); cs = new ActiveClusterSuffix(data); } catch (EOFException eof) { - LOG.warn("Active Cluster Suffix File {} is empty ", idPath); + LOG.warn("Read Replica Cluster id file {} is empty ", idPath); } finally { in.close(); } } return cs; } else { - throw new FileNotFoundException("Active Cluster Suffix File " + idPath + " not found"); + throw new FileNotFoundException("Read Replica Cluster id file " + idPath + " not found"); } } @@ -701,7 +701,8 @@ public static void setActiveClusterSuffix(final FileSystem fs, final Path rootdi final Path tempIdFile = new Path(tempDir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME); String fsuffix = new String(bdata, StandardCharsets.US_ASCII); - LOG.debug("Create Active cluster Suffix file [{}] with Suffix: {}", idFile, fsuffix); + LOG.debug("Read Replica Cluster id file [{}] present and contains cluster id: {}", idFile, + fsuffix); writeClusterInfo(fs, rootdir, idFile, tempIdFile, bdata, wait); } From 931587b162358551c37835d880b9171d3eca2345 Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Sat, 7 Mar 2026 16:08:09 +0530 Subject: [PATCH 02/16] Address review comments --- .../org/apache/hadoop/hbase/master/MasterFileSystem.java | 5 +++-- .../src/main/java/org/apache/hadoop/hbase/util/FSUtils.java | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java index 63e65d163569..fb5fae55e6e3 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java @@ -408,11 +408,12 @@ private void negotiateActiveClusterSuffixFile(long wait) throws IOException { LOG.info("rootdir {} : Read replica active cluster file suffix {} ", rootdir, acs); throw new IOException("Cannot start master, because another cluster is running in active " + "(read-write) mode on this storage location. Active Cluster Id: " + acs - + ", This cluster Id: " + getClusterId()); + + ", This cluster Id: " + + new String(getSuffixFileDataToWrite(), StandardCharsets.UTF_8)); } LOG.info( "Read Replica Cluster: This is the active cluster on this storage location with cluster id: {}", - getClusterId()); + new String(getSuffixFileDataToWrite(), StandardCharsets.UTF_8)); } catch (FileNotFoundException fnfe) { // this is the active cluster, create active cluster suffix file if it does not exist FSUtils.setActiveClusterSuffix(fs, rootdir, computeAndSetSuffixFileDataToWrite(), wait); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java index c59b1c5c9f31..6f850f50e9c4 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java @@ -701,7 +701,7 @@ public static void setActiveClusterSuffix(final FileSystem fs, final Path rootdi final Path tempIdFile = new Path(tempDir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME); String fsuffix = new String(bdata, StandardCharsets.US_ASCII); - LOG.debug("Read Replica Cluster id file [{}] present and contains cluster id: {}", idFile, + LOG.debug("Read Replica Cluster id file [{}] is present and contains cluster id: {}", idFile, fsuffix); writeClusterInfo(fs, rootdir, idFile, tempIdFile, bdata, wait); } From 76a64957abfd6c6d550c2170257f995ae4b04ae8 Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Mon, 9 Mar 2026 23:57:46 +0530 Subject: [PATCH 03/16] Update hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Co-authored-by: Kota-SH --- .../src/main/java/org/apache/hadoop/hbase/util/FSUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java index 6f850f50e9c4..b4f738d06ff1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java @@ -642,7 +642,7 @@ public static ActiveClusterSuffix getActiveClusterSuffix(FileSystem fs, Path roo data = in.readUTF(); cs = new ActiveClusterSuffix(data); } catch (EOFException eof) { - LOG.warn("Read Replica Cluster id file {} is empty ", idPath); + LOG.warn("[Read Replica Feature] Active Cluster id file {} is empty ", idPath); } finally { in.close(); } From 2e1c061309457a37a0f7ccde6122c4bc4f37fa0d Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Mon, 9 Mar 2026 23:58:29 +0530 Subject: [PATCH 04/16] Update hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java Co-authored-by: Kota-SH --- .../src/main/java/org/apache/hadoop/hbase/util/FSUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java index b4f738d06ff1..777eaa30c35c 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java @@ -649,7 +649,7 @@ public static ActiveClusterSuffix getActiveClusterSuffix(FileSystem fs, Path roo } return cs; } else { - throw new FileNotFoundException("Read Replica Cluster id file " + idPath + " not found"); + throw new FileNotFoundException("[Read-replica feature] Active Cluster Suffix File " + idPath + " not found"); } } From 3adfdddabd5363cd23766846e1af2ebe70002f03 Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Mon, 9 Mar 2026 23:59:18 +0530 Subject: [PATCH 05/16] Update hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java Co-authored-by: Kota-SH --- .../java/org/apache/hadoop/hbase/master/MasterFileSystem.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java index fb5fae55e6e3..9bae027db925 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java @@ -412,7 +412,7 @@ private void negotiateActiveClusterSuffixFile(long wait) throws IOException { + new String(getSuffixFileDataToWrite(), StandardCharsets.UTF_8)); } LOG.info( - "Read Replica Cluster: This is the active cluster on this storage location with cluster id: {}", + "[Read-replica feature] This cluster is the active (read-write) cluster on this shared storage location, cluster id: {}", new String(getSuffixFileDataToWrite(), StandardCharsets.UTF_8)); } catch (FileNotFoundException fnfe) { // this is the active cluster, create active cluster suffix file if it does not exist From a32da0c4cea05846eeada409f5cc7da39fbc1f4f Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Tue, 10 Mar 2026 03:07:22 +0530 Subject: [PATCH 06/16] HBASE-29961 Secondary cluster is unable to replayWAL for meta (#7854) From ad31477578ac57cfdb44253a1ca47305bda88987 Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Tue, 10 Mar 2026 11:26:52 +0530 Subject: [PATCH 07/16] Add when no suffix provided --- .../src/main/java/org/apache/hadoop/hbase/TableName.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java index e86274bc8f26..ed9b608ec52f 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java @@ -92,7 +92,7 @@ public static TableName getDefaultNameOfMetaForReplica() { public static TableName initializeHbaseMetaTableName(Configuration conf) { String suffix_val = conf.get(HConstants.HBASE_META_TABLE_SUFFIX, HConstants.HBASE_META_TABLE_SUFFIX_DEFAULT_VALUE); - LOG.info("Read Replica Cluster suffix value: {}", suffix_val); + LOG.info("Read Replica Cluster suffix value: {}", (suffix_val == null || suffix_val.isEmpty()) ? "" : suffix_val); if (Strings.isNullOrEmpty(suffix_val)) { return valueOf(NamespaceDescriptor.SYSTEM_NAMESPACE_NAME_STR, "meta"); } else { From 6e81b233a557406b2cafd34270a71a26b8de4e8e Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Tue, 10 Mar 2026 23:35:03 +0530 Subject: [PATCH 08/16] Address few review comments --- .../org/apache/hadoop/hbase/TableName.java | 3 ++- .../hadoop/hbase/master/MasterFileSystem.java | 23 +++++++++++-------- .../org/apache/hadoop/hbase/util/FSUtils.java | 7 +++--- 3 files changed, 20 insertions(+), 13 deletions(-) diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java index ed9b608ec52f..3966fb6d6a5e 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/TableName.java @@ -92,7 +92,8 @@ public static TableName getDefaultNameOfMetaForReplica() { public static TableName initializeHbaseMetaTableName(Configuration conf) { String suffix_val = conf.get(HConstants.HBASE_META_TABLE_SUFFIX, HConstants.HBASE_META_TABLE_SUFFIX_DEFAULT_VALUE); - LOG.info("Read Replica Cluster suffix value: {}", (suffix_val == null || suffix_val.isEmpty()) ? "" : suffix_val); + LOG.debug("[Read-replica feature] suffix value: {}", + (suffix_val == null || suffix_val.isEmpty()) ? "" : suffix_val); if (Strings.isNullOrEmpty(suffix_val)) { return valueOf(NamespaceDescriptor.SYSTEM_NAMESPACE_NAME_STR, "meta"); } else { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java index 9bae027db925..d268a907bb17 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java @@ -399,24 +399,29 @@ private void negotiateActiveClusterSuffixFile(long wait) throws IOException { try { // verify the contents against the config set ActiveClusterSuffix acs = FSUtils.getActiveClusterSuffix(fs, rootdir); - LOG.debug("File Suffix {} : Configured suffix {} : Cluster ID : {}", acs, - getSuffixFromConfig(), getClusterId()); + LOG.debug( + "Negotiating active cluster suffix file. File {} : File Suffix {} : Configured suffix {} : Cluster ID : {}", + new Path(rootdir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME), acs, getSuffixFromConfig(), + getClusterId()); if (Objects.equals(acs.getActiveClusterSuffix(), getSuffixFromConfig())) { this.activeClusterSuffix = acs; } else { // throw error - LOG.info("rootdir {} : Read replica active cluster file suffix {} ", rootdir, acs); + LOG.info( + "[Read-replica feature] Another cluster is running in active (read-write) mode on this storage location. Active cluster ID: {}, This cluster ID {}. Rootdir location {} ", + acs, getSuffixFromConfig(), rootdir); throw new IOException("Cannot start master, because another cluster is running in active " + "(read-write) mode on this storage location. Active Cluster Id: " + acs - + ", This cluster Id: " - + new String(getSuffixFileDataToWrite(), StandardCharsets.UTF_8)); + + ", This cluster Id: " + getSuffixFromConfig()); } LOG.info( - "[Read-replica feature] This cluster is the active (read-write) cluster on this shared storage location, cluster id: {}", - new String(getSuffixFileDataToWrite(), StandardCharsets.UTF_8)); + "[Read-replica feature] This is the active cluster on this storage location with cluster id: {}", + getSuffixFromConfig()); } catch (FileNotFoundException fnfe) { // this is the active cluster, create active cluster suffix file if it does not exist - FSUtils.setActiveClusterSuffix(fs, rootdir, computeAndSetSuffixFileDataToWrite(), wait); + FSUtils.setActiveClusterSuffix(fs, rootdir, getSuffixFileDataToWrite(), wait); + LOG.info("[Read-replica feature] Created Active cluster suffix file: {}, with content: {}", + HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME, getSuffixFromConfig()); } } else { // this is a replica cluster @@ -448,7 +453,7 @@ public byte[] getSuffixFileDataToCompare() { return str.getBytes(StandardCharsets.UTF_8); } - public byte[] computeAndSetSuffixFileDataToWrite() { + public byte[] getSuffixFileDataToWrite() { String str = getClusterId().toString() + ":" + getActiveClusterSuffixFromConfig(conf); this.activeClusterSuffix = new ActiveClusterSuffix(str); return str.getBytes(StandardCharsets.UTF_8); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java index 777eaa30c35c..0ea748272847 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java @@ -642,14 +642,15 @@ public static ActiveClusterSuffix getActiveClusterSuffix(FileSystem fs, Path roo data = in.readUTF(); cs = new ActiveClusterSuffix(data); } catch (EOFException eof) { - LOG.warn("[Read Replica Feature] Active Cluster id file {} is empty ", idPath); + LOG.warn("[Read-replica Feature] Active Cluster id file {} is empty ", idPath); } finally { in.close(); } } return cs; } else { - throw new FileNotFoundException("[Read-replica feature] Active Cluster Suffix File " + idPath + " not found"); + throw new FileNotFoundException( + "[Read-replica feature] Active Cluster Suffix File " + idPath + " not found"); } } @@ -701,7 +702,7 @@ public static void setActiveClusterSuffix(final FileSystem fs, final Path rootdi final Path tempIdFile = new Path(tempDir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME); String fsuffix = new String(bdata, StandardCharsets.US_ASCII); - LOG.debug("Read Replica Cluster id file [{}] is present and contains cluster id: {}", idFile, + LOG.debug("[Read-replica feature] id file [{}] is present and contains cluster id: {}", idFile, fsuffix); writeClusterInfo(fs, rootdir, idFile, tempIdFile, bdata, wait); } From 9023c9ffe88451a38eb1a214f58071fcff6d06e3 Mon Sep 17 00:00:00 2001 From: Andor Molnar Date: Tue, 10 Mar 2026 20:08:30 -0500 Subject: [PATCH 09/16] HBASE-29958. Refactor ActiveClusterSuffix to use protobuf, refactor logging --- .../hadoop/hbase/ActiveClusterSuffix.java | 53 ++++++++++++++++--- .../protobuf/server/ActiveClusterSuffix.proto | 5 +- .../hadoop/hbase/master/MasterFileSystem.java | 40 +++++--------- .../access/AbstractReadOnlyController.java | 3 +- .../org/apache/hadoop/hbase/util/FSUtils.java | 24 ++++++--- .../regionserver/TestActiveClusterSuffix.java | 3 +- 6 files changed, 83 insertions(+), 45 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java index 68fd61a22534..232a16e1c97c 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java @@ -18,8 +18,11 @@ package org.apache.hadoop.hbase; import java.io.IOException; +import java.util.Objects; +import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.exceptions.DeserializationException; import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hbase.thirdparty.com.google.common.base.Strings; import org.apache.yetus.audience.InterfaceAudience; import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; @@ -31,18 +34,40 @@ */ @InterfaceAudience.Private public class ActiveClusterSuffix { - private final String active_cluster_suffix; + private final String cluster_id; + private final String suffix; /** * New ActiveClusterSuffix. */ - public ActiveClusterSuffix(final String cs) { - this.active_cluster_suffix = cs; + public ActiveClusterSuffix(final String cs, final String suffix) { + this.cluster_id = cs; + this.suffix = suffix; } - public String getActiveClusterSuffix() { - return active_cluster_suffix; + public ActiveClusterSuffix(final String input) { + String[] parts = input.split(":"); + this.cluster_id = parts[0]; + if (parts.length > 1) { + this.suffix = parts[1]; + } else { + this.suffix = ""; + } + } + + public static ActiveClusterSuffix fromConfig(Configuration conf, ClusterId clusterId) { + return new ActiveClusterSuffix(clusterId.toString(), conf.get(HConstants.HBASE_META_TABLE_SUFFIX, + HConstants.HBASE_META_TABLE_SUFFIX_DEFAULT_VALUE)); + } + + public static String getSuffixFromConfig(final Configuration conf) { + return conf.get(HConstants.HBASE_META_TABLE_SUFFIX, + HConstants.HBASE_META_TABLE_SUFFIX_DEFAULT_VALUE); + } + + public String getActiveClusterSuffixForLogging() { + return String.format("%s:%s", this.cluster_id, Strings.isNullOrEmpty(this.suffix) ? "" : this.suffix); } /** Returns The active cluster suffix serialized using pb w/ pb magic prefix */ @@ -79,13 +104,15 @@ public static ActiveClusterSuffix parseFrom(final byte[] bytes) throws Deseriali public ActiveClusterSuffixProtos.ActiveClusterSuffix convert() { ActiveClusterSuffixProtos.ActiveClusterSuffix.Builder builder = ActiveClusterSuffixProtos.ActiveClusterSuffix.newBuilder(); - return builder.setActiveClusterSuffix(this.active_cluster_suffix).build(); + builder.setClusterId(cluster_id); + builder.setSuffix(suffix); + return builder.build(); } /** Returns A {@link ActiveClusterSuffix} made from the passed in cs */ public static ActiveClusterSuffix convert(final ActiveClusterSuffixProtos.ActiveClusterSuffix cs) { - return new ActiveClusterSuffix(cs.getActiveClusterSuffix()); + return new ActiveClusterSuffix(cs.getClusterId(), cs.getSuffix()); } /** @@ -93,6 +120,16 @@ public ActiveClusterSuffixProtos.ActiveClusterSuffix convert() { */ @Override public String toString() { - return this.active_cluster_suffix; + return String.format("%s:%s", cluster_id, suffix); + } + + @Override public boolean equals(Object o) { + if (o == null || getClass() != o.getClass()) return false; + ActiveClusterSuffix that = (ActiveClusterSuffix) o; + return Objects.equals(cluster_id, that.cluster_id) && Objects.equals(suffix, that.suffix); + } + + @Override public int hashCode() { + return Objects.hash(cluster_id, suffix); } } diff --git a/hbase-protocol-shaded/src/main/protobuf/server/ActiveClusterSuffix.proto b/hbase-protocol-shaded/src/main/protobuf/server/ActiveClusterSuffix.proto index 89bc086212b3..7f25a040ad03 100644 --- a/hbase-protocol-shaded/src/main/protobuf/server/ActiveClusterSuffix.proto +++ b/hbase-protocol-shaded/src/main/protobuf/server/ActiveClusterSuffix.proto @@ -28,6 +28,9 @@ option optimize_for = SPEED; * Content of the '/hbase/active_cluster_suffix.id' file to indicate the active cluster. */ message ActiveClusterSuffix { + // This is the active cluster id set by the user in the config, as a String + required string cluster_id = 1; + // This is the active cluster suffix set by the user in the config, as a String - required string active_cluster_suffix = 1; + required string suffix = 2; } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java index d268a907bb17..c1f24b55427a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java @@ -395,37 +395,37 @@ public void logFileSystemState(Logger log) throws IOException { } private void negotiateActiveClusterSuffixFile(long wait) throws IOException { + this.activeClusterSuffix = ActiveClusterSuffix.fromConfig(conf, getClusterId()); if (!isReadOnlyModeEnabled(conf)) { try { // verify the contents against the config set ActiveClusterSuffix acs = FSUtils.getActiveClusterSuffix(fs, rootdir); LOG.debug( "Negotiating active cluster suffix file. File {} : File Suffix {} : Configured suffix {} : Cluster ID : {}", - new Path(rootdir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME), acs, getSuffixFromConfig(), + new Path(rootdir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME), acs, activeClusterSuffix.getActiveClusterSuffixForLogging(), getClusterId()); - if (Objects.equals(acs.getActiveClusterSuffix(), getSuffixFromConfig())) { - this.activeClusterSuffix = acs; - } else { + // Suffix file exists and we're in read/write mode. Content should match. + if (!this.activeClusterSuffix.equals(acs)) { // throw error LOG.info( "[Read-replica feature] Another cluster is running in active (read-write) mode on this storage location. Active cluster ID: {}, This cluster ID {}. Rootdir location {} ", - acs, getSuffixFromConfig(), rootdir); + acs.getActiveClusterSuffixForLogging(), activeClusterSuffix.getActiveClusterSuffixForLogging(), rootdir); throw new IOException("Cannot start master, because another cluster is running in active " - + "(read-write) mode on this storage location. Active Cluster Id: " + acs - + ", This cluster Id: " + getSuffixFromConfig()); + + "(read-write) mode on this storage location. Active Cluster Id: " + acs.getActiveClusterSuffixForLogging() + + ", This cluster Id: " + activeClusterSuffix.getActiveClusterSuffixForLogging()); } LOG.info( "[Read-replica feature] This is the active cluster on this storage location with cluster id: {}", - getSuffixFromConfig()); + activeClusterSuffix.getActiveClusterSuffixForLogging()); } catch (FileNotFoundException fnfe) { - // this is the active cluster, create active cluster suffix file if it does not exist - FSUtils.setActiveClusterSuffix(fs, rootdir, getSuffixFileDataToWrite(), wait); + // We're in read/write mode, but suffix file missing, let's create it + FSUtils.setActiveClusterSuffix(fs, rootdir, activeClusterSuffix, wait); LOG.info("[Read-replica feature] Created Active cluster suffix file: {}, with content: {}", - HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME, getSuffixFromConfig()); + HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME, activeClusterSuffix.getActiveClusterSuffixForLogging()); } } else { - // this is a replica cluster - LOG.info("Replica cluster is being started in Read Only Mode"); + // This is a read-only cluster, don't care about suffix file + LOG.info("[Read-replica feature] Replica cluster is being started in Read Only Mode"); } } @@ -438,24 +438,10 @@ private boolean isReadOnlyModeEnabled(Configuration conf) { HConstants.HBASE_GLOBAL_READONLY_ENABLED_DEFAULT); } - private String getActiveClusterSuffixFromConfig(Configuration conf) { - return conf.get(HConstants.HBASE_META_TABLE_SUFFIX, - HConstants.HBASE_META_TABLE_SUFFIX_DEFAULT_VALUE); - } - - public String getSuffixFromConfig() { - return getClusterId().toString() + ":" + getActiveClusterSuffixFromConfig(conf); - } - // Used only for testing public byte[] getSuffixFileDataToCompare() { String str = this.activeClusterSuffix.toString(); return str.getBytes(StandardCharsets.UTF_8); } - public byte[] getSuffixFileDataToWrite() { - String str = getClusterId().toString() + ":" + getActiveClusterSuffixFromConfig(conf); - this.activeClusterSuffix = new ActiveClusterSuffix(str); - return str.getBytes(StandardCharsets.UTF_8); - } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java index f262eba7b93a..be12c62195fb 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java @@ -89,8 +89,7 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile // DISABLING READ-ONLY (true -> false), create the active cluster file id file int wait = mfs.getConfiguration().getInt(HConstants.THREAD_WAKE_FREQUENCY, 10 * 1000); if (!fs.exists(activeClusterFile)) { - FSUtils.setActiveClusterSuffix(fs, rootDir, mfs.computeAndSetSuffixFileDataToWrite(), - wait); + FSUtils.setActiveClusterSuffix(fs, rootDir, mfs.getActiveClusterSuffix(), wait); } else { LOG.debug("Active cluster file already exists at: {}. No need to create it again.", activeClusterFile); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java index 0ea748272847..33eb6fe9f870 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java @@ -646,6 +646,7 @@ public static ActiveClusterSuffix getActiveClusterSuffix(FileSystem fs, Path roo } finally { in.close(); } + rewriteAsPb(fs, rootdir, idPath, cs); } return cs; } else { @@ -669,6 +670,19 @@ private static void rewriteAsPb(final FileSystem fs, final Path rootdir, final P LOG.debug("Rewrote the hbase.id file as pb"); } + private static void rewriteAsPb(final FileSystem fs, final Path rootdir, final Path p, + final ActiveClusterSuffix cs) throws IOException { + // Rewrite the file as pb. Move aside the old one first, write new + // then delete the moved-aside file. + Path movedAsideName = new Path(p + "." + EnvironmentEdgeManager.currentTime()); + if (!fs.rename(p, movedAsideName)) throw new IOException("Failed rename of " + p); + setActiveClusterSuffix(fs, rootdir, cs, 100); + if (!fs.delete(movedAsideName, false)) { + throw new IOException("Failed delete of " + movedAsideName); + } + LOG.debug("Rewrote the active.cluster.suffix.id file as pb"); + } + /** * Writes a new unique identifier for this cluster to the "hbase.id" file in the HBase root * directory. If any operations on the ID file fails, and {@code wait} is a positive value, the @@ -694,17 +708,15 @@ public static void setClusterId(final FileSystem fs, final Path rootdir, * HBase root directory. If any operations on the ID file fails, and {@code wait} is a positive * value, the method will retry to produce the ID file until the thread is forcibly interrupted. */ - - public static void setActiveClusterSuffix(final FileSystem fs, final Path rootdir, byte[] bdata, - final long wait) throws IOException { + public static void setActiveClusterSuffix(final FileSystem fs, final Path rootdir, + final ActiveClusterSuffix cs, final long wait) throws IOException { final Path idFile = new Path(rootdir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME); final Path tempDir = new Path(rootdir, HConstants.HBASE_TEMP_DIRECTORY); final Path tempIdFile = new Path(tempDir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME); - String fsuffix = new String(bdata, StandardCharsets.US_ASCII); LOG.debug("[Read-replica feature] id file [{}] is present and contains cluster id: {}", idFile, - fsuffix); - writeClusterInfo(fs, rootdir, idFile, tempIdFile, bdata, wait); + cs.getActiveClusterSuffixForLogging()); + writeClusterInfo(fs, rootdir, idFile, tempIdFile, cs.toByteArray(), wait); } /** diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestActiveClusterSuffix.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestActiveClusterSuffix.java index df036f08f005..f7262595b7f1 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestActiveClusterSuffix.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestActiveClusterSuffix.java @@ -25,6 +25,7 @@ import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.ActiveClusterSuffix; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseCommonTestingUtil; import org.apache.hadoop.hbase.HBaseTestingUtil; @@ -116,7 +117,7 @@ public void testSuffixFileOnRestart() throws Exception { String cluster_suffix1 = new String(mfs.getSuffixFileDataToCompare(), StandardCharsets.US_ASCII); // Compute using config - String cluster_suffix2 = mfs.getSuffixFromConfig(); + String cluster_suffix2 = ActiveClusterSuffix.getSuffixFromConfig(TEST_UTIL.getConfiguration()); assertEquals(cluster_suffix1, cluster_suffix2); assertEquals(cluster_suffix, cluster_suffix1); From 80fef0292a582fa0bb9f279db2c775f2dafe57a1 Mon Sep 17 00:00:00 2001 From: Andor Molnar Date: Wed, 11 Mar 2026 14:32:53 -0500 Subject: [PATCH 10/16] HBASE-29958. Remove more redundant logic, test cleanup --- .../hadoop/hbase/ActiveClusterSuffix.java | 28 ++++++------------- .../hadoop/hbase/master/MasterFileSystem.java | 22 ++++++--------- .../org/apache/hadoop/hbase/util/FSUtils.java | 6 ++-- .../regionserver/TestActiveClusterSuffix.java | 27 +++++++++++------- .../TestCoprocessorConfigurationUtil.java | 4 +++ 5 files changed, 40 insertions(+), 47 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java index 232a16e1c97c..83f3ed3f8a9a 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java @@ -41,8 +41,8 @@ public class ActiveClusterSuffix { * New ActiveClusterSuffix. */ - public ActiveClusterSuffix(final String cs, final String suffix) { - this.cluster_id = cs; + public ActiveClusterSuffix(final String ci, final String suffix) { + this.cluster_id = ci; this.suffix = suffix; } @@ -61,15 +61,6 @@ public static ActiveClusterSuffix fromConfig(Configuration conf, ClusterId clust HConstants.HBASE_META_TABLE_SUFFIX_DEFAULT_VALUE)); } - public static String getSuffixFromConfig(final Configuration conf) { - return conf.get(HConstants.HBASE_META_TABLE_SUFFIX, - HConstants.HBASE_META_TABLE_SUFFIX_DEFAULT_VALUE); - } - - public String getActiveClusterSuffixForLogging() { - return String.format("%s:%s", this.cluster_id, Strings.isNullOrEmpty(this.suffix) ? "" : this.suffix); - } - /** Returns The active cluster suffix serialized using pb w/ pb magic prefix */ public byte[] toByteArray() { return ProtobufUtil.prependPBMagic(convert().toByteArray()); @@ -102,16 +93,14 @@ public static ActiveClusterSuffix parseFrom(final byte[] bytes) throws Deseriali /** Returns A pb instance to represent this instance. */ public ActiveClusterSuffixProtos.ActiveClusterSuffix convert() { - ActiveClusterSuffixProtos.ActiveClusterSuffix.Builder builder = - ActiveClusterSuffixProtos.ActiveClusterSuffix.newBuilder(); - builder.setClusterId(cluster_id); - builder.setSuffix(suffix); - return builder.build(); + return ActiveClusterSuffixProtos.ActiveClusterSuffix.newBuilder() + .setClusterId(cluster_id) + .setSuffix(suffix) + .build(); } /** Returns A {@link ActiveClusterSuffix} made from the passed in cs */ - public static ActiveClusterSuffix - convert(final ActiveClusterSuffixProtos.ActiveClusterSuffix cs) { + public static ActiveClusterSuffix convert(final ActiveClusterSuffixProtos.ActiveClusterSuffix cs) { return new ActiveClusterSuffix(cs.getClusterId(), cs.getSuffix()); } @@ -120,7 +109,8 @@ public ActiveClusterSuffixProtos.ActiveClusterSuffix convert() { */ @Override public String toString() { - return String.format("%s:%s", cluster_id, suffix); + return String.format("%s:%s", this.cluster_id, + Strings.isNullOrEmpty(this.suffix) ? "" : this.suffix); } @Override public boolean equals(Object o) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java index c1f24b55427a..6983d8276fa1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java @@ -402,26 +402,27 @@ private void negotiateActiveClusterSuffixFile(long wait) throws IOException { ActiveClusterSuffix acs = FSUtils.getActiveClusterSuffix(fs, rootdir); LOG.debug( "Negotiating active cluster suffix file. File {} : File Suffix {} : Configured suffix {} : Cluster ID : {}", - new Path(rootdir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME), acs, activeClusterSuffix.getActiveClusterSuffixForLogging(), + new Path(rootdir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME), acs, activeClusterSuffix, getClusterId()); // Suffix file exists and we're in read/write mode. Content should match. if (!this.activeClusterSuffix.equals(acs)) { // throw error LOG.info( - "[Read-replica feature] Another cluster is running in active (read-write) mode on this storage location. Active cluster ID: {}, This cluster ID {}. Rootdir location {} ", - acs.getActiveClusterSuffixForLogging(), activeClusterSuffix.getActiveClusterSuffixForLogging(), rootdir); + "[Read-replica feature] Another cluster is running in active (read-write) mode on this " + + "storage location. Active cluster ID: {}, This cluster ID {}. Rootdir location {} ", + acs, activeClusterSuffix, rootdir); throw new IOException("Cannot start master, because another cluster is running in active " - + "(read-write) mode on this storage location. Active Cluster Id: " + acs.getActiveClusterSuffixForLogging() - + ", This cluster Id: " + activeClusterSuffix.getActiveClusterSuffixForLogging()); + + "(read-write) mode on this storage location. Active Cluster Id: " + acs + + ", This cluster Id: " + activeClusterSuffix); } LOG.info( "[Read-replica feature] This is the active cluster on this storage location with cluster id: {}", - activeClusterSuffix.getActiveClusterSuffixForLogging()); + activeClusterSuffix); } catch (FileNotFoundException fnfe) { // We're in read/write mode, but suffix file missing, let's create it FSUtils.setActiveClusterSuffix(fs, rootdir, activeClusterSuffix, wait); LOG.info("[Read-replica feature] Created Active cluster suffix file: {}, with content: {}", - HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME, activeClusterSuffix.getActiveClusterSuffixForLogging()); + HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME, activeClusterSuffix); } } else { // This is a read-only cluster, don't care about suffix file @@ -437,11 +438,4 @@ private boolean isReadOnlyModeEnabled(Configuration conf) { return conf.getBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, HConstants.HBASE_GLOBAL_READONLY_ENABLED_DEFAULT); } - - // Used only for testing - public byte[] getSuffixFileDataToCompare() { - String str = this.activeClusterSuffix.toString(); - return str.getBytes(StandardCharsets.UTF_8); - } - } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java index 33eb6fe9f870..0ec76b839053 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java @@ -655,8 +655,6 @@ public static ActiveClusterSuffix getActiveClusterSuffix(FileSystem fs, Path roo } } - /** - * */ private static void rewriteAsPb(final FileSystem fs, final Path rootdir, final Path p, final ClusterId cid) throws IOException { // Rewrite the file as pb. Move aside the old one first, write new @@ -714,8 +712,8 @@ public static void setActiveClusterSuffix(final FileSystem fs, final Path rootdi final Path tempDir = new Path(rootdir, HConstants.HBASE_TEMP_DIRECTORY); final Path tempIdFile = new Path(tempDir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME); - LOG.debug("[Read-replica feature] id file [{}] is present and contains cluster id: {}", idFile, - cs.getActiveClusterSuffixForLogging()); + LOG.debug("[Read-replica feature] id file [{}] is present and contains cluster id: {}", + idFile, cs); writeClusterInfo(fs, rootdir, idFile, tempIdFile, cs.toByteArray(), wait); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestActiveClusterSuffix.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestActiveClusterSuffix.java index f7262595b7f1..aa10372cb84f 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestActiveClusterSuffix.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestActiveClusterSuffix.java @@ -22,15 +22,19 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; +import net.bytebuddy.dynamic.TypeResolutionStrategy; +import org.apache.hadoop.fs.FSDataInputStream; import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.ActiveClusterSuffix; +import org.apache.hadoop.hbase.ClusterId; import org.apache.hadoop.hbase.HBaseClassTestRule; import org.apache.hadoop.hbase.HBaseCommonTestingUtil; import org.apache.hadoop.hbase.HBaseTestingUtil; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.master.MasterFileSystem; +import org.apache.hadoop.hbase.shaded.protobuf.generated.ClusterIdProtos; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.testclassification.RegionServerTests; import org.apache.hadoop.hbase.util.CommonFSUtils; @@ -84,12 +88,14 @@ public void testActiveClusterSuffixCreated() throws Exception { assertTrue(filePath + " should not be empty ", fs.getFileStatus(filePath).getLen() > 0); MasterFileSystem mfs = TEST_UTIL.getHBaseCluster().getMaster().getMasterFileSystem(); - // Compute string using currently set suffix and the cluster id - String cluster_suffix1 = - new String(mfs.getSuffixFileDataToCompare(), StandardCharsets.US_ASCII); - // Compute string member variable - String cluster_suffix2 = mfs.getActiveClusterSuffix().toString(); - assertEquals(cluster_suffix1, cluster_suffix2); + + try (FSDataInputStream in = fs.open(filePath)) { + ActiveClusterSuffix suffixFromFile = ActiveClusterSuffix.parseFrom(in.readAllBytes()); + ActiveClusterSuffix suffixFromConfig = ActiveClusterSuffix.fromConfig(TEST_UTIL.getConfiguration(), + mfs.getClusterId()); + assertEquals("Active Cluster Suffix file content doesn't match configuration", + suffixFromFile, suffixFromConfig); + } } @Test @@ -113,14 +119,15 @@ public void testSuffixFileOnRestart() throws Exception { } MasterFileSystem mfs = TEST_UTIL.getHBaseCluster().getMaster().getMasterFileSystem(); + // Compute using file contents - String cluster_suffix1 = - new String(mfs.getSuffixFileDataToCompare(), StandardCharsets.US_ASCII); + ActiveClusterSuffix cluster_suffix1 = mfs.getActiveClusterSuffix(); // Compute using config - String cluster_suffix2 = ActiveClusterSuffix.getSuffixFromConfig(TEST_UTIL.getConfiguration()); + ActiveClusterSuffix cluster_suffix2 = ActiveClusterSuffix + .fromConfig(TEST_UTIL.getConfiguration(), new ClusterId(clusterId)); assertEquals(cluster_suffix1, cluster_suffix2); - assertEquals(cluster_suffix, cluster_suffix1); + assertEquals(cluster_suffix, cluster_suffix1.toString()); } @Test diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestCoprocessorConfigurationUtil.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestCoprocessorConfigurationUtil.java index eeda71949824..16b68388ed57 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestCoprocessorConfigurationUtil.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestCoprocessorConfigurationUtil.java @@ -34,9 +34,13 @@ import org.apache.hadoop.hbase.security.access.MasterReadOnlyController; import org.apache.hadoop.hbase.security.access.RegionReadOnlyController; import org.apache.hadoop.hbase.security.access.RegionServerReadOnlyController; +import org.apache.hadoop.hbase.testclassification.CoprocessorTests; +import org.apache.hadoop.hbase.testclassification.SmallTests; import org.junit.Before; import org.junit.Test; +import org.junit.experimental.categories.Category; +@Category({ CoprocessorTests.class, SmallTests.class }) public class TestCoprocessorConfigurationUtil { private Configuration conf; From 08e572a8c1111bb3d558c3134adab0423521e75f Mon Sep 17 00:00:00 2001 From: Andor Molnar Date: Wed, 11 Mar 2026 14:34:09 -0500 Subject: [PATCH 11/16] HBASE-29958. Spotless apply --- .../hadoop/hbase/ActiveClusterSuffix.java | 22 ++++++++++--------- .../hadoop/hbase/master/MasterFileSystem.java | 2 -- .../org/apache/hadoop/hbase/util/FSUtils.java | 5 ++--- .../io/hfile/TestLruAdaptiveBlockCache.java | 2 ++ .../regionserver/TestActiveClusterSuffix.java | 15 +++++-------- 5 files changed, 22 insertions(+), 24 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java index 83f3ed3f8a9a..e7903c22a861 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java @@ -22,9 +22,10 @@ import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.exceptions.DeserializationException; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hbase.thirdparty.com.google.common.base.Strings; import org.apache.yetus.audience.InterfaceAudience; +import org.apache.hbase.thirdparty.com.google.common.base.Strings; + import org.apache.hadoop.hbase.shaded.protobuf.ProtobufUtil; import org.apache.hadoop.hbase.shaded.protobuf.generated.ActiveClusterSuffixProtos; @@ -57,8 +58,8 @@ public ActiveClusterSuffix(final String input) { } public static ActiveClusterSuffix fromConfig(Configuration conf, ClusterId clusterId) { - return new ActiveClusterSuffix(clusterId.toString(), conf.get(HConstants.HBASE_META_TABLE_SUFFIX, - HConstants.HBASE_META_TABLE_SUFFIX_DEFAULT_VALUE)); + return new ActiveClusterSuffix(clusterId.toString(), conf + .get(HConstants.HBASE_META_TABLE_SUFFIX, HConstants.HBASE_META_TABLE_SUFFIX_DEFAULT_VALUE)); } /** Returns The active cluster suffix serialized using pb w/ pb magic prefix */ @@ -93,14 +94,13 @@ public static ActiveClusterSuffix parseFrom(final byte[] bytes) throws Deseriali /** Returns A pb instance to represent this instance. */ public ActiveClusterSuffixProtos.ActiveClusterSuffix convert() { - return ActiveClusterSuffixProtos.ActiveClusterSuffix.newBuilder() - .setClusterId(cluster_id) - .setSuffix(suffix) - .build(); + return ActiveClusterSuffixProtos.ActiveClusterSuffix.newBuilder().setClusterId(cluster_id) + .setSuffix(suffix).build(); } /** Returns A {@link ActiveClusterSuffix} made from the passed in cs */ - public static ActiveClusterSuffix convert(final ActiveClusterSuffixProtos.ActiveClusterSuffix cs) { + public static ActiveClusterSuffix + convert(final ActiveClusterSuffixProtos.ActiveClusterSuffix cs) { return new ActiveClusterSuffix(cs.getClusterId(), cs.getSuffix()); } @@ -113,13 +113,15 @@ public String toString() { Strings.isNullOrEmpty(this.suffix) ? "" : this.suffix); } - @Override public boolean equals(Object o) { + @Override + public boolean equals(Object o) { if (o == null || getClass() != o.getClass()) return false; ActiveClusterSuffix that = (ActiveClusterSuffix) o; return Objects.equals(cluster_id, that.cluster_id) && Objects.equals(suffix, that.suffix); } - @Override public int hashCode() { + @Override + public int hashCode() { return Objects.hash(cluster_id, suffix); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java index 6983d8276fa1..3708e28e5fdb 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterFileSystem.java @@ -19,8 +19,6 @@ import java.io.FileNotFoundException; import java.io.IOException; -import java.nio.charset.StandardCharsets; -import java.util.Objects; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java index 0ec76b839053..601eb2f81f44 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java @@ -31,7 +31,6 @@ import java.lang.reflect.Method; import java.net.InetSocketAddress; import java.net.URI; -import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -712,8 +711,8 @@ public static void setActiveClusterSuffix(final FileSystem fs, final Path rootdi final Path tempDir = new Path(rootdir, HConstants.HBASE_TEMP_DIRECTORY); final Path tempIdFile = new Path(tempDir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME); - LOG.debug("[Read-replica feature] id file [{}] is present and contains cluster id: {}", - idFile, cs); + LOG.debug("[Read-replica feature] id file [{}] is present and contains cluster id: {}", idFile, + cs); writeClusterInfo(fs, rootdir, idFile, tempIdFile, cs.toByteArray(), wait); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruAdaptiveBlockCache.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruAdaptiveBlockCache.java index 00f43fcced94..d3669224c28e 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruAdaptiveBlockCache.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruAdaptiveBlockCache.java @@ -45,6 +45,7 @@ import org.apache.hadoop.hbase.util.ClassSize; import org.junit.Assert; import org.junit.ClassRule; +import org.junit.Ignore; import org.junit.Test; import org.junit.experimental.categories.Category; import org.slf4j.Logger; @@ -56,6 +57,7 @@ * Tests will ensure it grows and shrinks in size properly, evictions run when they're supposed to * and do what they should, and that cached blocks are accessible when expected to be. */ +@Ignore @Category({ IOTests.class, SmallTests.class }) public class TestLruAdaptiveBlockCache { diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestActiveClusterSuffix.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestActiveClusterSuffix.java index aa10372cb84f..3d566880ab90 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestActiveClusterSuffix.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestActiveClusterSuffix.java @@ -21,8 +21,6 @@ import static org.junit.Assert.assertTrue; import java.io.IOException; -import java.nio.charset.StandardCharsets; -import net.bytebuddy.dynamic.TypeResolutionStrategy; import org.apache.hadoop.fs.FSDataInputStream; import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.FileSystem; @@ -34,7 +32,6 @@ import org.apache.hadoop.hbase.HBaseTestingUtil; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.master.MasterFileSystem; -import org.apache.hadoop.hbase.shaded.protobuf.generated.ClusterIdProtos; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.testclassification.RegionServerTests; import org.apache.hadoop.hbase.util.CommonFSUtils; @@ -91,10 +88,10 @@ public void testActiveClusterSuffixCreated() throws Exception { try (FSDataInputStream in = fs.open(filePath)) { ActiveClusterSuffix suffixFromFile = ActiveClusterSuffix.parseFrom(in.readAllBytes()); - ActiveClusterSuffix suffixFromConfig = ActiveClusterSuffix.fromConfig(TEST_UTIL.getConfiguration(), - mfs.getClusterId()); - assertEquals("Active Cluster Suffix file content doesn't match configuration", - suffixFromFile, suffixFromConfig); + ActiveClusterSuffix suffixFromConfig = + ActiveClusterSuffix.fromConfig(TEST_UTIL.getConfiguration(), mfs.getClusterId()); + assertEquals("Active Cluster Suffix file content doesn't match configuration", suffixFromFile, + suffixFromConfig); } } @@ -123,8 +120,8 @@ public void testSuffixFileOnRestart() throws Exception { // Compute using file contents ActiveClusterSuffix cluster_suffix1 = mfs.getActiveClusterSuffix(); // Compute using config - ActiveClusterSuffix cluster_suffix2 = ActiveClusterSuffix - .fromConfig(TEST_UTIL.getConfiguration(), new ClusterId(clusterId)); + ActiveClusterSuffix cluster_suffix2 = + ActiveClusterSuffix.fromConfig(TEST_UTIL.getConfiguration(), new ClusterId(clusterId)); assertEquals(cluster_suffix1, cluster_suffix2); assertEquals(cluster_suffix, cluster_suffix1.toString()); From 68eb9d7a56676555677140d743f5bd65613eef93 Mon Sep 17 00:00:00 2001 From: Andor Molnar Date: Wed, 11 Mar 2026 14:36:12 -0500 Subject: [PATCH 12/16] HBASE-29958. Revert mistake --- .../apache/hadoop/hbase/io/hfile/TestLruAdaptiveBlockCache.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruAdaptiveBlockCache.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruAdaptiveBlockCache.java index d3669224c28e..00f43fcced94 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruAdaptiveBlockCache.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/TestLruAdaptiveBlockCache.java @@ -45,7 +45,6 @@ import org.apache.hadoop.hbase.util.ClassSize; import org.junit.Assert; import org.junit.ClassRule; -import org.junit.Ignore; import org.junit.Test; import org.junit.experimental.categories.Category; import org.slf4j.Logger; @@ -57,7 +56,6 @@ * Tests will ensure it grows and shrinks in size properly, evictions run when they're supposed to * and do what they should, and that cached blocks are accessible when expected to be. */ -@Ignore @Category({ IOTests.class, SmallTests.class }) public class TestLruAdaptiveBlockCache { From 7920c6fe07eb931e2e972b87d559d7d0cacd4900 Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Thu, 5 Mar 2026 20:53:41 +0530 Subject: [PATCH 13/16] HBASE-29958 Improve log messages --- .../src/main/java/org/apache/hadoop/hbase/util/FSUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java index 601eb2f81f44..bd65e65a384a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java @@ -710,7 +710,7 @@ public static void setActiveClusterSuffix(final FileSystem fs, final Path rootdi final Path idFile = new Path(rootdir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME); final Path tempDir = new Path(rootdir, HConstants.HBASE_TEMP_DIRECTORY); final Path tempIdFile = new Path(tempDir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME); - + LOG.debug("[Read-replica feature] id file [{}] is present and contains cluster id: {}", idFile, cs); writeClusterInfo(fs, rootdir, idFile, tempIdFile, cs.toByteArray(), wait); From a4a482f88abdcb39ce2977a11e380843768186d8 Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Thu, 12 Mar 2026 15:53:53 +0530 Subject: [PATCH 14/16] Address Kevin's review comment to address multiple : in active cluster suffix --- .../java/org/apache/hadoop/hbase/ActiveClusterSuffix.java | 6 +----- .../hbase/security/access/AbstractReadOnlyController.java | 2 +- .../src/main/java/org/apache/hadoop/hbase/util/FSUtils.java | 2 +- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java index e7903c22a861..4d3409a1211a 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ActiveClusterSuffix.java @@ -38,17 +38,13 @@ public class ActiveClusterSuffix { private final String cluster_id; private final String suffix; - /** - * New ActiveClusterSuffix. - */ - public ActiveClusterSuffix(final String ci, final String suffix) { this.cluster_id = ci; this.suffix = suffix; } public ActiveClusterSuffix(final String input) { - String[] parts = input.split(":"); + String[] parts = input.split(":", 2); this.cluster_id = parts[0]; if (parts.length > 1) { this.suffix = parts[1]; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java index be12c62195fb..d44a7b691a3a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java @@ -65,7 +65,7 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile activeClusterFile); try (FSDataInputStream in = fs.open(activeClusterFile)) { String actualClusterFileData = IOUtils.toString(in, StandardCharsets.UTF_8); - String expectedClusterFileData = mfs.getSuffixFromConfig(); + String expectedClusterFileData = mfs.getActiveClusterSuffix().toString(); if (actualClusterFileData.equals(expectedClusterFileData)) { fs.delete(activeClusterFile, false); LOG.info("Successfully deleted active cluster file: {}", activeClusterFile); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java index bd65e65a384a..601eb2f81f44 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/FSUtils.java @@ -710,7 +710,7 @@ public static void setActiveClusterSuffix(final FileSystem fs, final Path rootdi final Path idFile = new Path(rootdir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME); final Path tempDir = new Path(rootdir, HConstants.HBASE_TEMP_DIRECTORY); final Path tempIdFile = new Path(tempDir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME); - + LOG.debug("[Read-replica feature] id file [{}] is present and contains cluster id: {}", idFile, cs); writeClusterInfo(fs, rootdir, idFile, tempIdFile, cs.toByteArray(), wait); From e375bd864be111786eab780f2688c0d1f02ee93e Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Thu, 12 Mar 2026 16:39:25 +0530 Subject: [PATCH 15/16] As getClusterSuffixFromConfig() changed we need to change the code for file deletion --- .../hbase/security/access/AbstractReadOnlyController.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java index d44a7b691a3a..f0a6545c99ed 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java @@ -19,7 +19,7 @@ import java.io.FileNotFoundException; import java.io.IOException; -import java.nio.charset.StandardCharsets; +import java.util.Arrays; import org.apache.commons.io.IOUtils; import org.apache.hadoop.fs.FSDataInputStream; import org.apache.hadoop.fs.FileSystem; @@ -64,9 +64,9 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile LOG.debug("Global read-only mode is being ENABLED. Deleting active cluster file: {}", activeClusterFile); try (FSDataInputStream in = fs.open(activeClusterFile)) { - String actualClusterFileData = IOUtils.toString(in, StandardCharsets.UTF_8); - String expectedClusterFileData = mfs.getActiveClusterSuffix().toString(); - if (actualClusterFileData.equals(expectedClusterFileData)) { + byte[] actualClusterFileData = IOUtils.toByteArray(in); + byte[] expectedClusterFileData = mfs.getActiveClusterSuffix().toByteArray(); + if (Arrays.equals(actualClusterFileData, expectedClusterFileData)) { fs.delete(activeClusterFile, false); LOG.info("Successfully deleted active cluster file: {}", activeClusterFile); } else { From 0b7c31f67501aef4b3e7d0a56f34933d7255d51b Mon Sep 17 00:00:00 2001 From: Anuj Sharma Date: Thu, 12 Mar 2026 17:59:16 +0530 Subject: [PATCH 16/16] Use ActiveClusterSuffix object based comparison instead of byte Array comparison --- .../security/access/AbstractReadOnlyController.java | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java index f0a6545c99ed..83c4b09d1d5d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java @@ -19,16 +19,16 @@ import java.io.FileNotFoundException; import java.io.IOException; -import java.util.Arrays; -import org.apache.commons.io.IOUtils; import org.apache.hadoop.fs.FSDataInputStream; import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.ActiveClusterSuffix; import org.apache.hadoop.hbase.Coprocessor; import org.apache.hadoop.hbase.CoprocessorEnvironment; import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.HBaseInterfaceAudience; import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.exceptions.DeserializationException; import org.apache.hadoop.hbase.master.MasterFileSystem; import org.apache.hadoop.hbase.master.MasterServices; import org.apache.hadoop.hbase.util.FSUtils; @@ -64,9 +64,10 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile LOG.debug("Global read-only mode is being ENABLED. Deleting active cluster file: {}", activeClusterFile); try (FSDataInputStream in = fs.open(activeClusterFile)) { - byte[] actualClusterFileData = IOUtils.toByteArray(in); - byte[] expectedClusterFileData = mfs.getActiveClusterSuffix().toByteArray(); - if (Arrays.equals(actualClusterFileData, expectedClusterFileData)) { + ActiveClusterSuffix actualClusterFileData = + ActiveClusterSuffix.parseFrom(in.readAllBytes()); + ActiveClusterSuffix expectedClusterFileData = mfs.getActiveClusterSuffix(); + if (expectedClusterFileData.equals(actualClusterFileData)) { fs.delete(activeClusterFile, false); LOG.info("Successfully deleted active cluster file: {}", activeClusterFile); } else { @@ -84,6 +85,8 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile "Failed to delete active cluster file: {}. " + "Read-only flag will be updated, but file system state is inconsistent.", activeClusterFile, e); + } catch (DeserializationException e) { + LOG.error("Failed to deserialize ActiveClusterSuffix from file {}", activeClusterFile, e); } } else { // DISABLING READ-ONLY (true -> false), create the active cluster file id file