HBASE-26268 Provide coprocessor hooks for updateConfiguration and cle…

…arRegionBlockCache (#5593) Co-authored-by: Charles Connell <[email protected]> Signed-off-by: Nick Dimiduk <[email protected]>
apache · Jan 17, 2024 · 6723b63 · 6723b63
1 parent 9f40f71
commit 6723b63
Show file tree

Hide file tree

Showing 10 changed files with 262 additions and 19 deletions.
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java
@@ -21,6 +21,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import org.apache.hadoop.conf.Configuration;
 import org.apache.hadoop.hbase.ClusterMetrics;
 import org.apache.hadoop.hbase.HBaseInterfaceAudience;
 import org.apache.hadoop.hbase.MetaMutationAnnotation;
@@ -1848,4 +1849,24 @@ default void preUpdateRSGroupConfig(final ObserverContext<MasterCoprocessorEnvir
   default void postUpdateRSGroupConfig(final ObserverContext<MasterCoprocessorEnvironment> ctx,
     final String groupName, final Map<String, String> configuration) throws IOException {
   }
+
+  /*
+   * Called before reloading the HMaster's {@link Configuration} from disk
+   * @param ctx the coprocessor instance's environment
+   * @param preReloadConf the {@link Configuration} in use prior to reload
+   * @throws IOException if you need to signal an IO error
+   */
+  default void preUpdateMasterConfiguration(ObserverContext<MasterCoprocessorEnvironment> ctx,
+    Configuration preReloadConf) throws IOException {
+  }
+
+  /**
+   * Called after reloading the HMaster's {@link Configuration} from disk
+   * @param ctx            the coprocessor instance's environment
+   * @param postReloadConf the {@link Configuration} that was loaded
+   * @throws IOException if you need to signal an IO error
+   */
+  default void postUpdateMasterConfiguration(ObserverContext<MasterCoprocessorEnvironment> ctx,
+    Configuration postReloadConf) throws IOException {
+  }
 }
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionServerObserver.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionServerObserver.java
@@ -18,6 +18,8 @@
 package org.apache.hadoop.hbase.coprocessor;
 
 import java.io.IOException;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.CacheEvictionStats;
 import org.apache.hadoop.hbase.HBaseInterfaceAudience;
 import org.apache.hadoop.hbase.client.Mutation;
 import org.apache.hadoop.hbase.replication.ReplicationEndpoint;
@@ -166,7 +168,47 @@ default void preReplicationSinkBatchMutate(
   default void postReplicationSinkBatchMutate(
     ObserverContext<RegionServerCoprocessorEnvironment> ctx, AdminProtos.WALEntry walEntry,
     Mutation mutation) throws IOException {
+  }
+
+  /*
+   * Called before clearing the block caches for one or more regions
+   * @param ctx the coprocessor instance's environment
+   * @throws IOException if you need to signal an IO error
+   */
+  default void preClearRegionBlockCache(ObserverContext<RegionServerCoprocessorEnvironment> ctx)
+    throws IOException {
+  }
+
+  /**
+   * Called after clearing the block caches for one or more regions
+   * @param ctx   the coprocessor instance's environment
+   * @param stats statistics about the cache evictions that happened
+   * @throws IOException if you need to signal an IO error
+   */
+  default void postClearRegionBlockCache(ObserverContext<RegionServerCoprocessorEnvironment> ctx,
+    CacheEvictionStats stats) throws IOException {
+  }
 
+  /**
+   * Called before reloading the RegionServer's {@link Configuration} from disk
+   * @param ctx           the coprocessor instance's environment
+   * @param preReloadConf the {@link Configuration} in use prior to reload
+   * @throws IOException if you need to signal an IO error
+   */
+  default void preUpdateRegionServerConfiguration(
+    ObserverContext<RegionServerCoprocessorEnvironment> ctx, Configuration preReloadConf)
+    throws IOException {
+  }
+
+  /**
+   * Called after reloading the RegionServer's {@link Configuration} from disk
+   * @param ctx            the coprocessor instance's environment
+   * @param postReloadConf the {@link Configuration} that was loaded
+   * @throws IOException if you need to signal an IO error
+   */
+  default void postUpdateRegionServerConfiguration(
+    ObserverContext<RegionServerCoprocessorEnvironment> ctx, Configuration postReloadConf)
+    throws IOException {
   }
 
 }
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
@@ -564,6 +564,20 @@ private void registerConfigurationObservers() {
     configurationManager.registerObserver(this);
   }
 
+  @Override
+  protected void preUpdateConfiguration() throws IOException {
+    if (cpHost != null) {
+      cpHost.preUpdateConfiguration(conf);
+    }
+  }
+
+  @Override
+  protected void postUpdateConfiguration() throws IOException {
+    if (cpHost != null) {
+      cpHost.postUpdateConfiguration(conf);
+    }
+  }
+
   // Main run loop. Calls through to the regionserver run loop AFTER becoming active Master; will
   // block in here until then.
   @Override

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/master/MasterCoprocessorHost.java
@@ -1984,4 +1984,22 @@ public void call(MasterObserver observer) throws IOException {
       }
     });
   }
