diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java index c1ba9e274adb..c8199058c362 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/CoprocessorHost.java @@ -115,6 +115,19 @@ public Set getCoprocessors() { return returnValue; } + /** + * Get the full class names of all loaded coprocessors. This method returns the complete class + * names including package information, which is useful for precise coprocessor identification and + * comparison. + */ + public Set getCoprocessorClassNames() { + Set returnValue = new TreeSet<>(); + for (E e : coprocEnvironments) { + returnValue.add(e.getInstance().getClass().getName()); + } + return returnValue; + } + /** * Load system coprocessors once only. Read the class names from configuration. Called by * constructor. 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 f9e75a0438fb..074bc319a4c6 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 @@ -1025,7 +1025,7 @@ private void finishActiveMasterInitialization() if (!maintenanceMode) { startupTaskGroup.addTask("Initializing master coprocessors"); setQuotasObserver(conf); - initializeCoprocessorHost(conf); + this.cpHost = new MasterCoprocessorHost(this, conf); } // Checking if meta needs initializing. @@ -4264,11 +4264,11 @@ public void onConfigurationChange(Configuration newConf) { setQuotasObserver(newConf); // update region server coprocessor if the configuration has changed. if ( - CoprocessorConfigurationUtil.checkConfigurationChange(getConfiguration(), newConf, + CoprocessorConfigurationUtil.checkConfigurationChange(this.cpHost, newConf, CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY) && !maintenanceMode ) { LOG.info("Update the master coprocessor(s) because the configuration has changed"); - initializeCoprocessorHost(newConf); + this.cpHost = new MasterCoprocessorHost(this, newConf); } } @@ -4286,9 +4286,4 @@ 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); - } - } 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 ca0e7db716dd..8d56b730e91c 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 @@ -8418,7 +8418,7 @@ public void onConfigurationChange(Configuration conf) { this.storeHotnessProtector.update(conf); // update coprocessorHost if the configuration has changed. if ( - CoprocessorConfigurationUtil.checkConfigurationChange(getReadOnlyConfiguration(), conf, + CoprocessorConfigurationUtil.checkConfigurationChange(this.coprocessorHost, conf, CoprocessorHost.REGION_COPROCESSOR_CONF_KEY, CoprocessorHost.USER_REGION_COPROCESSOR_CONF_KEY) ) { 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 afa47425380c..512e2cb76f2d 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 @@ -3850,7 +3850,7 @@ public void onConfigurationChange(Configuration newConf) { // update region server coprocessor if the configuration has changed. if ( - CoprocessorConfigurationUtil.checkConfigurationChange(getConfiguration(), newConf, + CoprocessorConfigurationUtil.checkConfigurationChange(this.rsHost, newConf, CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY) ) { LOG.info("Update region server coprocessors because the configuration has changed"); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/CoprocessorConfigurationUtil.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/CoprocessorConfigurationUtil.java index 93c88a897717..6fb66489c211 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/CoprocessorConfigurationUtil.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/CoprocessorConfigurationUtil.java @@ -17,11 +17,14 @@ */ package org.apache.hadoop.hbase.util; -import org.apache.commons.lang3.StringUtils; +import java.util.HashSet; +import java.util.Set; import org.apache.hadoop.conf.Configuration; +import org.apache.hadoop.hbase.coprocessor.CoprocessorHost; import org.apache.yetus.audience.InterfaceAudience; import org.apache.hbase.thirdparty.com.google.common.base.Preconditions; +import org.apache.hbase.thirdparty.com.google.common.base.Strings; /** * Helper class for coprocessor host when configuration changes. @@ -32,19 +35,65 @@ public final class CoprocessorConfigurationUtil { private CoprocessorConfigurationUtil() { } - public static boolean checkConfigurationChange(Configuration oldConfig, Configuration newConfig, - String... configurationKey) { + /** + * Check configuration change by comparing current loaded coprocessors with configuration values. + * This method is useful when the configuration object has been updated but we need to determine + * if coprocessor configuration has actually changed compared to what's currently loaded. + * @param coprocessorHost the coprocessor host to check current loaded coprocessors (can be null) + * @param conf the configuration to check + * @param configurationKey the configuration keys to check + * @return true if configuration has changed, false otherwise + */ + public static boolean checkConfigurationChange(CoprocessorHost coprocessorHost, + Configuration conf, String... configurationKey) { Preconditions.checkArgument(configurationKey != null, "Configuration Key(s) must be provided"); - boolean isConfigurationChange = false; + Preconditions.checkArgument(conf != null, "Configuration must be provided"); + + if (coprocessorHost == null) { + // If no coprocessor host exists, check if any coprocessors are now configured + return hasCoprocessorsConfigured(conf, configurationKey); + } + + // Get currently loaded coprocessor class names + Set currentlyLoaded = coprocessorHost.getCoprocessorClassNames(); + + // Get coprocessor class names from configuration + Set configuredClasses = new HashSet<>(); + for (String key : configurationKey) { + String[] classes = conf.getStrings(key); + if (classes != null) { + for (String className : classes) { + // Handle the className|priority|path format + String[] classNameToken = className.split("\\|"); + String actualClassName = classNameToken[0].trim(); + if (!Strings.isNullOrEmpty(actualClassName)) { + configuredClasses.add(actualClassName); + } + } + } + } + + // Compare the two sets + return !currentlyLoaded.equals(configuredClasses); + } + + /** + * Helper method to check if there are any coprocessors configured. + */ + private static boolean hasCoprocessorsConfigured(Configuration conf, String... configurationKey) { + if ( + !conf.getBoolean(CoprocessorHost.COPROCESSORS_ENABLED_CONF_KEY, + CoprocessorHost.DEFAULT_COPROCESSORS_ENABLED) + ) { + return false; + } + for (String key : configurationKey) { - String oldValue = oldConfig.get(key); - String newValue = newConfig.get(key); - // check if the coprocessor key has any difference - if (!StringUtils.equalsIgnoreCase(oldValue, newValue)) { - isConfigurationChange = true; - break; + String[] coprocessors = conf.getStrings(key); + if (coprocessors != null && coprocessors.length > 0) { + return true; } } - return isConfigurationChange; + return false; } } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java index c26dcdd6fe7a..f821a1536a73 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestHRegion.java @@ -7745,26 +7745,25 @@ public void testRegionOnCoprocessorsChange() throws IOException { assertNull(region.getCoprocessorHost()); // set and verify the system coprocessors for region and user region - Configuration newConf = new Configuration(conf); - newConf.set(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY, MetaTableMetrics.class.getName()); - newConf.set(CoprocessorHost.USER_REGION_COPROCESSOR_CONF_KEY, + conf.set(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY, MetaTableMetrics.class.getName()); + conf.set(CoprocessorHost.USER_REGION_COPROCESSOR_CONF_KEY, NoOpRegionCoprocessor.class.getName()); // trigger configuration change - region.onConfigurationChange(newConf); - assertTrue(region.getCoprocessorHost() != null); + region.onConfigurationChange(conf); + assertNotNull(region.getCoprocessorHost()); Set coprocessors = region.getCoprocessorHost().getCoprocessors(); - assertTrue(coprocessors.size() == 2); + assertEquals(2, coprocessors.size()); assertTrue(region.getCoprocessorHost().getCoprocessors() .contains(MetaTableMetrics.class.getSimpleName())); assertTrue(region.getCoprocessorHost().getCoprocessors() .contains(NoOpRegionCoprocessor.class.getSimpleName())); // remove region coprocessor and keep only user region coprocessor - newConf.unset(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY); - region.onConfigurationChange(newConf); - assertTrue(region.getCoprocessorHost() != null); + conf.unset(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY); + region.onConfigurationChange(conf); + assertNotNull(region.getCoprocessorHost()); coprocessors = region.getCoprocessorHost().getCoprocessors(); - assertTrue(coprocessors.size() == 1); + assertEquals(1, coprocessors.size()); assertTrue(region.getCoprocessorHost().getCoprocessors() .contains(NoOpRegionCoprocessor.class.getSimpleName())); } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerOnlineConfigChange.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerOnlineConfigChange.java index 961c08f8a16a..de6f926f71a1 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerOnlineConfigChange.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestRegionServerOnlineConfigChange.java @@ -230,8 +230,10 @@ public void testStoreConfigurationOnlineChange() { @Test public void testCoprocessorConfigurationOnlineChange() { assertNull(rs1.getRegionServerCoprocessorHost().findCoprocessor(JMXListener.class.getName())); - conf.set(CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY, JMXListener.class.getName()); - rs1.getConfigurationManager().notifyAllObservers(conf); + // Update configuration directly to simulate dynamic configuration reload + Configuration rsConf = rs1.getConfiguration(); + rsConf.set(CoprocessorHost.REGIONSERVER_COPROCESSOR_CONF_KEY, JMXListener.class.getName()); + rs1.getConfigurationManager().notifyAllObservers(rsConf); assertNotNull( rs1.getRegionServerCoprocessorHost().findCoprocessor(JMXListener.class.getName())); } @@ -239,9 +241,11 @@ public void testCoprocessorConfigurationOnlineChange() { @Test public void testCoprocessorConfigurationOnlineChangeOnMaster() { assertNull(hMaster.getMasterCoprocessorHost().findCoprocessor(JMXListener.class.getName())); - conf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, JMXListener.class.getName()); + // Update configuration directly to simulate dynamic configuration reload + Configuration masterConf = hMaster.getConfiguration(); + masterConf.set(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY, JMXListener.class.getName()); assertFalse(hMaster.isInMaintenanceMode()); - hMaster.getConfigurationManager().notifyAllObservers(conf); + hMaster.getConfigurationManager().notifyAllObservers(masterConf); assertNotNull(hMaster.getMasterCoprocessorHost().findCoprocessor(JMXListener.class.getName())); }