Copilot commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4257767948
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterWrapperImpl.java:
##########
@@ -267,4 +269,33 @@ public long getOldWALsDirSize() {
}
return master.getMasterWalManager().getOldWALsDirSize();
}
+
+ @Override
+ public Map<String, Integer> getSwitchStates() {
+ if (!master.isActiveMaster() || !master.isInitialized()) {
+ return Collections.emptyMap();
+ }
+ Map<String, Integer> states = new HashMap<>();
+ states.put(MetricsMasterSource.BALANCER_ENABLED_NAME,
master.isBalancerOn() ? 1 : 0);
+ states.put(MetricsMasterSource.NORMALIZER_ENABLED_NAME,
master.isNormalizerOn() ? 1 : 0);
+ states.put(MetricsMasterSource.SPLIT_ENABLED_NAME,
+ master.isSplitOrMergeEnabled(MasterSwitchType.SPLIT) ? 1 : 0);
+ states.put(MetricsMasterSource.MERGE_ENABLED_NAME,
+ master.isSplitOrMergeEnabled(MasterSwitchType.MERGE) ? 1 : 0);
+ states.put(MetricsMasterSource.CLEANER_CHORE_ENABLED_NAME,
+ master.isCleanerChoreEnabled() ? 1 : 0);
+ states.put(MetricsMasterSource.CATALOG_JANITOR_ENABLED_NAME,
+ master.isCatalogJanitorEnabled() ? 1 : 0);
+ states.put(MetricsMasterSource.SNAPSHOT_CLEANUP_ENABLED_NAME,
+ master.snapshotCleanupStateStore.get() ? 1 : 0);
Review Comment:
This directly dereferences `master.snapshotCleanupStateStore` and will throw
if it is ever unset/null at runtime. The new wrapper contract says unavailable
states should be omitted, so guard against a missing store (omit the metric)
rather than blindly dereferencing the field.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterRepairMode.java:
##########
@@ -96,6 +103,33 @@ public void testNewCluster() throws Exception {
}
}
+ @Test
+ public void testSwitchMetricsWithQuotasEnabled() throws Exception {
+ enableMaintenanceMode();
+ TEST_UTIL.getConfiguration().setBoolean(QuotaUtil.QUOTA_CONF_KEY, true);
+ TEST_UTIL.startMiniCluster(
+
StartTestingClusterOption.builder().numRegionServers(0).numDataNodes(1).build());
+
+ HMaster master = TEST_UTIL.getMiniHBaseCluster().getMaster();
+ assertTrue(master.isInMaintenanceMode());
+ assertNull(master.getMasterQuotaManager());
+ MetricsMasterSource source = master.getMasterMetrics().getMetricsSource();
+ MetricsAssertHelper metricsHelper =
CompatibilityFactory.getInstance(MetricsAssertHelper.class);
+ Map<String, Integer> states = new
MetricsMasterWrapperImpl(master).getSwitchStates();
+ assertEquals(8, states.size());
+
assertFalse(states.containsKey(MetricsMasterSource.RPC_THROTTLE_ENABLED_NAME));
Review Comment:
`assertEquals(8, states.size())` is brittle: it will fail if new
switch-state gauges are added in the future (even if behavior is correct).
Prefer asserting on the exact expected key set (using the
`MetricsMasterSource.*_NAME` constants) and separately asserting absence of
`RPC_THROTTLE_ENABLED_NAME` in this mode.
##########
hbase-website/app/pages/_docs/docs/_mdx/(multi-page)/operational-management/metrics-and-monitoring.mdx:
##########
@@ -133,6 +133,38 @@ The number of regions that have been in transition longer
than a threshold time
**hbase.master.ritOldestAge**
The age of the longest region in transition, in milliseconds
+#### Master switch states
+
+The active, initialized Master exposes numeric switch-state gauges under
+`Hadoop:service=HBase,name=Master,sub=Server`. A value of `1` means enabled and
+`0` means disabled. The gauges are absent on backup Masters and before the
Master
+is initialized.
+Enabled states indicate that an operation is permitted, rather than that it is
+currently running.
+
+| Gauge | HBase shell command
|
+| ------------------------------------ |
---------------------------------------------- |
+| `balancerEnabled` | `balance_switch true\|false`
|
+| `normalizerEnabled` | `normalizer_switch true\|false`
|
+| `splitEnabled` | `splitormerge_switch 'SPLIT',
true\|false` |
+| `mergeEnabled` | `splitormerge_switch 'MERGE',
true\|false` |
+| `cleanerChoreEnabled` | `cleaner_chore_switch true\|false`
|
+| `catalogJanitorEnabled` | `catalogjanitor_switch true\|false`
|
+| `snapshotCleanupEnabled` | `snapshot_cleanup_switch true\|false`
|
+| `replicationPeerModificationEnabled` | `peer_modification_switch
true\|false` |
+| `rpcThrottleEnabled` | `enable_rpc_throttle` /
`disable_rpc_throttle` |
Review Comment:
The Markdown table uses a double leading pipe (`||`), which typically
renders as an extra empty first column (or otherwise mis-renders depending on
the renderer). Use a single leading pipe (`|`) for the header, separator, and
each row to ensure consistent table rendering.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java:
##########
@@ -147,6 +149,34 @@ public static void after() throws Exception {
TEST_UTIL.shutdownMiniCluster();
}
+ @Test
+ public void testSwitchMetricsFollowAdminChanges() throws IOException {
+ Admin admin = TEST_UTIL.getAdmin();
+ MetricsMasterSource source = master.getMasterMetrics().getMetricsSource();
+ Map<String, IOExceptionConsumer<Boolean>> switches = new HashMap<>();
+ switches.put("balancerEnabled", enabled -> admin.balancerSwitch(enabled,
false));
+ switches.put("normalizerEnabled", admin::normalizerSwitch);
+ switches.put("splitEnabled", enabled -> admin.splitSwitch(enabled, false));
+ switches.put("mergeEnabled", enabled -> admin.mergeSwitch(enabled, false));
+ switches.put("cleanerChoreEnabled", admin::cleanerChoreSwitch);
+ switches.put("catalogJanitorEnabled", admin::catalogJanitorSwitch);
+ switches.put("snapshotCleanupEnabled", enabled ->
admin.snapshotCleanupSwitch(enabled, false));
+ switches.put("replicationPeerModificationEnabled",
admin::replicationPeerModificationSwitch);
Review Comment:
The test hard-codes metric names as string literals even though this PR
introduces `MetricsMasterSource.*_NAME` constants for these gauges. Using the
constants here reduces the chance of typo/drift if metric names change in the
future and keeps tests aligned with the production source.
--
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]
