Skip to content

Commit d5f43b0

Browse files
kgeiszkgeisz
committed
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
1 parent f1023e6 commit d5f43b0

8 files changed

Lines changed: 427 additions & 18 deletions

File tree

‎hbase-client/src/main/java/org/apache/hadoop/hbase/ReadOnlyTransitionException.java‎

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,56 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing, software
13+
* distributed under the License is distributed on an "AS IS" BASIS,
14+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
15+
* See the License for the specific language governing permissions and
16+
* limitations under the License.
17+
*/
18+
package org.apache.hadoop.hbase;
19+
20+
import org.apache.yetus.audience.InterfaceAudience;
21+
22+
/**
23+
* Thrown when a replica cluster attempts to disable read-only mode while another active cluster
24+
* already exists on the same storage location.
25+
*/
26+
@InterfaceAudience.Public
27+
public class ReadOnlyTransitionException extends DoNotRetryIOException {
28+
29+
private static final long serialVersionUID = 1L;
30+
31+
public ReadOnlyTransitionException() {
32+
super();
33+
}
34+
35+
/**
36+
* @param message the message for this exception
37+
*/
38+
public ReadOnlyTransitionException(String message) {
39+
super(message);
40+
}
41+
42+
/**
43+
* @param message the message for this exception
44+
* @param throwable the {@link Throwable} to use for this exception
45+
*/
46+
public ReadOnlyTransitionException(String message, Throwable throwable) {
47+
super(message, throwable);
48+
}
49+
50+
/**
51+
* @param throwable the {@link Throwable} to use for this exception
52+
*/
53+
public ReadOnlyTransitionException(Throwable throwable) {
54+
super(throwable);
55+
}
56+
}

‎hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,7 @@
7373
import org.apache.hadoop.hbase.unsafe.HBasePlatformDependent;
7474
import org.apache.hadoop.hbase.util.Addressing;
7575
import org.apache.hadoop.hbase.util.CommonFSUtils;
76+
import org.apache.hadoop.hbase.util.ConfigurationUtil;
7677
import org.apache.hadoop.hbase.util.DNS;
7778
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
7879
import org.apache.hadoop.hbase.util.FSTableDescriptors;
@@ -102,15 +103,19 @@ public abstract class HBaseServerBase<R extends HBaseRpcServicesBase<?>> extends
102103
protected final AtomicBoolean abortRequested = new AtomicBoolean(false);
103104

104105
// Set when a report to the master comes back with a message asking us to
105-
// shutdown. Also set by call to stop when debugging or running unit tests
106+
// shut down. Also set by call to stop when debugging or running unit tests
106107
// of HRegionServer in isolation.
107108
protected volatile boolean stopped = false;
108109

110+
// Flag set when a read-only to read-write transition is blocked because another active cluster
111+
// exists
112+
protected volatile boolean readOnlyTransitionBlocked = false;
113+
109114
// Only for testing
110115
private boolean isShutdownHookInstalled = false;
111116

112117
/**
113-
* This servers startcode.
118+
* This server's startcode.
114119
*/
115120
protected final long startcode;
116121

@@ -639,11 +644,30 @@ public void updateConfiguration() throws IOException {
639644
LOG.info("Reloading the configuration from disk.");
640645
// Reload the configuration from disk.
641646
preUpdateConfiguration();
647+
this.readOnlyTransitionBlocked = false;
642648
conf.reloadConfiguration();
643649
configurationManager.notifyAllObservers(conf);
650+
this.checkForBlockedReadOnlyTransition();
644651
postUpdateConfiguration();
645652
}
646653

