HBASE-29236: Add Support for Dynamic Configuration at the Coprocessor Level

hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java

@@ -37,6 +37,8 @@
 import org.apache.hadoop.hbase.CoprocessorEnvironment;
 import org.apache.hadoop.hbase.DoNotRetryIOException;
 import org.apache.hadoop.hbase.HConstants;
+import org.apache.hadoop.hbase.conf.ConfigurationManager;
+import org.apache.hadoop.hbase.conf.ConfigurationObserver;
 import org.apache.hadoop.hbase.ipc.RpcServer;
 import org.apache.hadoop.hbase.security.User;
 import org.apache.hadoop.hbase.util.CoprocessorClassLoader;
@@ -116,6 +118,39 @@ public Set<String> getCoprocessors() {
     return returnValue;
   }
 
+  /**
+   * Used to help make the relevant loaded coprocessors dynamically configurable by registering them
+   * to the {@link ConfigurationManager}. Coprocessors are considered "relevant" if they implement
+   * the {@link ConfigurationObserver} interface.
+   * @param configurationManager the ConfigurationManager the coprocessors get registered to
+   */
+  public void registerConfigurationObservers(ConfigurationManager configurationManager) {
+    Coprocessor foundCp;
+    Set<String> coprocessors = this.getCoprocessors();
+    for (String cp : coprocessors) {
+      foundCp = this.findCoprocessor(cp);
+      if (foundCp instanceof ConfigurationObserver) {
+        configurationManager.registerObserver((ConfigurationObserver) foundCp);
+      }
+    }
+  }
+
+  /**
+   * Deregisters relevant coprocessors from the {@link ConfigurationManager}. Coprocessors are
+   * considered "relevant" if they implement the {@link ConfigurationObserver} interface.
+   * @param configurationManager the ConfigurationManager the coprocessors get deregistered from
+   */
+  public void deregisterConfigurationObservers(ConfigurationManager configurationManager) {
+    Coprocessor foundCp;
+    Set<String> coprocessors = this.getCoprocessors();
+    for (String cp : coprocessors) {
+      foundCp = this.findCoprocessor(cp);
+      if (foundCp instanceof ConfigurationObserver) {
+        configurationManager.deregisterObserver((ConfigurationObserver) foundCp);
+      }
+    }
+  }
+
   /**
    * Load system coprocessors once only. Read the class names from configuration. Called by
    * constructor.

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

@@ -609,6 +609,12 @@ protected String getUseThisHostnameInstead(Configuration conf) {
   private void registerConfigurationObservers() {
     configurationManager.registerObserver(this.rpcServices);
     configurationManager.registerObserver(this);
+    if (cpHost != null) {
+      cpHost.registerConfigurationObservers(configurationManager);
+    } else {
+      LOG.warn("Could not register HMaster coprocessors to the ConfigurationManager because "
+        + "MasterCoprocessorHost is null");
+    }
   }
 
   // Main run loop. Calls through to the regionserver run loop AFTER becoming active Master; will
@@ -617,7 +623,6 @@ private void registerConfigurationObservers() {
   public void run() {
     try {
       installShutdownHook();
-      registerConfigurationObservers();
       Threads.setDaemonThreadRunning(new Thread(TraceUtil.tracedRunnable(() -> {
         try {
           int infoPort = putUpJettyServer();
@@ -4506,6 +4511,7 @@ private void setQuotasObserver(Configuration conf) {
   private void initializeCoprocessorHost(Configuration conf) {
     // initialize master side coprocessors before we start handling requests
     this.cpHost = new MasterCoprocessorHost(this, conf);
+    registerConfigurationObservers();
   }
 
   @Override

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

@@ -8832,6 +8832,12 @@ public void onConfigurationChange(Configuration conf) {
   public void registerChildren(ConfigurationManager manager) {
     configurationManager = manager;
     stores.values().forEach(manager::registerObserver);
+    if (coprocessorHost != null) {
+      coprocessorHost.registerConfigurationObservers(manager);
+    } else {
+      LOG.warn("Could not register HRegion coprocessors to the ConfigurationManager because "
+        + "RegionCoprocessorHost is null");
+    }
   }
 
   /**
@@ -8840,6 +8846,12 @@ public void registerChildren(ConfigurationManager manager) {
   @Override
   public void deregisterChildren(ConfigurationManager manager) {
     stores.values().forEach(configurationManager::deregisterObserver);
+    if (coprocessorHost != null) {
+      coprocessorHost.deregisterConfigurationObservers(manager);
+    } else {
+      LOG.warn("Could not deregister HRegion coprocessors from the ConfigurationManager because "
+        + "RegionCoprocessorHost is null");
+    }
   }
 
   @Override

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

@@ -2136,6 +2136,12 @@ private void registerConfigurationObservers() {
     configurationManager.registerObserver(this.rpcServices);
     configurationManager.registerObserver(this.prefetchExecutorNotifier);
     configurationManager.registerObserver(this);
+    if (rsHost != null) {
+      rsHost.registerConfigurationObservers(configurationManager);
+    } else {
+      LOG.warn("Could not register HRegionServer coprocessors to the ConfigurationManager because "
+        + "RegionServerCoprocessorHost is null");
+    }
   }
 
   /*

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

@@ -35,6 +35,7 @@
 import org.apache.hadoop.hbase.client.Result;
 import org.apache.hadoop.hbase.client.SnapshotDescription;
 import org.apache.hadoop.hbase.client.TableDescriptor;
+import org.apache.hadoop.hbase.conf.ConfigurationObserver;
 import org.apache.hadoop.hbase.coprocessor.BulkLoadObserver;
 import org.apache.hadoop.hbase.coprocessor.CoreCoprocessor;
 import org.apache.hadoop.hbase.coprocessor.EndpointObserver;
@@ -65,26 +66,24 @@
 
 @CoreCoprocessor
 @InterfaceAudience.LimitedPrivate(HBaseInterfaceAudience.CONFIG)
-public class ReadOnlyController
-  implements MasterCoprocessor, RegionCoprocessor, MasterObserver, RegionObserver,
-  RegionServerCoprocessor, RegionServerObserver, EndpointObserver, BulkLoadObserver {
+public class ReadOnlyController implements MasterCoprocessor, RegionCoprocessor, MasterObserver,
+  RegionObserver, RegionServerCoprocessor, RegionServerObserver, EndpointObserver, BulkLoadObserver,
+  ConfigurationObserver {
 
   private static final Logger LOG = LoggerFactory.getLogger(ReadOnlyController.class);
-  private Configuration conf;
+  private volatile boolean globalReadOnlyEnabled;
 
   private void internalReadOnlyGuard() throws IOException {
-    if (
-      conf.getBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY,
-        HConstants.HBASE_GLOBAL_READONLY_ENABLED_DEFAULT)
-    ) {
-      // throw new FailedSanityCheckException("Operation not allowed in Read-Only Mode");
+    if (this.globalReadOnlyEnabled) {
       throw new IOException("Operation not allowed in Read-Only Mode");
     }
   }
 
   @Override
   public void start(CoprocessorEnvironment env) throws IOException {
-    conf = env.getConfiguration();
+    this.globalReadOnlyEnabled =
+      env.getConfiguration().getBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY,
+        HConstants.HBASE_GLOBAL_READONLY_ENABLED_DEFAULT);
   }
 
   @Override
@@ -100,7 +99,8 @@ public Optional<RegionObserver> getRegionObserver() {
   @Override
   public void prePut(ObserverContext<? extends RegionCoprocessorEnvironment> c, Put put,
     WALEdit edit) throws IOException {
-    if (edit.isMetaEdit() || edit.isEmpty()) {
+    TableName tableName = c.getEnvironment().getRegionInfo().getTable();
+    if (tableName.isSystemTable()) {
       return;
     }
     internalReadOnlyGuard();
@@ -115,13 +115,11 @@ public void preDelete(ObserverContext<? extends RegionCoprocessorEnvironment> c,
   @Override
   public void preBatchMutate(ObserverContext<? extends RegionCoprocessorEnvironment> c,
     MiniBatchOperationInProgress<Mutation> miniBatchOp) throws IOException {
-    for (int i = 0; i < miniBatchOp.size(); i++) {
-      WALEdit edit = miniBatchOp.getWalEdit(i);
-      if (edit == null || edit.isMetaEdit() || edit.isEmpty()) {
-        continue;
-      }
-      internalReadOnlyGuard();
+    TableName tableName = c.getEnvironment().getRegionInfo().getTable();
+    if (tableName.isSystemTable()) {
+      return;
     }
+    internalReadOnlyGuard();
   }
 
   @Override
@@ -390,4 +388,16 @@ public void preCleanupBulkLoad(ObserverContext<RegionCoprocessorEnvironment> ctx
     internalReadOnlyGuard();
     BulkLoadObserver.super.preCleanupBulkLoad(ctx);
   }
+
+  /* ---- ConfigurationObserver Overrides ---- */
+  @Override
+  public void onConfigurationChange(Configuration conf) {
+    boolean maybeUpdatedConfValue = conf.getBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY,
+      HConstants.HBASE_GLOBAL_READONLY_ENABLED_DEFAULT);
+    if (this.globalReadOnlyEnabled != maybeUpdatedConfValue) {
+      this.globalReadOnlyEnabled = maybeUpdatedConfValue;
+      LOG.info("Config {} has been dynamically changed to {}",
+        HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, this.globalReadOnlyEnabled);
+    }
+  }
 }

