sharmaar12 commented on code in PR #7743:
URL: https://github.com/apache/hbase/pull/7743#discussion_r2844882562
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -4527,6 +4552,52 @@ private void setQuotasObserver(Configuration conf) {
}
}
+ private void addReadOnlyCoprocessors(Configuration conf) {
+ String[] masterCoprocs =
conf.getStrings(CoprocessorHost.MASTER_COPROCESSOR_CONF_KEY);
+ // If already present, do nothing
+ if (
+ masterCoprocs != null
+ &&
Arrays.asList(masterCoprocs).contains(MasterReadOnlyController.class.getName())
+ ) {
+ return;
+ }
+
+ final int length = null == masterCoprocs ? 0 : masterCoprocs.length;
+ String[] updatedCoprocs = new String[length + 1];
+ if (length > 0) {
+ System.arraycopy(masterCoprocs, 0, updatedCoprocs, 0,
masterCoprocs.length);
+ }
+ updatedCoprocs[length] = MasterReadOnlyController.class.getName();
Review Comment:
Done.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/HMaster.java:
##########
@@ -4422,13 +4430,30 @@ public void onConfigurationChange(Configuration
newConf) {
}
// append the quotas observer back to the master coprocessor key
setQuotasObserver(newConf);
+
+ boolean readOnlyMode =
newConf.getBoolean(HConstants.HBASE_GLOBAL_READONLY_ENABLED_KEY,
+ HConstants.HBASE_GLOBAL_READONLY_ENABLED_DEFAULT);
+ if (readOnlyMode) {
+ addReadOnlyCoprocessors(newConf);
+ } else {
+ // Needed as safety measure in case the coprocessors are added in
hbase-site.xml manually and
+ // the user toggles the read only mode on and off.
Review Comment:
There is another reason for adding this also. As our
`onConfigurationChange(Configuration newConf)` does not treat it as constant so
any mutation to `newConf` is actually reflecting in the application code (test
code in our case, this may not appear in client server scenario). So our tests
which does multiple toggle of the ReadOnlyMode, called
`testReadOnlyControllerLoadUnloadedWhenMultipleReadOnlyToggle`, gets its conf
object modified as side effect of calling `onConfigurationChange(Configuration
newConf)` which should not be intended behavior.
Earlier I thought to make copy of `newConf` and mutate it so that our
modification will not get propagated to the caller but it may not be acceptable
as copying entire conf will be time consuming hence I chose this route of
removing in case it was there. It also safeguard in case use deliberately add
them.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java:
##########
@@ -1292,6 +1306,80 @@ RegionEventDescriptor.EventType.REGION_CLOSE,
getRegionInfo(), mvcc.getReadPoint
}
}
+ private String[] append(String[] original, String value) {
+ String[] updated = new String[original.length + 1];
+ System.arraycopy(original, 0, updated, 0, original.length);
+ updated[original.length] = value;
+ return updated;
+ }
Review Comment:
Done. Used `ArrayList`.
--
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]