654+
protected Configuration blockReadOnlyTransition(Configuration updatedConf) {
655+
LOG.error(
656+
"Cannot disable read-only mode. The {} file contains a different cluster ID, which means "
657+
+ "that cluster is already the active cluster. Reverting {} to true",
658+
HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME, HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY);
659+
this.readOnlyTransitionBlocked = true;
660+
return ConfigurationUtil.getReadOnlyEnabledConfigurationCopy(updatedConf);
661+
}
662+
663+
protected void checkForBlockedReadOnlyTransition() throws ReadOnlyTransitionException {
664+
if (this.readOnlyTransitionBlocked) {
665+
throw new ReadOnlyTransitionException(
666+
"Cannot disable read-only mode because another active cluster already exists on this "
667+
+ "storage location. The read-only coprocessors have not been removed.");
668+
}
669+
}
670+
647671
@Override
648672
public KeyManagementService getKeyManagementService() {
649673
return this;

‎hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java‎

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4520,11 +4520,31 @@ public void onConfigurationChange(Configuration updatedConf) {
45204520

45214521
boolean originalIsReadOnlyEnabled = CoprocessorConfigurationUtil
45224522
.areReadOnlyCoprocessorsLoaded(this.conf, CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY);
4523+
boolean newReadOnlyEnabled = ConfigurationUtil.isReadOnlyModeEnabledInConf(updatedConf);
45234524

4524-
// updatedConf and this.conf reference the same Configuration object in an actual HBase
4525-
// deployment. However, in unit test cases they reference different Configuration objects, so
4526-
// this.conf needs to be updated.
4527-
CoprocessorConfigurationUtil.maybeUpdateCoprocessors(updatedConf, this.conf,
4525+
// The updatedConf is potentially a shared Configuration object, so we do not want to directly
4526+
// revert its read-only value if another active cluster already exists. For now, we reference
4527+
// updatedConf and create a copy for modification below if necessary.
4528+
Configuration confForCoprocessors = updatedConf;
4529+
4530+
if (originalIsReadOnlyEnabled && !newReadOnlyEnabled) {
4531+
// Changing this cluster from a replica to an active cluster. There should not be another
4532+
// active cluster already.
4533+
MasterFileSystem mfs = this.getMasterFileSystem();
4534+
if (
4535+
AbstractReadOnlyController.isAnotherClusterActive(mfs.getFileSystem(), mfs.getRootDir(),
4536+
mfs.getActiveClusterSuffix())
4537+
) {
4538+
// Revert read-only mode here
4539+
confForCoprocessors = this.blockReadOnlyTransition(updatedConf);
4540+
}
4541+
}
4542+
4543+
// In a real HBase deployment, confForCoprocessors may reference the same object as this.conf.
4544+
// This is assuming confForCoprocessors still references updatedConf, as mentioned in a previous
4545+
// comment. For unit tests, this Configuration object is not shared, so we need to make sure to
4546+
// update the coprocessors specifically for this.conf.
4547+
CoprocessorConfigurationUtil.maybeUpdateCoprocessors(confForCoprocessors, this.conf,
45284548
originalIsReadOnlyEnabled, this.cpHost, CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY,
45294549
this.maintenanceMode, this.toString(), this::initializeCoprocessorHost);
45304550

‎hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java‎

Lines changed: 41 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -78,12 +78,14 @@
7878
import org.apache.hadoop.fs.FileSystem;
7979
import org.apache.hadoop.fs.LocatedFileStatus;
8080
import org.apache.hadoop.fs.Path;
81+
import org.apache.hadoop.hbase.ActiveClusterSuffix;
8182
import org.apache.hadoop.hbase.Cell;
8283
import org.apache.hadoop.hbase.CellBuilderType;
8384
import org.apache.hadoop.hbase.CellComparator;
8485
import org.apache.hadoop.hbase.CellComparatorImpl;
8586
import org.apache.hadoop.hbase.CellScanner;
8687
import org.apache.hadoop.hbase.CellUtil;
88+
import org.apache.hadoop.hbase.ClusterId;
8789
import org.apache.hadoop.hbase.CompareOperator;
8890
import org.apache.hadoop.hbase.CompoundConfiguration;
8991
import org.apache.hadoop.hbase.DoNotRetryIOException;
@@ -172,6 +174,7 @@
172174
import org.apache.hadoop.hbase.replication.ReplicationUtils;
173175
import org.apache.hadoop.hbase.replication.regionserver.ReplicationObserver;
174176
import org.apache.hadoop.hbase.security.User;
177+
import org.apache.hadoop.hbase.security.access.AbstractReadOnlyController;
175178
import org.apache.hadoop.hbase.snapshot.SnapshotDescriptionUtils;
176179
import org.apache.hadoop.hbase.snapshot.SnapshotManifest;
177180
import org.apache.hadoop.hbase.trace.TraceUtil;
@@ -8997,20 +9000,54 @@ public void onConfigurationChange(Configuration updatedConf) {
89979000

89989001
boolean originalIsReadOnlyEnabled = CoprocessorConfigurationUtil
89999002
.areReadOnlyCoprocessorsLoaded(this.conf, CoprocessorHost.REGION_COPROCESSOR_CONF_KEY);
9003+
boolean newReadOnlyEnabled = ConfigurationUtil.isReadOnlyModeEnabledInConf(updatedConf);
9004+
9005+
// The updatedConf is potentially a shared Configuration object, so we do not want to directly
9006+
// revert its read-only value if another active cluster already exists. For now, we reference
9007+
// updatedConf and create a copy for modification below if necessary.
9008+
Configuration confForCoprocessors = updatedConf;
9009+
9010+
if (originalIsReadOnlyEnabled && !newReadOnlyEnabled) {
9011+
// Changing this cluster from a replica to an active cluster. There should not be another
9012+
// active cluster already.
9013+
try {
9014+
FileSystem regionFs = getFilesystem();
9015+
Path rootDir = CommonFSUtils.getRootDir(this.conf);
9016+
ClusterId clusterId = FSUtils.getClusterIdFile(regionFs, rootDir, new ClusterId.Parser());
9017+
if (clusterId != null) {
9018+
ActiveClusterSuffix localSuffix = ActiveClusterSuffix.fromConfig(updatedConf, clusterId);
9019+
if (AbstractReadOnlyController.isAnotherClusterActive(regionFs, rootDir, localSuffix)) {
9020+
LOG.error(
9021+
"Cannot disable read-only mode for region {}. Another cluster is already "
9022+
+ "the active cluster on this storage location. Reverting {} to true.",
9023+
this, HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY);
9024+
// Revert read-only mode here
9025+
confForCoprocessors =
9026+
ConfigurationUtil.getReadOnlyEnabledConfigurationCopy(updatedConf);
9027+
newReadOnlyEnabled = true;
9028+
}
9029+
}
9030+
} catch (IOException e) {
9031+
LOG.error("Failed to check active cluster status for region {}. "
9032+
+ "Blocking read-only mode transition to prevent potential data corruption.", this, e);
9033+
// Revert read-only mode here
9034+
confForCoprocessors = ConfigurationUtil.getReadOnlyEnabledConfigurationCopy(updatedConf);
9035+
newReadOnlyEnabled = true;
9036+
}
9037+
}
90009038

90019039
// HRegion's this.conf is a special Configuration type called CompoundConfiguration. This means
9002-
// we don't want to use the updatedConf provided in onConfigurationChange() for creating a new
9040+
// we don't want to use the confForCoprocessors Configuration for creating a new
90039041
// RegionCoprocessorHost. Instead, we update this.conf and use that for decorating the region
90049042
// config and updating this.coprocessorHost.
9005-
CoprocessorConfigurationUtil.maybeUpdateCoprocessors(updatedConf, this.conf,
9043+
CoprocessorConfigurationUtil.maybeUpdateCoprocessors(confForCoprocessors, this.conf,
90069044
originalIsReadOnlyEnabled, this.coprocessorHost, CoprocessorHost.REGION_COPROCESSOR_CONF_KEY,
90079045
false, this.toString(), conf -> {
90089046
decorateRegionConfiguration(conf);
90099047
this.coprocessorHost = new RegionCoprocessorHost(this, rsServices, conf);
90109048
});
90119049

9012-
boolean newReadOnlyEnabled = ConfigurationUtil.isReadOnlyModeEnabledInConf(updatedConf);
9013-
9050+
// Changing this cluster from a replica to an active cluster
90149051
if (originalIsReadOnlyEnabled && !newReadOnlyEnabled) {
90159052
LOG.info("Cluster Read Only mode disabled");
90169053
for (HStore store : stores.values()) {

‎hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegionServer.java‎

Lines changed: 30 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -74,9 +74,11 @@
7474
import org.apache.hadoop.fs.FileSystem;
7575
import org.apache.hadoop.fs.Path;
7676
import org.apache.hadoop.hbase.Abortable;
77+
import org.apache.hadoop.hbase.ActiveClusterSuffix;
7778
import org.apache.hadoop.hbase.CacheEvictionStats;
7879
import org.apache.hadoop.hbase.CallQueueTooBigException;
7980
import org.apache.hadoop.hbase.ClockOutOfSyncException;
81+
import org.apache.hadoop.hbase.ClusterId;
8082
import org.apache.hadoop.hbase.DoNotRetryIOException;
8183
import org.apache.hadoop.hbase.ExecutorStatusChore;
8284
import org.apache.hadoop.hbase.HBaseConfiguration;
@@ -155,9 +157,11 @@
155157
import org.apache.hadoop.hbase.security.Superusers;
156158
import org.apache.hadoop.hbase.security.User;
157159
import org.apache.hadoop.hbase.security.UserProvider;
160+
import org.apache.hadoop.hbase.security.access.AbstractReadOnlyController;
158161
import org.apache.hadoop.hbase.trace.TraceUtil;
159162
import org.apache.hadoop.hbase.util.Bytes;
160163
import org.apache.hadoop.hbase.util.CompressionTest;
164+
import org.apache.hadoop.hbase.util.ConfigurationUtil;
161165
import org.apache.hadoop.hbase.util.CoprocessorConfigurationUtil;
162166
import org.apache.hadoop.hbase.util.DNS;
163167
import org.apache.hadoop.hbase.util.EnvironmentEdgeManager;
@@ -315,7 +319,6 @@ public class HRegionServer extends HBaseServerBase<RSRpcServices>
315319
private LeaseManager leaseManager;
316320

317321
private volatile boolean dataFsOk;
318-
private volatile boolean isGlobalReadOnlyEnabled;
319322

320323
static final String ABORT_TIMEOUT = "hbase.regionserver.abort.timeout";
321324
// Default abort timeout is 1200 seconds for safe
@@ -3505,11 +3508,32 @@ public void onConfigurationChange(Configuration updatedConf) {
35053508

35063509
boolean originalIsReadOnlyEnabled = CoprocessorConfigurationUtil
35073510
.areReadOnlyCoprocessorsLoaded(this.conf, CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY);
3508-
3509-
// updatedConf and this.conf reference the same Configuration object in an actual HBase
3510-
// deployment. However, in unit test cases they reference different Configuration objects, so
3511-
// this.conf needs to be updated.
3512-
CoprocessorConfigurationUtil.maybeUpdateCoprocessors(updatedConf, this.conf,
3511+
boolean newReadOnlyEnabled = ConfigurationUtil.isReadOnlyModeEnabledInConf(updatedConf);
3512+
3513+
// The updatedConf is potentially a shared Configuration object, so we do not want to directly
3514+
// revert its read-only value if another active cluster already exists. For now, we reference
3515+
// updatedConf and create a copy for modification below if necessary.
3516+
Configuration confForCoprocessors = updatedConf;
3517+
3518+
if (originalIsReadOnlyEnabled && !newReadOnlyEnabled) {
3519+
// Changing this cluster from a replica to an active cluster. There should not be another
3520+
// active cluster already.
3521+
ActiveClusterSuffix localSuffix =
3522+
ActiveClusterSuffix.fromConfig(this.conf, new ClusterId(getClusterId()));
3523+
if (
3524+
AbstractReadOnlyController.isAnotherClusterActive(getFileSystem(), getDataRootDir(),
3525+
localSuffix)
3526+
) {
3527+
// Revert read-only mode here
3528+
confForCoprocessors = this.blockReadOnlyTransition(updatedConf);
3529+
}
3530+
}
3531+
3532+
// In a real HBase deployment, confForCoprocessors may reference the same object as this.conf.
3533+
// This is assuming confForCoprocessors still references updatedConf, as mentioned in a previous
3534+
// comment. For unit tests, this Configuration object is not shared, so we need to make sure to
3535+
// update the coprocessors specifically for this.conf.
3536+
CoprocessorConfigurationUtil.maybeUpdateCoprocessors(confForCoprocessors, this.conf,
35133537
originalIsReadOnlyEnabled, this.rsHost, CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY,
35143538
false, this.toString(), conf -> this.rsHost = new RegionServerCoprocessorHost(this, conf));
35153539
}

‎hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java‎

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,33 @@ public void start(CoprocessorEnvironment env) throws IOException {
6868
public void stop(CoprocessorEnvironment env) {
6969
}
7070

71+
/**
72+
* Checks whether another cluster is currently active on this storage location by reading the
73+
* {@value HConstants#ACTIVE_CLUSTER_SUFFIX_FILE_NAME} file on the filesystem.
74+
* @param fs the filesystem to read from
75+
* @param rootDir the HBase root directory
76+
* @param localClusterSuffix the local cluster's ActiveClusterSuffix identity
77+
* @return true if the active cluster file exists and belongs to a different cluster; false if the
78+
* file does not exist or belongs to this cluster
79+
*/
80+
public static boolean isAnotherClusterActive(FileSystem fs, Path rootDir,
81+
ActiveClusterSuffix localClusterSuffix) {
82+
try {
83+
ActiveClusterSuffix fileData =
84+
FSUtils.getClusterIdFile(fs, rootDir, new ActiveClusterSuffix.Parser());
85+
if (fileData == null) {
86+
return false;
87+
}
88+
return !localClusterSuffix.equals(fileData);
89+
} catch (IOException e) {
90+
LOG.error(
91+
"Failed to read active cluster suffix file from {} at {}. "
92+
+ "Assuming another cluster is active to prevent potential data corruption.",
93+
HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME, rootDir, e);
94+
return true;
95+
}
96+
}
97+
7198
public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFileSystem mfs) {
7299
FileSystem fs = mfs.getFileSystem();
73100
Path rootDir = mfs.getRootDir();
@@ -110,8 +137,23 @@ public static void manageActiveClusterIdFile(boolean readOnlyEnabled, MasterFile
110137
FSUtils.setClusterIdFile(fs, rootDir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME,
111138
mfs.getActiveClusterSuffix(), wait);
112139
} else {
113-
LOG.debug("Active cluster file already exists at: {}. No need to create it again.",
114-
activeClusterFile);
140+
try (FSDataInputStream in = fs.open(activeClusterFile)) {
141+
ActiveClusterSuffix existingData = ActiveClusterSuffix.parseFrom(in.readAllBytes());
142+
ActiveClusterSuffix localData = mfs.getActiveClusterSuffix();
143+
if (localData.equals(existingData)) {
144+
LOG.debug("Active cluster file already exists at {} and belongs to this cluster. "
145+
+ "No need to create it again.", activeClusterFile);
146+
} else {
147+
LOG.error(
148+
"Active cluster file at {} belongs to a different cluster. "
149+
+ "ID from active cluster file: {}, ID of this cluster: {}. "
150+
+ "Another cluster is already the active cluster.",
151+
activeClusterFile, existingData, localData);
152+
}
153+
} catch (DeserializationException e) {
154+
LOG.error("Failed to deserialize ActiveClusterSuffix from file {}", activeClusterFile,
155+
e);
156+
}
115157
}
116158
}
117159
} catch (IOException e) {

‎hbase-server/src/main/java/org/apache/hadoop/hbase/util/ConfigurationUtil.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,8 +116,22 @@ public static List<Map.Entry<String, String>> getKeyValues(Configuration conf, S
116116
return rtn;
117117
}
118118

119+
/**
120+
* Returns true if the provided Configuration object has
121+
* {@link HConstants#HBASE_GLOBAL_READONLY_ENABLED_KEY} set to true; false otherwise.
122+
*/
119123
public static boolean isReadOnlyModeEnabledInConf(Configuration conf) {
120124
return conf.getBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY,
121125
HConstants.HBASE_GLOBAL_READONLY_ENABLED_DEFAULT);
122126
}
127+
128+
/**
129+
* Returns a copied version of the provided Configuration object that has
130+
* {@link HConstants#HBASE_GLOBAL_READONLY_ENABLED_KEY} set to true.
131+
*/
132+
public static Configuration getReadOnlyEnabledConfigurationCopy(Configuration conf) {
133+
Configuration readOnlyConf = new Configuration(conf);
134+
readOnlyConf.setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, true);
135+
return readOnlyConf;
136+
}
123137
}

0 commit comments

Comments
 (0)