hbase-server/src/test/java/org/apache/hadoop/hbase/security/access/TestCanStartHBaseInReadOnlyMode.java

@@ -0,0 +1,74 @@
+/*
+ * 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.security.access;
+
+import static org.apache.hadoop.hbase.HConstants.HBASE_CLIENT_RETRIES_NUMBER;
+
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.*;
+import org.apache.hadoop.hbase.client.*;
+import org.apache.hadoop.hbase.coprocessor.CoprocessorHost;
+import org.apache.hadoop.hbase.testclassification.LargeTests;
+import org.apache.hadoop.hbase.testclassification.SecurityTests;
+import org.junit.*;
+import org.junit.experimental.categories.Category;
+import org.junit.rules.TestName;
+
+@Category({ SecurityTests.class, LargeTests.class })
+@SuppressWarnings("deprecation")
+public class TestCanStartHBaseInReadOnlyMode {
+
+  @ClassRule
+  public static final HBaseClassTestRule CLASS_RULE =
+    HBaseClassTestRule.forClass(TestCanStartHBaseInReadOnlyMode.class);
+
+  private static final String READ_ONLY_CONTROLLER_NAME = ReadOnlyController.class.getName();
+  private static final HBaseTestingUtil TEST_UTIL = new HBaseTestingUtil();
+  private static Configuration conf;
+
+  @Rule
+  public TestName name = new TestName();
+
+  @BeforeClass
+  public static void beforeClass() throws Exception {
+    conf = TEST_UTIL.getConfiguration();
+
+    // Shorten the run time of failed unit tests by limiting retries and the session timeout
+    // threshold
+    conf.setInt(HBASE_CLIENT_RETRIES_NUMBER, 0);
+    conf.setInt("hbase.master.init.timeout.localHBaseCluster", 10000);
+
+    // Enable Read-Only mode to prove the cluster can be started in this state
+    conf.setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, true);
+
+    // Add ReadOnlyController coprocessors to the master, region server, and regions
+    conf.set(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY, READ_ONLY_CONTROLLER_NAME);
+    conf.set(CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY, READ_ONLY_CONTROLLER_NAME);
+    conf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, READ_ONLY_CONTROLLER_NAME);
+  }
+
+  @AfterClass
+  public static void afterClass() throws Exception {
+    TEST_UTIL.shutdownMiniCluster();
+  }
+
+  @Test
+  public void testCanStartHBaseInReadOnlyMode() throws Exception {
+    TEST_UTIL.startMiniCluster(1);
+  }
+}