+
+  public void preUpdateConfiguration(Configuration preReloadConf) throws IOException {
+    execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() {
+      @Override
+      public void call(MasterObserver observer) throws IOException {
+        observer.preUpdateMasterConfiguration(this, preReloadConf);
+      }
+    });
+  }
+
+  public void postUpdateConfiguration(Configuration postReloadConf) throws IOException {
+    execOperation(coprocEnvironments.isEmpty() ? null : new MasterObserverOperation() {
+      @Override
+      public void call(MasterObserver observer) throws IOException {
+        observer.postUpdateMasterConfiguration(this, postReloadConf);
+      }
+    });
+  }
 }
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
@@ -707,7 +707,7 @@ public HRegionServer(final Configuration conf) throws IOException {
       initializeFileSystem();
 
       this.configurationManager = new ConfigurationManager();
-      setupWindows(conf, configurationManager);
+      setupSignalHandlers();
 
       // Some unit tests don't need a cluster, so no zookeeper at all
       // Open connection to zookeeper and set primary watcher
@@ -772,14 +772,14 @@ protected String getUseThisHostnameInstead(Configuration conf) throws IOExceptio
     }
   }
 
-  /**
-   * If running on Windows, do windows-specific setup.
-   */
-  private static void setupWindows(final Configuration conf, ConfigurationManager cm) {
+  private void setupSignalHandlers() {
     if (!SystemUtils.IS_OS_WINDOWS) {
       HBasePlatformDependent.handle("HUP", (number, name) -> {
-        conf.reloadConfiguration();
-        cm.notifyAllObservers(conf);
+        try {
+          updateConfiguration();
+        } catch (IOException e) {
+          LOG.error("Problem while reloading configuration", e);
+        }
       });
     }
   }
@@ -3886,11 +3886,25 @@ public TableDescriptors getTableDescriptors() {
   /**
    * Reload the configuration from disk.
    */
-  void updateConfiguration() {
+  void updateConfiguration() throws IOException {
     LOG.info("Reloading the configuration from disk.");
     // Reload the configuration from disk.
+    preUpdateConfiguration();
     conf.reloadConfiguration();
     configurationManager.notifyAllObservers(conf);
+    postUpdateConfiguration();
+  }
+
+  protected void preUpdateConfiguration() throws IOException {
+    if (rsHost != null) {
+      rsHost.preUpdateConfiguration(conf);
+    }
+  }
+
+  protected void postUpdateConfiguration() throws IOException {
+    if (rsHost != null) {
+      rsHost.postUpdateConfiguration(conf);
+    }
   }
 
   CacheEvictionStats clearRegionBlockCache(Region region) {

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java
@@ -3844,19 +3844,26 @@ public GetSpaceQuotaSnapshotsResponse getSpaceQuotaSnapshots(RpcController contr
   @Override
   public ClearRegionBlockCacheResponse clearRegionBlockCache(RpcController controller,
     ClearRegionBlockCacheRequest request) throws ServiceException {
-    rpcPreCheck("clearRegionBlockCache");
-    ClearRegionBlockCacheResponse.Builder builder = ClearRegionBlockCacheResponse.newBuilder();
-    CacheEvictionStatsBuilder stats = CacheEvictionStats.builder();
-    List<HRegion> regions = getRegions(request.getRegionList(), stats);
-    for (HRegion region : regions) {
-      try {
-        stats = stats.append(this.regionServer.clearRegionBlockCache(region));
-      } catch (Exception e) {
-        stats.addException(region.getRegionInfo().getRegionName(), e);
+
+    try {
+      rpcPreCheck("clearRegionBlockCache");
+      ClearRegionBlockCacheResponse.Builder builder = ClearRegionBlockCacheResponse.newBuilder();
+      CacheEvictionStatsBuilder stats = CacheEvictionStats.builder();
+      regionServer.getRegionServerCoprocessorHost().preClearRegionBlockCache();
+      List<HRegion> regions = getRegions(request.getRegionList(), stats);
+      for (HRegion region : regions) {
+        try {
+          stats = stats.append(this.regionServer.clearRegionBlockCache(region));
+        } catch (Exception e) {
+          stats.addException(region.getRegionInfo().getRegionName(), e);
+        }
       }
+      stats.withMaxCacheSize(regionServer.getBlockCache().map(BlockCache::getMaxSize).orElse(0L));
+      regionServer.getRegionServerCoprocessorHost().postClearRegionBlockCache(stats.build());
+      return builder.setStats(ProtobufUtil.toCacheEvictionStats(stats.build())).build();
+    } catch (IOException e) {
+      throw new ServiceException(e);
     }
-    stats.withMaxCacheSize(regionServer.getBlockCache().map(BlockCache::getMaxSize).orElse(0L));
-    return builder.setStats(ProtobufUtil.toCacheEvictionStats(stats.build())).build();
   }
 
   private void executeOpenRegionProcedures(OpenRegionRequest request,

diff --git a/...erver/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java b/...erver/src/main/java/org/apache/hadoop/hbase/regionserver/RegionServerCoprocessorHost.java
@@ -21,6 +21,7 @@
 import java.io.IOException;
 import java.lang.reflect.InvocationTargetException;
 import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.CacheEvictionStats;
 import org.apache.hadoop.hbase.ServerName;
 import org.apache.hadoop.hbase.SharedConnection;
 import org.apache.hadoop.hbase.client.Connection;
@@ -247,6 +248,42 @@ public void call(RegionServerObserver observer) throws IOException {
     });
   }
 
+  public void preUpdateConfiguration(Configuration preReloadConf) throws IOException {
+    execOperation(coprocEnvironments.isEmpty() ? null : new RegionServerObserverOperation() {
+      @Override
+      public void call(RegionServerObserver observer) throws IOException {
+        observer.preUpdateRegionServerConfiguration(this, preReloadConf);
+      }
+    });
+  }
+
+  public void postUpdateConfiguration(Configuration postReloadConf) throws IOException {
+    execOperation(coprocEnvironments.isEmpty() ? null : new RegionServerObserverOperation() {
+      @Override
+      public void call(RegionServerObserver observer) throws IOException {
+        observer.postUpdateRegionServerConfiguration(this, postReloadConf);
+      }
+    });
+  }
+
+  public void preClearRegionBlockCache() throws IOException {
+    execOperation(coprocEnvironments.isEmpty() ? null : new RegionServerObserverOperation() {
+      @Override
+      public void call(RegionServerObserver observer) throws IOException {
+        observer.preClearRegionBlockCache(this);
+      }
+    });
+  }
+
+  public void postClearRegionBlockCache(CacheEvictionStats stats) throws IOException {
+    execOperation(coprocEnvironments.isEmpty() ? null : new RegionServerObserverOperation() {
+      @Override
+      public void call(RegionServerObserver observer) throws IOException {
+        observer.postClearRegionBlockCache(this, stats);
+      }
+    });
+  }
+
   /**
    * Coprocessor environment extension providing access to region server related services.
    */

diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AccessController.java
@@ -2465,4 +2465,26 @@ private void preHasUserPermissions(User caller, String userName, List<Permission
       }
     }
   }
+
+  @Override
+  public void preClearRegionBlockCache(ObserverContext<RegionServerCoprocessorEnvironment> ctx)
+    throws IOException {
+    accessChecker.requirePermission(getActiveUser(ctx), "clearRegionBlockCache", null,
+      Permission.Action.ADMIN);
+  }
+
+  @Override
+  public void preUpdateRegionServerConfiguration(
+    ObserverContext<RegionServerCoprocessorEnvironment> ctx, Configuration preReloadConf)
+    throws IOException {
+    accessChecker.requirePermission(getActiveUser(ctx), "updateConfiguration", null,
+      Permission.Action.ADMIN);
+  }
+
+  @Override
+  public void preUpdateMasterConfiguration(ObserverContext<MasterCoprocessorEnvironment> ctx,
+    Configuration preReloadConf) throws IOException {
+    accessChecker.requirePermission(getActiveUser(ctx), "updateConfiguration", null,
+      Permission.Action.ADMIN);
+  }
 }
diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/coprocessor/TestMasterObserver.java
@@ -191,6 +191,8 @@ public static class CPMasterObserver implements MasterCoprocessor, MasterObserve
     private boolean postLockHeartbeatCalled;
     private boolean preMasterStoreFlushCalled;
     private boolean postMasterStoreFlushCalled;
+    private boolean preUpdateMasterConfigurationCalled;
+    private boolean postUpdateMasterConfigurationCalled;
 
     public void resetStates() {
       preCreateTableRegionInfosCalled = false;
@@ -284,6 +286,8 @@ public void resetStates() {
       postLockHeartbeatCalled = false;
       preMasterStoreFlushCalled = false;
       postMasterStoreFlushCalled = false;
+      preUpdateMasterConfigurationCalled = false;
+      postUpdateMasterConfigurationCalled = false;
     }
 
     @Override
@@ -1261,6 +1265,17 @@ public void postRollBackMergeRegionsAction(
       throws IOException {
     }
 
+    @Override
+    public void preUpdateMasterConfiguration(ObserverContext<MasterCoprocessorEnvironment> ctx,
+      Configuration preReloadConf) throws IOException {
+      preUpdateMasterConfigurationCalled = true;
+    }
+
+    @Override
+    public void postUpdateMasterConfiguration(ObserverContext<MasterCoprocessorEnvironment> ctx,
+      Configuration postReloadConf) throws IOException {
+      postUpdateMasterConfigurationCalled = true;
+    }
   }
 
   private static HBaseTestingUtility UTIL = new HBaseTestingUtility();
@@ -1710,4 +1725,22 @@ public void testMasterStoreOperations() throws Exception {
       assertTrue("Master store flush called", cp.postMasterStoreFlushCalled);
     }
   }
+
+  @Test
+  public void testUpdateConfiguration() throws Exception {
+    HMaster master = UTIL.getMiniHBaseCluster().getMaster();
+    MasterCoprocessorHost host = master.getMasterCoprocessorHost();
+    CPMasterObserver cp = host.findCoprocessor(CPMasterObserver.class);
+    cp.resetStates();
+    assertFalse("No update configuration call", cp.preUpdateMasterConfigurationCalled);
+    assertFalse("No update configuration call", cp.postUpdateMasterConfigurationCalled);
+
+    try (Connection connection = ConnectionFactory.createConnection(UTIL.getConfiguration());
+      Admin admin = connection.getAdmin()) {
+      admin.updateConfiguration();
+
+      assertTrue("Update configuration called", cp.preUpdateMasterConfigurationCalled);
+      assertTrue("Update configuration called", cp.postUpdateMasterConfigurationCalled);
+    }
+  }
 }