From d5f43b0636d91c7bed02f1b70f93801dd8bdca51 Mon Sep 17 00:00:00 2001 From: Kevin Geiszler Date: Thu, 11 Jun 2026 13:33:15 -0700 Subject: [PATCH 1/3] HBASE-30220: A replica cluster can have read-only mode disabled even when another active cluster already exists Code generated with Claude Opus 4.6 and modified by hand after Change-Id: Ifb6992d1c9d6982cdcec0522d7b75ed9f985c0e3 --- .../hbase/ReadOnlyTransitionException.java | 56 +++++ .../apache/hadoop/hbase/HBaseServerBase.java | 28 ++- .../apache/hadoop/hbase/master/HMaster.java | 28 ++- .../hadoop/hbase/regionserver/HRegion.java | 45 +++- .../hbase/regionserver/HRegionServer.java | 36 +++- .../access/AbstractReadOnlyController.java | 46 ++++- .../hadoop/hbase/util/ConfigurationUtil.java | 14 ++ .../TestReadOnlyManageActiveClusterFile.java | 192 ++++++++++++++++++ 8 files changed, 427 insertions(+), 18 deletions(-) create mode 100644 hbase-client/src/main/java/org/apache/hadoop/hbase/ReadOnlyTransitionException.java diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ReadOnlyTransitionException.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ReadOnlyTransitionException.java new file mode 100644 index 000000000000..71b25198073c --- /dev/null +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ReadOnlyTransitionException.java @@ -0,0 +1,56 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.hadoop.hbase; + +import org.apache.yetus.audience.InterfaceAudience; + +/** + * Thrown when a replica cluster attempts to disable read-only mode while another active cluster + * already exists on the same storage location. + */ +@InterfaceAudience.Public +public class ReadOnlyTransitionException extends DoNotRetryIOException { + + private static final long serialVersionUID = 1L; + + public ReadOnlyTransitionException() { + super(); + } + + /** + * @param message the message for this exception + */ + public ReadOnlyTransitionException(String message) { + super(message); + } + + /** + * @param message the message for this exception + * @param throwable the {@link Throwable} to use for this exception + */ + public ReadOnlyTransitionException(String message, Throwable throwable) { + super(message, throwable); + } + + /** + * @param throwable the {@link Throwable} to use for this exception + */ + public ReadOnlyTransitionException(Throwable throwable) { + super(throwable); + } +} diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java index 59da3ff7e609..12bd126ddaa1 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java @@ -73,6 +73,7 @@ import org.apache.hadoop.hbase.unsafe.HBasePlatformDependent; import org.apache.hadoop.hbase.util.Addressing; import org.apache.hadoop.hbase.util.CommonFSUtils; +import org.apache.hadoop.hbase.util.ConfigurationUtil; import org.apache.hadoop.hbase.util.DNS; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.FSTableDescriptors; @@ -102,15 +103,19 @@ public abstract class HBaseServerBase> extends protected final AtomicBoolean abortRequested = new AtomicBoolean(false); // Set when a report to the master comes back with a message asking us to - // shutdown. Also set by call to stop when debugging or running unit tests + // shut down. Also set by call to stop when debugging or running unit tests // of HRegionServer in isolation. protected volatile boolean stopped = false; + // Flag set when a read-only to read-write transition is blocked because another active cluster + // exists + protected volatile boolean readOnlyTransitionBlocked = false; + // Only for testing private boolean isShutdownHookInstalled = false; /** - * This servers startcode. + * This server's startcode. */ protected final long startcode; @@ -639,11 +644,30 @@ public void updateConfiguration() throws IOException { LOG.info("Reloading the configuration from disk."); // Reload the configuration from disk. preUpdateConfiguration(); + this.readOnlyTransitionBlocked = false; conf.reloadConfiguration(); configurationManager.notifyAllObservers(conf); + this.checkForBlockedReadOnlyTransition(); postUpdateConfiguration(); } + protected Configuration blockReadOnlyTransition(Configuration updatedConf) { + LOG.error( + "Cannot disable read-only mode. The {} file contains a different cluster ID, which means " + + "that cluster is already the active cluster. Reverting {} to true", + HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME, HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY); + this.readOnlyTransitionBlocked = true; + return ConfigurationUtil.getReadOnlyEnabledConfigurationCopy(updatedConf); + } + + protected void checkForBlockedReadOnlyTransition() throws ReadOnlyTransitionException { + if (this.readOnlyTransitionBlocked) { + throw new ReadOnlyTransitionException( + "Cannot disable read-only mode because another active cluster already exists on this " + + "storage location. The read-only coprocessors have not been removed."); + } + } + @Override public KeyManagementService getKeyManagementService() { return this; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index f5aaa6e333ea..b392398b9082 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -4520,11 +4520,31 @@ public void onConfigurationChange(Configuration updatedConf) { boolean originalIsReadOnlyEnabled = CoprocessorConfigurationUtil .areReadOnlyCoprocessorsLoaded(this.conf, CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY); + boolean newReadOnlyEnabled = ConfigurationUtil.isReadOnlyModeEnabledInConf(updatedConf); - // updatedConf and this.conf reference the same Configuration object in an actual HBase - // deployment. However, in unit test cases they reference different Configuration objects, so - // this.conf needs to be updated. - CoprocessorConfigurationUtil.maybeUpdateCoprocessors(updatedConf, this.conf, + // The updatedConf is potentially a shared Configuration object, so we do not want to directly + // revert its read-only value if another active cluster already exists. For now, we reference + // updatedConf and create a copy for modification below if necessary. + Configuration confForCoprocessors = updatedConf; + + if (originalIsReadOnlyEnabled && !newReadOnlyEnabled) { + // Changing this cluster from a replica to an active cluster. There should not be another + // active cluster already. + MasterFileSystem mfs = this.getMasterFileSystem(); + if ( + AbstractReadOnlyController.isAnotherClusterActive(mfs.getFileSystem(), mfs.getRootDir(), + mfs.getActiveClusterSuffix()) + ) { + // Revert read-only mode here + confForCoprocessors = this.blockReadOnlyTransition(updatedConf); + } + } + + // In a real HBase deployment, confForCoprocessors may reference the same object as this.conf. + // This is assuming confForCoprocessors still references updatedConf, as mentioned in a previous + // comment. For unit tests, this Configuration object is not shared, so we need to make sure to + // update the coprocessors specifically for this.conf. + CoprocessorConfigurationUtil.maybeUpdateCoprocessors(confForCoprocessors, this.conf, originalIsReadOnlyEnabled, this.cpHost, CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, this.maintenanceMode, this.toString(), this::initializeCoprocessorHost); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 6871a79324f5..de920146ccef 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -78,12 +78,14 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.LocatedFileStatus; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.ActiveClusterSuffix; import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellBuilderType; import org.apache.hadoop.hbase.CellComparator; import org.apache.hadoop.hbase.CellComparatorImpl; import org.apache.hadoop.hbase.CellScanner; import org.apache.hadoop.hbase.CellUtil; +import org.apache.hadoop.hbase.ClusterId; import org.apache.hadoop.hbase.CompareOperator; import org.apache.hadoop.hbase.CompoundConfiguration; import org.apache.hadoop.hbase.DoNotRetryIOException; @@ -172,6 +174,7 @@ import org.apache.hadoop.hbase.replication.ReplicationUtils; import org.apache.hadoop.hbase.replication.regionserver.ReplicationObserver; import org.apache.hadoop.hbase.security.User; +import org.apache.hadoop.hbase.security.access.AbstractReadOnlyController; import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils; import org.apache.hadoop.hbase.snapshot.SnapshotManifest; import org.apache.hadoop.hbase.trace.TraceUtil; @@ -8997,20 +9000,54 @@ public void onConfigurationChange(Configuration updatedConf) { boolean originalIsReadOnlyEnabled = CoprocessorConfigurationUtil .areReadOnlyCoprocessorsLoaded(this.conf, CoprocessorHost.REGION_COPROCESSOR_CONF_KEY); + boolean newReadOnlyEnabled = ConfigurationUtil.isReadOnlyModeEnabledInConf(updatedConf); + + // The updatedConf is potentially a shared Configuration object, so we do not want to directly + // revert its read-only value if another active cluster already exists. For now, we reference + // updatedConf and create a copy for modification below if necessary. + Configuration confForCoprocessors = updatedConf; + + if (originalIsReadOnlyEnabled && !newReadOnlyEnabled) { + // Changing this cluster from a replica to an active cluster. There should not be another + // active cluster already. + try { + FileSystem regionFs = getFilesystem(); + Path rootDir = CommonFSUtils.getRootDir(this.conf); + ClusterId clusterId = FSUtils.getClusterIdFile(regionFs, rootDir, new ClusterId.Parser()); + if (clusterId != null) { + ActiveClusterSuffix localSuffix = ActiveClusterSuffix.fromConfig(updatedConf, clusterId); + if (AbstractReadOnlyController.isAnotherClusterActive(regionFs, rootDir, localSuffix)) { + LOG.error( + "Cannot disable read-only mode for region {}. Another cluster is already " + + "the active cluster on this storage location. Reverting {} to true.", + this, HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY); + // Revert read-only mode here + confForCoprocessors = + ConfigurationUtil.getReadOnlyEnabledConfigurationCopy(updatedConf); + newReadOnlyEnabled = true; + } + } + } catch (IOException e) { + LOG.error("Failed to check active cluster status for region {}. " + + "Blocking read-only mode transition to prevent potential data corruption.", this, e); + // Revert read-only mode here + confForCoprocessors = ConfigurationUtil.getReadOnlyEnabledConfigurationCopy(updatedConf); + newReadOnlyEnabled = true; + } + } // HRegion's this.conf is a special Configuration type called CompoundConfiguration. This means - // we don't want to use the updatedConf provided in onConfigurationChange() for creating a new + // we don't want to use the confForCoprocessors Configuration for creating a new // RegionCoprocessorHost. Instead, we update this.conf and use that for decorating the region // config and updating this.coprocessorHost. - CoprocessorConfigurationUtil.maybeUpdateCoprocessors(updatedConf, this.conf, + CoprocessorConfigurationUtil.maybeUpdateCoprocessors(confForCoprocessors, this.conf, originalIsReadOnlyEnabled, this.coprocessorHost, CoprocessorHost.REGION_COPROCESSOR_CONF_KEY, false, this.toString(), conf -> { decorateRegionConfiguration(conf); this.coprocessorHost = new RegionCoprocessorHost(this, rsServices, conf); }); - boolean newReadOnlyEnabled = ConfigurationUtil.isReadOnlyModeEnabledInConf(updatedConf); - + // Changing this cluster from a replica to an active cluster if (originalIsReadOnlyEnabled && !newReadOnlyEnabled) { LOG.info("Cluster Read Only mode disabled"); for (HStore store : stores.values()) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index a7bf72dabd74..a7a44dc515db 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java @@ -74,9 +74,11 @@ import org.apache.hadoop.fs.FileSystem; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.Abortable; +import org.apache.hadoop.hbase.ActiveClusterSuffix; import org.apache.hadoop.hbase.CacheEvictionStats; import org.apache.hadoop.hbase.CallQueueTooBigException; import org.apache.hadoop.hbase.ClockOutOfSyncException; +import org.apache.hadoop.hbase.ClusterId; import org.apache.hadoop.hbase.DoNotRetryIOException; import org.apache.hadoop.hbase.ExecutorStatusChore; import org.apache.hadoop.hbase.HBaseConfiguration; @@ -155,9 +157,11 @@ import org.apache.hadoop.hbase.security.Superusers; import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.UserProvider; +import org.apache.hadoop.hbase.security.access.AbstractReadOnlyController; import org.apache.hadoop.hbase.trace.TraceUtil; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.CompressionTest; +import org.apache.hadoop.hbase.util.ConfigurationUtil; import org.apache.hadoop.hbase.util.CoprocessorConfigurationUtil; import org.apache.hadoop.hbase.util.DNS; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; @@ -315,7 +319,6 @@ public class HRegionServer extends HBaseServerBase private LeaseManager leaseManager; private volatile boolean dataFsOk; - private volatile boolean isGlobalReadOnlyEnabled; static final String ABORT_TIMEOUT = "hbase.regionserver.abort.timeout"; // Default abort timeout is 1200 seconds for safe @@ -3505,11 +3508,32 @@ public void onConfigurationChange(Configuration updatedConf) { boolean originalIsReadOnlyEnabled = CoprocessorConfigurationUtil .areReadOnlyCoprocessorsLoaded(this.conf, CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY); - - // updatedConf and this.conf reference the same Configuration object in an actual HBase - // deployment. However, in unit test cases they reference different Configuration objects, so - // this.conf needs to be updated. - CoprocessorConfigurationUtil.maybeUpdateCoprocessors(updatedConf, this.conf, + boolean newReadOnlyEnabled = ConfigurationUtil.isReadOnlyModeEnabledInConf(updatedConf); + + // The updatedConf is potentially a shared Configuration object, so we do not want to directly + // revert its read-only value if another active cluster already exists. For now, we reference + // updatedConf and create a copy for modification below if necessary. + Configuration confForCoprocessors = updatedConf; + + if (originalIsReadOnlyEnabled && !newReadOnlyEnabled) { + // Changing this cluster from a replica to an active cluster. There should not be another + // active cluster already. + ActiveClusterSuffix localSuffix = + ActiveClusterSuffix.fromConfig(this.conf, new ClusterId(getClusterId())); + if ( + AbstractReadOnlyController.isAnotherClusterActive(getFileSystem(), getDataRootDir(), + localSuffix) + ) { + // Revert read-only mode here + confForCoprocessors = this.blockReadOnlyTransition(updatedConf); + } + } + + // In a real HBase deployment, confForCoprocessors may reference the same object as this.conf. + // This is assuming confForCoprocessors still references updatedConf, as mentioned in a previous + // comment. For unit tests, this Configuration object is not shared, so we need to make sure to + // update the coprocessors specifically for this.conf. + CoprocessorConfigurationUtil.maybeUpdateCoprocessors(confForCoprocessors, this.conf, originalIsReadOnlyEnabled, this.rsHost, CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY, false, this.toString(), conf -> this.rsHost = new RegionServerCoprocessorHost(this, conf)); } 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 d13c84779196..8ad0a10d3f8c 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 @@ -68,6 +68,33 @@ public void start(CoprocessorEnvironment env) throws IOException { public void stop(CoprocessorEnvironment env) { } + /** + * Checks whether another cluster is currently active on this storage location by reading the + * {@value HConstants#ACTIVE_CLUSTER_SUFFIX_FILE_NAME} file on the filesystem. + * @param fs the filesystem to read from + * @param rootDir the HBase root directory + * @param localClusterSuffix the local cluster's ActiveClusterSuffix identity + * @return true if the active cluster file exists and belongs to a different cluster; false if the + * file does not exist or belongs to this cluster + */ + public static boolean isAnotherClusterActive(FileSystem fs, Path rootDir, + ActiveClusterSuffix localClusterSuffix) { + try { + ActiveClusterSuffix fileData = + FSUtils.getClusterIdFile(fs, rootDir, new ActiveClusterSuffix.Parser()); + if (fileData == null) { + return false; + } + return !localClusterSuffix.equals(fileData); + } catch (IOException e) { + LOG.error( + "Failed to read active cluster suffix file from {} at {}. " + + "Assuming another cluster is active to prevent potential data corruption.", + HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME, rootDir, e); + return true; + } + } + public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFileSystem mfs) { FileSystem fs = mfs.getFileSystem(); Path rootDir = mfs.getRootDir(); @@ -110,8 +137,23 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile FSUtils.setClusterIdFile(fs, rootDir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME, mfs.getActiveClusterSuffix(), wait); } else { - LOG.debug("Active cluster file already exists at: {}. No need to create it again.", - activeClusterFile); + try (FSDataInputStream in = fs.open(activeClusterFile)) { + ActiveClusterSuffix existingData = ActiveClusterSuffix.parseFrom(in.readAllBytes()); + ActiveClusterSuffix localData = mfs.getActiveClusterSuffix(); + if (localData.equals(existingData)) { + LOG.debug("Active cluster file already exists at {} and belongs to this cluster. " + + "No need to create it again.", activeClusterFile); + } else { + LOG.error( + "Active cluster file at {} belongs to a different cluster. " + + "ID from active cluster file: {}, ID of this cluster: {}. " + + "Another cluster is already the active cluster.", + activeClusterFile, existingData, localData); + } + } catch (DeserializationException e) { + LOG.error("Failed to deserialize ActiveClusterSuffix from file {}", activeClusterFile, + e); + } } } } catch (IOException e) { diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/ConfigurationUtil.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/ConfigurationUtil.java index 63e641f53643..78ad69d6be5a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/ConfigurationUtil.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/ConfigurationUtil.java @@ -116,8 +116,22 @@ public static List> getKeyValues(Configuration conf, S return rtn; } + /** + * Returns true if the provided Configuration object has + * {@link HConstants#HBASE_GLOBAL_READONLY_ENABLED_KEY} set to true; false otherwise. + */ public static boolean isReadOnlyModeEnabledInConf(Configuration conf) { return conf.getBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, HConstants.HBASE_GLOBAL_READONLY_ENABLED_DEFAULT); } + + /** + * Returns a copied version of the provided Configuration object that has + * {@link HConstants#HBASE_GLOBAL_READONLY_ENABLED_KEY} set to true. + */ + public static Configuration getReadOnlyEnabledConfigurationCopy(Configuration conf) { + Configuration readOnlyConf = new Configuration(conf); + readOnlyConf.setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, true); + return readOnlyConf; + } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestReadOnlyManageActiveClusterFile.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestReadOnlyManageActiveClusterFile.java index dae8dc3e27a2..1f94a0090edd 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestReadOnlyManageActiveClusterFile.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestReadOnlyManageActiveClusterFile.java @@ -17,21 +17,41 @@ */ package org.apache.hadoop.hbase.security.access; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.assertTrue; import java.io.IOException; +import java.util.List; import org.apache.hadoop.conf.Configuration; +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.HBaseTestingUtil; import org.apache.hadoop.hbase.HConstants; +import org.apache.hadoop.hbase.ReadOnlyTransitionException; +import org.apache.hadoop.hbase.TableName; +import org.apache.hadoop.hbase.client.ColumnFamilyDescriptorBuilder; +import org.apache.hadoop.hbase.client.Put; +import org.apache.hadoop.hbase.client.Table; +import org.apache.hadoop.hbase.client.TableDescriptor; +import org.apache.hadoop.hbase.client.TableDescriptorBuilder; +import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; +import org.apache.hadoop.hbase.coprocessor.SimpleRegionObserver; import org.apache.hadoop.hbase.master.HMaster; import org.apache.hadoop.hbase.master.MasterFileSystem; +import org.apache.hadoop.hbase.regionserver.HRegion; import org.apache.hadoop.hbase.regionserver.HRegionServer; +import org.apache.hadoop.hbase.regionserver.NoOpScanPolicyObserver; +import org.apache.hadoop.hbase.regionserver.RegionCoprocessorHost; import org.apache.hadoop.hbase.testclassification.MediumTests; import org.apache.hadoop.hbase.testclassification.SecurityTests; +import org.apache.hadoop.hbase.util.Bytes; +import org.apache.hadoop.hbase.util.CoprocessorConfigurationUtil; import org.junit.jupiter.api.AfterEach; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Tag; @@ -146,4 +166,176 @@ public void testDoNotDeleteActiveClusterIdFileWhenSwitchingToReadOnlyIfNotOwnedB // switching to readonly mode assertTrue(activeClusterIdFileExists()); } + + @Test + public void testCannotDisableReadOnlyWhenAnotherClusterIsActive() throws Exception { + // First enable read-only mode (simulating a replica cluster) + setReadOnlyMode(true); + assertFalse(activeClusterIdFileExists()); + + // Now write an active cluster file with a DIFFERENT cluster's data (simulating another active + // cluster owning the storage) + overwriteExistingFile(); + assertTrue(activeClusterIdFileExists()); + + // Attempt to disable read-only mode, but get an exception because another active cluster + // already exists. + master.getConfiguration().setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, false); + + // Master's updateConfiguration should throw because another cluster is active + assertThrows(ReadOnlyTransitionException.class, () -> master.updateConfiguration()); + + // Verify read-only coprocessors are still loaded on the master + assertTrue(CoprocessorConfigurationUtil.areReadOnlyCoprocessorsLoaded(master.getConfiguration(), + CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY)); + + // Verify read-only coprocessors are still loaded on the region server + assertTrue(CoprocessorConfigurationUtil.areReadOnlyCoprocessorsLoaded( + regionServer.getConfiguration(), CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY)); + } + + @Test + public void testCanDisableReadOnlyWhenOwnClusterIsActive() throws Exception { + // Enable read-only mode + setReadOnlyMode(true); + assertFalse(activeClusterIdFileExists()); + + // Verify read-only coprocessors are loaded + assertTrue(CoprocessorConfigurationUtil.areReadOnlyCoprocessorsLoaded(master.getConfiguration(), + CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY)); + + // Disable read-only mode (our own cluster, no conflicting active cluster file) + setReadOnlyMode(false); + + // Active cluster file should be recreated + assertTrue(activeClusterIdFileExists()); + + // Verify read-only coprocessors are removed + assertFalse(CoprocessorConfigurationUtil.areReadOnlyCoprocessorsLoaded( + master.getConfiguration(), CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY)); + } + + @Test + public void testRegionCoprocessorsStillLoadWhenReadOnlyTransitionBlocked() throws Exception { + // Create a table so we have a region to work with + TableName tableName = TableName.valueOf("testCoprocessorLoadTable"); + TableDescriptor desc = TableDescriptorBuilder.newBuilder(tableName) + .setColumnFamily(ColumnFamilyDescriptorBuilder.of("cf")).build(); + TEST_UTIL.getAdmin().createTable(desc); + List regions = regionServer.getRegions(tableName); + assertFalse(regions.isEmpty()); + HRegion region = regions.get(0); + + // Enable read-only mode to load ReadOnly coprocessors on the region + Configuration readOnlyConf = new Configuration(conf); + readOnlyConf.setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, true); + region.onConfigurationChange(readOnlyConf); + + // Verify ReadOnly coprocessors are loaded + RegionCoprocessorHost regionCPHost = region.getCoprocessorHost(); + assertNotNull(regionCPHost.findCoprocessor(RegionReadOnlyController.class.getName()), + "RegionReadOnlyController should be loaded after enabling read-only mode"); + + // Simulate another active cluster by writing a foreign cluster ID to the active cluster file + overwriteExistingFile(); + assertTrue(activeClusterIdFileExists()); + + // Build a config that attempts to disable read-only AND adds new coprocessors + Configuration newConf = new Configuration(conf); + newConf.setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, false); + + // Add a system and user region coprocessors + newConf.set(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY, SimpleRegionObserver.class.getName()); + newConf.set(CoprocessorHost.USER_REGION_COPROCESSOR_CONF_KEY, + NoOpScanPolicyObserver.class.getName()); + + // Trigger dynamic configuration change on the region + region.onConfigurationChange(newConf); + + // Verify ReadOnly coprocessors are still loaded since the read-only transition was blocked + regionCPHost = region.getCoprocessorHost(); + assertTrue( + CoprocessorConfigurationUtil.areReadOnlyCoprocessorsLoaded(region.getConfiguration(), + CoprocessorHost.REGION_COPROCESSOR_CONF_KEY), + "ReadOnly coprocessors should remain in the region configuration"); + + // Verify new system and user coprocessors were loaded despite the blocked read-only transition + assertNotNull(regionCPHost.findCoprocessor(SimpleRegionObserver.class.getName()), + "SimpleRegionObserver should be loaded even when read-only transition is blocked"); + assertNotNull(regionCPHost.findCoprocessor(NoOpScanPolicyObserver.class.getName()), + "NoOpScanPolicyObserver should be loaded even when read-only transition is blocked"); + } + + @Test + public void testRegionServerCannotDisableReadOnlyWhenAnotherClusterIsActive() throws Exception { + // First enable read-only mode + setReadOnlyMode(true); + assertFalse(activeClusterIdFileExists()); + + // Write an active cluster file with a different cluster's data + overwriteExistingFile(); + assertTrue(activeClusterIdFileExists()); + + // Attempt to disable read-only mode, but get an exception because another active cluster + // already exists + regionServer.getConfiguration().setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, false); + + // RegionServer's updateConfiguration should throw because another cluster is active + assertThrows(ReadOnlyTransitionException.class, () -> regionServer.updateConfiguration()); + + // Verify read-only coprocessors are still loaded on the region server + assertTrue(CoprocessorConfigurationUtil.areReadOnlyCoprocessorsLoaded( + regionServer.getConfiguration(), CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY)); + } + + @Test + public void testBlockedThenSuccessfulReadOnlyTransition() throws Exception { + // Put cluster in read-only mode with a foreign active cluster file + setReadOnlyMode(true); + assertFalse(activeClusterIdFileExists()); + overwriteExistingFile(); + assertTrue(activeClusterIdFileExists()); + + // Attempt to disable read-only mode. This should fail because another cluster is active. + // We need to run updateConfiguration() to ensure the readOnlyTransitionBlocked boolean in + // HBaseServerBase gets set. + master.getConfiguration().setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, false); + assertThrows(ReadOnlyTransitionException.class, () -> master.updateConfiguration()); + regionServer.getConfiguration().setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, false); + assertThrows(ReadOnlyTransitionException.class, () -> regionServer.updateConfiguration()); + + // Try to create a table. This should fail because the cluster is still in read-only mode + TableName tableName = TableName.valueOf("testBlockedTransitionTable"); + TableDescriptor desc = TableDescriptorBuilder.newBuilder(tableName) + .setColumnFamily(ColumnFamilyDescriptorBuilder.of("cf")).build(); + assertThrows(IOException.class, () -> TEST_UTIL.getAdmin().createTable(desc)); + + // Delete the foreign active cluster file + fs.delete(activeClusterFile, false); + assertFalse(activeClusterIdFileExists()); + + // Disable read-only mode again. This should succeed now that no foreign active cluster file + // exists. We need to run updateConfiguration() to ensure the readOnlyTransitionBlocked boolean + // in HBaseServerBase was reset. + master.getConfiguration().setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, false); + master.updateConfiguration(); + regionServer.getConfiguration().setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, false); + regionServer.updateConfiguration(); + + // Verify the active.cluster.suffix.id file contains this cluster's ID + assertTrue(activeClusterIdFileExists()); + try (FSDataInputStream in = fs.open(activeClusterFile)) { + ActiveClusterSuffix actual = ActiveClusterSuffix.parseFrom(in.readAllBytes()); + ActiveClusterSuffix expected = ActiveClusterSuffix.fromConfig(conf, mfs.getClusterId()); + assertEquals(expected, actual); + } + + // Create a table and add a row to verify read-only mode has been disabled + TEST_UTIL.getAdmin().createTable(desc); + try (Table table = TEST_UTIL.getConnection().getTable(tableName)) { + Put put = new Put(Bytes.toBytes("row1")); + put.addColumn(Bytes.toBytes("cf"), Bytes.toBytes("q1"), Bytes.toBytes("value1")); + table.put(put); + } + } } From 1cee44f479a977f81e21fab057d6a83a677428bc Mon Sep 17 00:00:00 2001 From: Kevin Geiszler Date: Fri, 10 Jul 2026 12:37:16 -0400 Subject: [PATCH 2/3] Change readOnlyTransitionBlocked from volatile boolean to AtomicBoolean in HBaseServerBase Change-Id: I2794ed6ce5e6ea3665d55185c0938fa430525f85 --- .../java/org/apache/hadoop/hbase/HBaseServerBase.java | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java index 12bd126ddaa1..f38d82ecccb7 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java @@ -109,7 +109,7 @@ public abstract class HBaseServerBase> extends // Flag set when a read-only to read-write transition is blocked because another active cluster // exists - protected volatile boolean readOnlyTransitionBlocked = false; + protected AtomicBoolean readOnlyTransitionBlocked; // Only for testing private boolean isShutdownHookInstalled = false; @@ -644,7 +644,7 @@ public void updateConfiguration() throws IOException { LOG.info("Reloading the configuration from disk."); // Reload the configuration from disk. preUpdateConfiguration(); - this.readOnlyTransitionBlocked = false; + this.readOnlyTransitionBlocked.set(false); conf.reloadConfiguration(); configurationManager.notifyAllObservers(conf); this.checkForBlockedReadOnlyTransition(); @@ -656,12 +656,12 @@ protected Configuration blockReadOnlyTransition(Configuration updatedConf) { "Cannot disable read-only mode. The {} file contains a different cluster ID, which means " + "that cluster is already the active cluster. Reverting {} to true", HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME, HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY); - this.readOnlyTransitionBlocked = true; + this.readOnlyTransitionBlocked.set(true); return ConfigurationUtil.getReadOnlyEnabledConfigurationCopy(updatedConf); } protected void checkForBlockedReadOnlyTransition() throws ReadOnlyTransitionException { - if (this.readOnlyTransitionBlocked) { + if (this.readOnlyTransitionBlocked.get()) { throw new ReadOnlyTransitionException( "Cannot disable read-only mode because another active cluster already exists on this " + "storage location. The read-only coprocessors have not been removed."); From 59f8783ed16203e4fde2fbcd300fe8ee304d6e51 Mon Sep 17 00:00:00 2001 From: Kevin Geiszler Date: Fri, 10 Jul 2026 14:40:16 -0400 Subject: [PATCH 3/3] Encapsulate active cluster ID in ReadOnlyTransitionException Change-Id: Iecf2e678787193e9faabeb55ba21ebcc37a52808 --- .../hbase/ReadOnlyTransitionException.java | 28 +++++++++++++++++++ .../apache/hadoop/hbase/HBaseServerBase.java | 16 +++++++++-- .../apache/hadoop/hbase/master/HMaster.java | 6 +++- .../hadoop/hbase/regionserver/HRegion.java | 5 ++-- .../hbase/regionserver/HRegionServer.java | 4 ++- .../org/apache/hadoop/hbase/util/FSUtils.java | 16 +++++++++++ 6 files changed, 68 insertions(+), 7 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/ReadOnlyTransitionException.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/ReadOnlyTransitionException.java index 71b25198073c..1f97cd93c70b 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/ReadOnlyTransitionException.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/ReadOnlyTransitionException.java @@ -28,8 +28,11 @@ public class ReadOnlyTransitionException extends DoNotRetryIOException { private static final long serialVersionUID = 1L; + private final String activeClusterId; + public ReadOnlyTransitionException() { super(); + this.activeClusterId = null; } /** @@ -37,6 +40,16 @@ public ReadOnlyTransitionException() { */ public ReadOnlyTransitionException(String message) { super(message); + this.activeClusterId = null; + } + + /** + * @param message the message for this exception + * @param activeClusterId the ID of the cluster that is currently active + */ + public ReadOnlyTransitionException(String message, String activeClusterId) { + super(message); + this.activeClusterId = activeClusterId; } /** @@ -45,6 +58,7 @@ public ReadOnlyTransitionException(String message) { */ public ReadOnlyTransitionException(String message, Throwable throwable) { super(message, throwable); + this.activeClusterId = null; } /** @@ -52,5 +66,19 @@ public ReadOnlyTransitionException(String message, Throwable throwable) { */ public ReadOnlyTransitionException(Throwable throwable) { super(throwable); + this.activeClusterId = null; + } + + /** Returns the ID of the cluster that is currently active, or null if unknown */ + public String getActiveClusterId() { + return activeClusterId; + } + + @Override + public String getMessage() { + if (activeClusterId != null) { + return super.getMessage() + " Current active cluster ID: " + activeClusterId; + } + return super.getMessage(); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java index f38d82ecccb7..a786bbde18ba 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java @@ -32,6 +32,7 @@ import java.net.InetAddress; import java.net.InetSocketAddress; import java.util.concurrent.atomic.AtomicBoolean; +import java.util.concurrent.atomic.AtomicReference; import javax.servlet.http.HttpServlet; import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.SystemUtils; @@ -109,7 +110,10 @@ public abstract class HBaseServerBase> extends // Flag set when a read-only to read-write transition is blocked because another active cluster // exists - protected AtomicBoolean readOnlyTransitionBlocked; + protected final AtomicBoolean readOnlyTransitionBlocked; + + // Tracks the active cluster in a read-replica setup when a ReadOnlyTransitionException occurs + private final AtomicReference blockingActiveClusterId; // Only for testing private boolean isShutdownHookInstalled = false; @@ -254,6 +258,8 @@ protected final void initializeFileSystem() throws IOException { public HBaseServerBase(Configuration conf, String name) throws IOException { super(name); // thread name + this.readOnlyTransitionBlocked = new AtomicBoolean(false); + this.blockingActiveClusterId = new AtomicReference<>(null); final Span span = TraceUtil.createSpan("HBaseServerBase.cxtor"); try (Scope ignored = span.makeCurrent()) { this.conf = conf; @@ -645,18 +651,21 @@ public void updateConfiguration() throws IOException { // Reload the configuration from disk. preUpdateConfiguration(); this.readOnlyTransitionBlocked.set(false); + this.blockingActiveClusterId.set(null); conf.reloadConfiguration(); configurationManager.notifyAllObservers(conf); this.checkForBlockedReadOnlyTransition(); postUpdateConfiguration(); } - protected Configuration blockReadOnlyTransition(Configuration updatedConf) { + protected Configuration blockReadOnlyTransition(Configuration updatedConf, + String activeClusterId) { LOG.error( "Cannot disable read-only mode. The {} file contains a different cluster ID, which means " + "that cluster is already the active cluster. Reverting {} to true", HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME, HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY); this.readOnlyTransitionBlocked.set(true); + this.blockingActiveClusterId.set(activeClusterId); return ConfigurationUtil.getReadOnlyEnabledConfigurationCopy(updatedConf); } @@ -664,7 +673,8 @@ protected void checkForBlockedReadOnlyTransition() throws ReadOnlyTransitionExce if (this.readOnlyTransitionBlocked.get()) { throw new ReadOnlyTransitionException( "Cannot disable read-only mode because another active cluster already exists on this " - + "storage location. The read-only coprocessors have not been removed."); + + "storage location. The read-only coprocessors have not been removed.", + this.blockingActiveClusterId.get()); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java index b392398b9082..eb69d790b572 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java @@ -66,6 +66,7 @@ import org.apache.hadoop.fs.FSDataInputStream; import org.apache.hadoop.fs.FSDataOutputStream; import org.apache.hadoop.fs.Path; +import org.apache.hadoop.hbase.ActiveClusterSuffix; import org.apache.hadoop.hbase.CatalogFamilyFormat; import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellBuilderFactory; @@ -261,6 +262,7 @@ import org.apache.hadoop.hbase.util.DNS; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.FSTableDescriptors; +import org.apache.hadoop.hbase.util.FSUtils; import org.apache.hadoop.hbase.util.FutureUtils; import org.apache.hadoop.hbase.util.HBaseFsck; import org.apache.hadoop.hbase.util.HFileArchiveUtil; @@ -4535,8 +4537,10 @@ public void onConfigurationChange(Configuration updatedConf) { AbstractReadOnlyController.isAnotherClusterActive(mfs.getFileSystem(), mfs.getRootDir(), mfs.getActiveClusterSuffix()) ) { + String activeClusterId = FSUtils.getClusterIdFromActiveClusterFile(mfs.getFileSystem(), + mfs.getRootDir()); // Revert read-only mode here - confForCoprocessors = this.blockReadOnlyTransition(updatedConf); + confForCoprocessors = this.blockReadOnlyTransition(updatedConf, activeClusterId); } } diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index de920146ccef..b71c5f8f5e04 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -9017,10 +9017,11 @@ public void onConfigurationChange(Configuration updatedConf) { if (clusterId != null) { ActiveClusterSuffix localSuffix = ActiveClusterSuffix.fromConfig(updatedConf, clusterId); if (AbstractReadOnlyController.isAnotherClusterActive(regionFs, rootDir, localSuffix)) { + String activeClusterId = FSUtils.getClusterIdFromActiveClusterFile(regionFs, rootDir); LOG.error( - "Cannot disable read-only mode for region {}. Another cluster is already " + "Cannot disable read-only mode for region {}. Another cluster with ID {} is already " + "the active cluster on this storage location. Reverting {} to true.", - this, HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY); + this, activeClusterId, HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY); // Revert read-only mode here confForCoprocessors = ConfigurationUtil.getReadOnlyEnabledConfigurationCopy(updatedConf); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java index a7a44dc515db..f66e57480c17 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java @@ -3524,8 +3524,10 @@ public void onConfigurationChange(Configuration updatedConf) { AbstractReadOnlyController.isAnotherClusterActive(getFileSystem(), getDataRootDir(), localSuffix) ) { + String activeClusterId = FSUtils.getClusterIdFromActiveClusterFile(getFileSystem(), + getDataRootDir()); // Revert read-only mode here - confForCoprocessors = this.blockReadOnlyTransition(updatedConf); + confForCoprocessors = this.blockReadOnlyTransition(updatedConf, activeClusterId); } } 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 40c084cbac82..fc7f70d1d023 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 @@ -65,6 +65,7 @@ import org.apache.hadoop.fs.PathFilter; import org.apache.hadoop.fs.StorageType; import org.apache.hadoop.fs.permission.FsPermission; +import org.apache.hadoop.hbase.ActiveClusterSuffix; import org.apache.hadoop.hbase.ClusterIdFile; import org.apache.hadoop.hbase.ClusterIdFileParser; import org.apache.hadoop.hbase.HConstants; @@ -610,6 +611,21 @@ public static T getClusterIdFile(FileSystem fs, Path r return cs; } + public static String getClusterIdFromActiveClusterFile(FileSystem fs, + Path rootDir) { + String activeClusterId = null; + try { + ActiveClusterSuffix active = + FSUtils.getClusterIdFile(fs, rootDir, new ActiveClusterSuffix.Parser()); + if (active != null) { + activeClusterId = active.toString(); + } + } catch (IOException e) { + LOG.debug("Failed to read active cluster ID from file", e); + } + return activeClusterId; + } + private static void rewriteAsPb(final FileSystem fs, final Path rootdir, final Path p, final String fileName, final T cs) throws IOException { // Rewrite the file as pb. Move aside the old one first, write new