taklwu commented on a change in pull request #4181:
URL: https://github.com/apache/hbase/pull/4181#discussion_r823250446
##########
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:
I missed that, but I found that `maintenanceMode` should not be in the
picture because `cpHost` only initialized when `maintenanceMode=false`
your second part of question is a harder one, in other words, you are asking
if there is any existing CP that uses a boolean flag instead of using
coprocessor class configuration in hbase-site. if so, how do we handle such
override.
I don't have a good idea but at least for the `MasterQuotasObserver`, I can
handle it by adding block of `updateConfigurationForQuotasObserver` before the
reload of the master coprocessor host.
`RSGroupAdminEndpoint` should be deprecated, and it's a new object
`rsGroupInfoManager` if the coprocessor class name was found as part of the
`MASTER_COPROCESSOR_CONF_KEY` . we can also handle it as part of the reload.
but for a more general CP that has the boolean switch, this change may break
them.
Would you suggest we don't support master coprocessor with dynamic
configuration ?
--
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]
