taklwu commented on a change in pull request #4181:
URL: https://github.com/apache/hbase/pull/4181#discussion_r824224881



##########
File path: 
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java
##########
@@ -4088,6 +4090,12 @@ public void onConfigurationChange(Configuration newConf) 
{
     } catch (IOException e) {
       LOG.warn("Failed to initialize SuperUsers on reloading of the 
configuration");
     }
+    // update region server coprocessor if the configuration has changed.
+    if 
(CoprocessorConfigurationUtil.checkConfigurationChange(getConfiguration(), 
newConf,
+      CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY)) {
+      LOG.info("Update the master coprocessor(s) because the configuration has 
changed");
+      this.cpHost = new MasterCoprocessorHost(this, newConf);
+    }

Review comment:
       after a second thought on the `rsGroupInfoManager`, it's not a 
coprocessor and is instead a core executor within the HMaster. So it should not 
be reloaded when the configuration change. But the deprecated 
`RSGroupAdminEndpoint` (for compatible with old client) could be reloaded as 
usual as other master coprocessor if it's list as part of the 
`MASTER_COPROCESSOR_CONF_KEY`.  (such that the coprocessor configuration only 
works for old hbase client) 
   
   if the user really want to disable `rsGroupInfoManager`, the user will need 
to turn off by setting `hbase.balancer.rsgroup.enabled=false`, then perform a 
hard restart. what do you think? 




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to