HBASE-29756: Programmatically register related co-processor during initialization#7743
HBASE-29756: Programmatically register related co-processor during initialization#7743sharmaar12 wants to merge 3 commits intoapache:HBASE-29081from
Conversation
sharmaar12
force-pushed
the
prog_reg_cop
branch
from
b61c399 to
2909891
Compare
| LOG.info("Updated {} readOnlyEnabled={}", this.getClass().getName(), | ||
| readOnlyEnabledFromConfig); | ||
| if (this.masterServices != null) { | ||
| manageActiveClusterIdFile(readOnlyEnabledFromConfig); |
There was a problem hiding this comment.
I think there's an issue with this approach.
- We talked about briefly that the local
globalReadOnlyEnabledis 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. - As a consequence this method will always be called with
readOnlyEnabledFromConfig == trueand 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.
sharmaar12
force-pushed
the
prog_reg_cop
branch
from
b04b6cb to
f3889b3
Compare
| addReadOnlyCoprocessors(this.baseConf); | ||
| addReadOnlyCoprocessors(this.conf); | ||
| } else { | ||
| removeReadOnlyCoprocessors(this.baseConf); |
There was a problem hiding this comment.
We need to add this to introduce the REGION_COPROCESSOR_CONF_KEY property in the conf object, which is instance of CompoundConfiguration. This is needed because if the property is not present as a local copy in CompoundConfiguration then updating it using setStrings does not work so though Coprocessors are not needed still the property must be set to empty collection.
| this.conf.setBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY, readOnlyMode); | ||
| // If readonly is true then add the coprocessor of master | ||
| if (readOnlyMode) { | ||
| addReadOnlyCoprocessors(this.baseConf); |
There was a problem hiding this comment.
We must sync both conf as well as baseConf object because baseConf is used when new table/region is created and synching just conf will make the new table to use initial value with which the cluster started. This is also important when region split happens as in the split baseConf gets used instead of conf object.
| // remove the coprocessors | ||
| // Also conf.unset will not have any effect on CompoundConfiguration, unlike with Hmaster and | ||
| // HRegionServer | ||
| conf.setStrings(CoprocessorHost.REGION_COPROCESSOR_CONF_KEY, new String[0]); |
There was a problem hiding this comment.
We have to explicitly use setStrings instead of conf.unset() because unset does not work with CompoundConfiguration (shallow copy), as setting it to null or using unset has no effect on CompoundConfiguration.
3 participants
No description provided.