ndimiduk commented on code in PR #5593:
URL: https://github.com/apache/hbase/pull/5593#discussion_r1441510222
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java:
##########
@@ -614,11 +616,41 @@ public ConfigurationManager getConfigurationManager() {
/**
* Reload the configuration from disk.
*/
- public void updateConfiguration() {
+ public void updateConfiguration() throws IOException {
Review Comment:
The signal handler should be modified to also go through this method. It is
registered in the inaccurately named `setupWindows` method. But in this case,
there's no user identity to present for permission enforcement. You may have to
explicitly set in place a system user for this call...
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/HBaseServerBase.java:
##########
@@ -614,11 +616,41 @@ public ConfigurationManager getConfigurationManager() {
/**
* Reload the configuration from disk.
*/
- public void updateConfiguration() {
+ public void updateConfiguration() throws IOException {
LOG.info("Reloading the configuration from disk.");
// Reload the configuration from disk.
+ preUpdateConfiguration();
conf.reloadConfiguration();
configurationManager.notifyAllObservers(conf);
+ postUpdateConfiguration();
+ }
+
+ private void preUpdateConfiguration() throws IOException {
+ if (rpcServices instanceof RSRpcServices && rpcServices.server != null) {
+ RSRpcServices rsRpcServices = (RSRpcServices) rpcServices;
+ if (rsRpcServices.server.getRegionServerCoprocessorHost() != null) {
+
rsRpcServices.server.getRegionServerCoprocessorHost().preUpdateConfiguration(conf);
Review Comment:
Reaching for the `cpHost` via the `rpcServices` instance is a little
strange. Should this class instead have an `abstract protected` accessor method
that can be used to retrieve the instance? I guess unless we replicate this
class hierarchy in the CoprocessorHost, we end up doing casts anyway.
Meh.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/MasterObserver.java:
##########
@@ -1873,4 +1874,12 @@ default void
preHasUserPermissions(ObserverContext<MasterCoprocessorEnvironment>
default void
postHasUserPermissions(ObserverContext<MasterCoprocessorEnvironment> ctx,
String userName, List<Permission> permissions) throws IOException {
}
+
+ default void
preUpdateMasterConfiguration(ObserverContext<MasterCoprocessorEnvironment> ctx,
Review Comment:
These methods are exposed to our public API, so a javadoc string would be
appreciated.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/coprocessor/RegionServerObserver.java:
##########
@@ -169,4 +171,22 @@ default void postReplicationSinkBatchMutate(
}
+ default void
preClearRegionBlockCache(ObserverContext<RegionServerCoprocessorEnvironment>
ctx)
Review Comment:
Likewise re: javadocs.
--
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]
