Skip to content

HBASE-29642 Active cluster file is not being updated after promoting … #7437

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: HBASE-29081
Choose a base branch
from
+77 −5
Open

HBASE-29642 Active cluster file is not being updated after promoting … #7437

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions ...erver/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterCoprocessorEnvironment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import org.apache.hadoop.hbase.HBaseInterfaceAudience;
import org.apache.hadoop.hbase.ServerName;
import org.apache.hadoop.hbase.client.Connection;
import org.apache.hadoop.hbase.master.MasterServices;
import org.apache.hadoop.hbase.metrics.MetricRegistry;
import org.apache.yetus.audience.InterfaceAudience;
import org.apache.yetus.audience.InterfaceStability;
Expand Down Expand Up @@ -88,4 +89,6 @@ public interface MasterCoprocessorEnvironment extends CoprocessorEnvironment<Mas
* @return A MetricRegistry for the coprocessor class to track and export metrics.
*/
MetricRegistry getMetricRegistryForMaster();

MasterServices getMasterServices();
}
5 changes: 5 additions & 0 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ public MetricRegistry getMetricRegistryForMaster() {
return metricRegistry;
}

@Override
public MasterServices getMasterServices() {
return services;
}

@Override
public void shutdown() {
super.shutdown();
Expand Down
74 changes: 69 additions & 5 deletions hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/ReadOnlyController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,13 @@
package org.apache.hadoop.hbase.security.access;

import java.io.IOException;
import java.nio.charset.StandardCharsets;
import java.util.List;
import java.util.Optional;
import org.apache.hadoop.conf.Configuration;
import org.apache.hadoop.fs.FSDataOutputStream;
import org.apache.hadoop.fs.FileSystem;
import org.apache.hadoop.fs.Path;
import org.apache.hadoop.hbase.CompareOperator;
import org.apache.hadoop.hbase.CoprocessorEnvironment;
import org.apache.hadoop.hbase.HBaseInterfaceAudience;
Expand Down Expand Up @@ -51,6 +55,8 @@
import org.apache.hadoop.hbase.coprocessor.RegionServerObserver;
import org.apache.hadoop.hbase.filter.ByteArrayComparable;
import org.apache.hadoop.hbase.filter.Filter;
import org.apache.hadoop.hbase.master.MasterFileSystem;
import org.apache.hadoop.hbase.master.MasterServices;
import org.apache.hadoop.hbase.regionserver.FlushLifeCycleTracker;
import org.apache.hadoop.hbase.regionserver.MiniBatchOperationInProgress;
import org.apache.hadoop.hbase.util.Pair;
Expand All @@ -71,6 +77,8 @@ public class ReadOnlyController implements MasterCoprocessor, RegionCoprocessor,
ConfigurationObserver {

private static final Logger LOG = LoggerFactory.getLogger(ReadOnlyController.class);
private MasterServices masterServices;

private volatile boolean globalReadOnlyEnabled;

private void internalReadOnlyGuard() throws IOException {
Expand All @@ -81,6 +89,13 @@ private void internalReadOnlyGuard() throws IOException {

@Override
public void start(CoprocessorEnvironment env) throws IOException {
if (env instanceof MasterCoprocessorEnvironment) {
this.masterServices = ((MasterCoprocessorEnvironment) env).getMasterServices();
LOG.info("ReadOnlyController obtained MasterServices reference from start().");
} else {
LOG.warn("ReadOnlyController loaded in a non-Master environment. "
+ "File system operations for read-only state will not work.");
}
this.globalReadOnlyEnabled =
env.getConfiguration().getBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY,
HConstants.HBASE_GLOBAL_READONLY_ENABLED_DEFAULT);
Expand Down Expand Up @@ -400,14 +415,63 @@ public void preCleanupBulkLoad(ObserverContext<RegionCoprocessorEnvironment> ctx
BulkLoadObserver.super.preCleanupBulkLoad(ctx);
}

private void manageActiveClusterIdFile(boolean newValue) {
MasterFileSystem mfs = this.masterServices.getMasterFileSystem();
FileSystem fs = mfs.getFileSystem();
Path rootDir = mfs.getRootDir();
Path activeClusterFile = new Path(rootDir, HConstants.ACTIVE_CLUSTER_SUFFIX_FILE_NAME);

try {
if (newValue) {
// ENABLING READ-ONLY (false -> true), delete the active cluster file.
LOG.debug("Global read-only mode is being ENABLED. Deleting active cluster file: {}",
activeClusterFile);

if (fs.exists(activeClusterFile)) {
if (fs.delete(activeClusterFile, false)) {
LOG.info("Successfully deleted active cluster file: {}", activeClusterFile);
} else {
LOG.error(
"Failed to delete active cluster file: {}. "
+ "Read-only flag will be updated, but file system state is inconsistent.",
activeClusterFile);
}
} else {
LOG.debug("Active cluster file not present, nothing to delete.");
Copy link
Contributor

@anmolnar anmolnar Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an error situation, right? Because you're transitioning from R/W -> R/O mode and trying to delete the file which should be there. I would just delete it right away an log the error like this:

try {
  fs.delete(activeClusterFile, false));
  LOG.info("Successfully deleted active cluster file: {}", activeClusterFile);
} catch (IOException e) {
  LOG.error(...);
}

}
} else {
// DISABLING READ-ONLY (true -> false), create the active cluster file id file
try (FSDataOutputStream stream = fs.create(activeClusterFile, true)) {
stream.writeUTF(new String(mfs.getSuffixFileDataToWrite(), StandardCharsets.UTF_8));
Copy link
Contributor

@anmolnar anmolnar Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You already have a helper method for this FSUtils.setActiveClusterSuffix(). Why not using it?

LOG.debug(
"Global read-only mode is being DISABLED. Successfully created active "
+ "cluster file {} with suffix {}",
activeClusterFile, mfs.getSuffixFileDataToWrite());
}
}
} catch (IOException e) {
// We still update the flag, but log that the operation failed.
LOG.error("Failed to perform file operation for read-only switch. "
+ "Flag will be updated, but file system state may be inconsistent.", e);
this.globalReadOnlyEnabled = newValue;
LOG.info("Config {} has been dynamically changed to {}. Encountered FS error",
HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, this.globalReadOnlyEnabled);
}
}

/* ---- ConfigurationObserver Overrides ---- */
@Override
public void onConfigurationChange(Configuration conf) {
boolean maybeUpdatedConfValue = conf.getBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY,
boolean newValue = conf.getBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY,
Copy link
Contributor

@anmolnar anmolnar Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: I think the previous name of the variable is fine, no need to rename.

HConstants.HBASE_GLOBAL_READONLY_ENABLED_DEFAULT);
if (this.globalReadOnlyEnabled != maybeUpdatedConfValue) {
this.globalReadOnlyEnabled = maybeUpdatedConfValue;
LOG.info("Config {} has been dynamically changed to {}",
if (this.globalReadOnlyEnabled != newValue) {
if (this.masterServices != null) {
manageActiveClusterIdFile(newValue);
} else {
LOG.debug("MasterServices is not initialized. Cannot perform file operations for "
Copy link
Contributor

@anmolnar anmolnar Nov 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This indicates a problematic situation to me, but it's actually not. I don't think you log anything here, but if you really want to, rephrase to something like 'Global R/O flag changed, but not running on master'

+ "read-only mode switch.");
}
this.globalReadOnlyEnabled = newValue;
LOG.debug("Config {} has been dynamically changed to {}. (No FS ops performed 1)",
HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, this.globalReadOnlyEnabled);
}
}
Expand Down