anmolnar commented on code in PR #7743:
URL: https://github.com/apache/hbase/pull/7743#discussion_r2800005607
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/security/access/AbstractReadOnlyController.java:
##########
@@ -48,6 +45,19 @@ protected void internalReadOnlyGuard() throws
DoNotRetryIOException {
}
}
+ public void setReadOnlyEnabled(boolean readOnlyEnabledFromConfig) {
+ if (this.globalReadOnlyEnabled != readOnlyEnabledFromConfig) {
+ this.globalReadOnlyEnabled = readOnlyEnabledFromConfig;
+ LOG.info("Updated {} readOnlyEnabled={}", this.getClass().getName(),
+ readOnlyEnabledFromConfig);
+ if (this.masterServices != null) {
+ manageActiveClusterIdFile(readOnlyEnabledFromConfig);
Review Comment:
I think there's an issue with this approach.
1. We talked about briefly that the local `globalReadOnlyEnabled` is not
needed anymore, because coprocs will be loaded only when R/O mode is enabled.
So, I think you should remove the field in this patch.
2. As a consequence this method will always be called with
`readOnlyEnabledFromConfig == true` and the active cluster id file will never
be removed.
I suggest to make this method static and call it from HMaster at line 4440.
You don't need to resolve the coproc, just call the static method directly with
the new configuration and do the management of cluster id file.
--
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]
