You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@hbase.apache.org by "junegunn (via GitHub)" <gi...@apache.org> on 2026/10/12 07:26:25 UTC
[PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
junegunn opened a new pull request, #8758:
URL: https://github.com/apache/hbase/pull/8758
Jira: https://issues.apache.org/jira/browse/HBASE-30473
```json
{
"name": "Hadoop:service=HBase,name=Master,sub=Server",
"modelerType": "Master,sub=Server",
"tag.liveRegionServers": "192.168.35.231,16020,1791788928190",
"tag.deadRegionServers": "",
"tag.drainingRegionServers": "",
"tag.zookeeperQuorum": "127.0.0.1:2181",
"tag.serverName": "192.168.35.231,16000,1791788926592",
"tag.clusterId": "07aeab61-7d0f-487b-9f71-4bf8d38565c1",
"tag.isActiveMaster": "true",
"tag.Context": "master",
"tag.Hostname": "AD02514045",
"mergePlanCount": 0,
"splitPlanCount": 0,
"masterActiveTime": 1791788928555,
"masterStartTime": 1791788926592,
"masterFinishedInitializationTime": 1791788934045,
"averageLoad": 6.0,
"onlineRegionCount": 5,
"offlineRegionCount": 0,
"numRegionServers": 1,
"numDeadRegionServers": 0,
"numDrainingRegionServers": 0,
"oldWALsDirSize": 0,
"catalogJanitorEnabled": 1,
"mergeEnabled": 1,
"normalizerEnabled": 1,
"snapshotCleanupEnabled": 1,
"splitEnabled": 1,
"replicationPeerModificationEnabled": 1,
"cleanerChoreEnabled": 1,
"rpcThrottleEnabled": 0,
"balancerEnabled": 1,
"clusterRequests": 19,
"clusterWriteRequests": 10,
"clusterReadRequests": 18,
"ServerCrashTime_num_ops": 1,
"ServerCrashTime_min": 0,
"ServerCrashTime_max": 0,
"ServerCrashTime_mean": 0,
"ServerCrashTime_25th_percentile": 0,
"ServerCrashTime_median": 0,
"ServerCrashTime_75th_percentile": 0,
"ServerCrashTime_90th_percentile": 0,
"ServerCrashTime_95th_percentile": 0,
"ServerCrashTime_98th_percentile": 0,
"ServerCrashTime_99th_percentile": 0,
"ServerCrashTime_99.9th_percentile": 0,
"ServerCrashSubmittedCount": 1,
"ServerCrashFailedCount": 0
}
```
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "NihalJain (via GitHub)" <gi...@apache.org>.
NihalJain commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4250408057
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterWrapperImpl.java:
##########
@@ -267,4 +269,31 @@ public long getOldWALsDirSize() {
}
return master.getMasterWalManager().getOldWALsDirSize();
}
+
+ @Override
+ public Map<String, Integer> getSwitchStates() {
+ if (!master.isActiveMaster() || !master.isInitialized()) {
+ return Collections.emptyMap();
Review Comment:
If data is not available should we not return a map of -1 so that parser doesnot need to worry about key exitence?
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4254096476
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterSwitchMetrics.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Consumer;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.CompatibilityFactory;
+import org.apache.hadoop.hbase.client.MasterSwitchType;
+import org.apache.hadoop.hbase.master.snapshot.SnapshotCleanupStateStore;
+import org.apache.hadoop.hbase.metrics.MetricRegistries;
+import org.apache.hadoop.hbase.quotas.MasterQuotaManager;
+import org.apache.hadoop.hbase.quotas.QuotaUtil;
+import org.apache.hadoop.hbase.quotas.RpcThrottleStateStore;
+import org.apache.hadoop.hbase.test.MetricsAssertHelper;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+@Tag(MasterTests.TAG)
+@Tag(SmallTests.TAG)
+public class TestMasterSwitchMetrics {
+ private static final MetricsAssertHelper METRICS_HELPER =
+ CompatibilityFactory.getInstance(MetricsAssertHelper.class);
+
+ private HMaster master;
+ private Configuration conf;
+ private RpcThrottleStateStore rpcThrottleStateStore;
+ private MetricsMasterWrapperImpl wrapper;
+ private MetricsMasterSourceImpl source;
+
+ @BeforeEach
+ public void setUp() {
+ master = mock(HMaster.class);
+ conf = new Configuration(false);
+ conf.setBoolean(QuotaUtil.QUOTA_CONF_KEY, true);
+ when(master.getConfiguration()).thenReturn(conf);
+ when(master.isActiveMaster()).thenReturn(true);
+ when(master.isInitialized()).thenReturn(true);
+ when(master.getMasterWalManager()).thenReturn(mock(MasterWalManager.class));
+ when(master.getMasterQuotaManager()).thenReturn(mock(MasterQuotaManager.class));
+ master.snapshotCleanupStateStore = mock(SnapshotCleanupStateStore.class);
+ rpcThrottleStateStore = mock(RpcThrottleStateStore.class);
+ when(master.getRpcThrottleStateStore()).thenReturn(rpcThrottleStateStore);
+ wrapper = new MetricsMasterWrapperImpl(master);
+ source = new MetricsMasterSourceImpl(wrapper);
+ }
+
+ @AfterEach
+ public void tearDown() {
+ DefaultMetricsSystem.instance().unregisterSource(MetricsMasterSource.METRICS_JMX_CONTEXT);
+ MetricRegistries.global().remove(source.getMetricRegistryInfo());
+ }
+
+ @Test
+ public void testEachSwitchChangesItsGauge() {
+ Map<String, Consumer<Boolean>> switches = new HashMap<>();
+ switches.put("balancerEnabled", enabled -> when(master.isBalancerOn()).thenReturn(enabled));
Review Comment:
Good point, addressed in cd37bb7508.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4250713813
##########
hbase-website/app/pages/_docs/docs/_mdx/(multi-page)/operational-management/metrics-and-monitoring.mdx:
##########
@@ -133,6 +133,37 @@ 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. A value of `-1` means the state is unknown. 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` |
Review Comment:
The backslash escapes (`\\|`) are inside inline code spans, where the backslash typically renders literally, so the docs will show `true\\|false` instead of `true|false`. Consider removing the backslashes inside the code spans (and, if needed for table parsing, escape the pipe outside the code span or use an HTML entity).
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterSwitchMetrics.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Consumer;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.CompatibilityFactory;
+import org.apache.hadoop.hbase.client.MasterSwitchType;
+import org.apache.hadoop.hbase.master.snapshot.SnapshotCleanupStateStore;
+import org.apache.hadoop.hbase.metrics.MetricRegistries;
+import org.apache.hadoop.hbase.quotas.MasterQuotaManager;
+import org.apache.hadoop.hbase.quotas.QuotaUtil;
+import org.apache.hadoop.hbase.quotas.RpcThrottleStateStore;
+import org.apache.hadoop.hbase.test.MetricsAssertHelper;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+@Tag(MasterTests.TAG)
+@Tag(SmallTests.TAG)
+public class TestMasterSwitchMetrics {
+ private static final MetricsAssertHelper METRICS_HELPER =
+ CompatibilityFactory.getInstance(MetricsAssertHelper.class);
+
+ private HMaster master;
+ private Configuration conf;
+ private RpcThrottleStateStore rpcThrottleStateStore;
+ private MetricsMasterWrapperImpl wrapper;
+ private MetricsMasterSourceImpl source;
+
+ @BeforeEach
+ public void setUp() {
+ master = mock(HMaster.class);
+ conf = new Configuration(false);
+ conf.setBoolean(QuotaUtil.QUOTA_CONF_KEY, true);
+ when(master.getConfiguration()).thenReturn(conf);
+ when(master.isActiveMaster()).thenReturn(true);
+ when(master.isInitialized()).thenReturn(true);
+ when(master.getMasterWalManager()).thenReturn(mock(MasterWalManager.class));
+ when(master.getMasterQuotaManager()).thenReturn(mock(MasterQuotaManager.class));
+ master.snapshotCleanupStateStore = mock(SnapshotCleanupStateStore.class);
+ rpcThrottleStateStore = mock(RpcThrottleStateStore.class);
+ when(master.getRpcThrottleStateStore()).thenReturn(rpcThrottleStateStore);
+ wrapper = new MetricsMasterWrapperImpl(master);
+ source = new MetricsMasterSourceImpl(wrapper);
+ }
+
+ @AfterEach
+ public void tearDown() {
+ DefaultMetricsSystem.instance().unregisterSource(MetricsMasterSource.METRICS_JMX_CONTEXT);
+ MetricRegistries.global().remove(source.getMetricRegistryInfo());
+ }
+
+ @Test
+ public void testEachSwitchChangesItsGauge() {
+ Map<String, Consumer<Boolean>> switches = new HashMap<>();
+ switches.put("balancerEnabled", enabled -> when(master.isBalancerOn()).thenReturn(enabled));
+ switches.put("normalizerEnabled", enabled -> when(master.isNormalizerOn()).thenReturn(enabled));
+ switches.put("splitEnabled",
+ enabled -> when(master.isSplitOrMergeEnabled(MasterSwitchType.SPLIT)).thenReturn(enabled));
+ switches.put("mergeEnabled",
+ enabled -> when(master.isSplitOrMergeEnabled(MasterSwitchType.MERGE)).thenReturn(enabled));
+ switches.put("cleanerChoreEnabled",
+ enabled -> when(master.isCleanerChoreEnabled()).thenReturn(enabled));
+ switches.put("catalogJanitorEnabled",
+ enabled -> when(master.isCatalogJanitorEnabled()).thenReturn(enabled));
+ switches.put("snapshotCleanupEnabled",
+ enabled -> when(master.snapshotCleanupStateStore.get()).thenReturn(enabled));
+ switches.put("replicationPeerModificationEnabled",
+ enabled -> when(master.isReplicationPeerModificationEnabled()).thenReturn(enabled));
+ switches.put("rpcThrottleEnabled",
+ enabled -> when(rpcThrottleStateStore.get()).thenReturn(enabled));
+ Map<String, Integer> expected = new HashMap<>();
+ switches.keySet().forEach(name -> expected.put(name, 0));
+ for (Map.Entry<String, Consumer<Boolean>> entry : switches.entrySet()) {
+ for (boolean enabled : new boolean[] { true, false }) {
+ entry.getValue().accept(enabled);
+ expected.put(entry.getKey(), enabled ? 1 : 0);
+ assertEquals(expected, wrapper.getSwitchStates());
+ expected
+ .forEach((name, state) -> METRICS_HELPER.assertGauge(name, state.longValue(), source));
+ }
+ }
+ verifyNoInteractions(master.getMasterQuotaManager());
Review Comment:
`verifyNoInteractions(master.getMasterQuotaManager())` calls `getMasterQuotaManager()` as part of the verification statement, which makes the intent slightly harder to read and couples the check to the stubbing. Prefer storing the quota manager mock in a local field/variable in `setUp()` (e.g., `MasterQuotaManager quotaManager = ...`) and then verifying `verifyNoInteractions(quotaManager)`.
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterSwitchMetrics.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Consumer;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.CompatibilityFactory;
+import org.apache.hadoop.hbase.client.MasterSwitchType;
+import org.apache.hadoop.hbase.master.snapshot.SnapshotCleanupStateStore;
+import org.apache.hadoop.hbase.metrics.MetricRegistries;
+import org.apache.hadoop.hbase.quotas.MasterQuotaManager;
+import org.apache.hadoop.hbase.quotas.QuotaUtil;
+import org.apache.hadoop.hbase.quotas.RpcThrottleStateStore;
+import org.apache.hadoop.hbase.test.MetricsAssertHelper;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+@Tag(MasterTests.TAG)
+@Tag(SmallTests.TAG)
+public class TestMasterSwitchMetrics {
+ private static final MetricsAssertHelper METRICS_HELPER =
+ CompatibilityFactory.getInstance(MetricsAssertHelper.class);
+
+ private HMaster master;
+ private Configuration conf;
+ private RpcThrottleStateStore rpcThrottleStateStore;
+ private MetricsMasterWrapperImpl wrapper;
+ private MetricsMasterSourceImpl source;
+
+ @BeforeEach
+ public void setUp() {
+ master = mock(HMaster.class);
+ conf = new Configuration(false);
+ conf.setBoolean(QuotaUtil.QUOTA_CONF_KEY, true);
+ when(master.getConfiguration()).thenReturn(conf);
+ when(master.isActiveMaster()).thenReturn(true);
+ when(master.isInitialized()).thenReturn(true);
+ when(master.getMasterWalManager()).thenReturn(mock(MasterWalManager.class));
+ when(master.getMasterQuotaManager()).thenReturn(mock(MasterQuotaManager.class));
+ master.snapshotCleanupStateStore = mock(SnapshotCleanupStateStore.class);
+ rpcThrottleStateStore = mock(RpcThrottleStateStore.class);
+ when(master.getRpcThrottleStateStore()).thenReturn(rpcThrottleStateStore);
+ wrapper = new MetricsMasterWrapperImpl(master);
+ source = new MetricsMasterSourceImpl(wrapper);
+ }
+
+ @AfterEach
+ public void tearDown() {
+ DefaultMetricsSystem.instance().unregisterSource(MetricsMasterSource.METRICS_JMX_CONTEXT);
+ MetricRegistries.global().remove(source.getMetricRegistryInfo());
+ }
+
+ @Test
+ public void testEachSwitchChangesItsGauge() {
+ Map<String, Consumer<Boolean>> switches = new HashMap<>();
+ switches.put("balancerEnabled", enabled -> when(master.isBalancerOn()).thenReturn(enabled));
+ switches.put("normalizerEnabled", enabled -> when(master.isNormalizerOn()).thenReturn(enabled));
+ switches.put("splitEnabled",
+ enabled -> when(master.isSplitOrMergeEnabled(MasterSwitchType.SPLIT)).thenReturn(enabled));
+ switches.put("mergeEnabled",
+ enabled -> when(master.isSplitOrMergeEnabled(MasterSwitchType.MERGE)).thenReturn(enabled));
+ switches.put("cleanerChoreEnabled",
+ enabled -> when(master.isCleanerChoreEnabled()).thenReturn(enabled));
+ switches.put("catalogJanitorEnabled",
+ enabled -> when(master.isCatalogJanitorEnabled()).thenReturn(enabled));
+ switches.put("snapshotCleanupEnabled",
+ enabled -> when(master.snapshotCleanupStateStore.get()).thenReturn(enabled));
+ switches.put("replicationPeerModificationEnabled",
+ enabled -> when(master.isReplicationPeerModificationEnabled()).thenReturn(enabled));
+ switches.put("rpcThrottleEnabled",
Review Comment:
The test hard-codes metric names as string literals. To reduce brittleness and ensure refactors don’t silently break tests/docs alignment, consider using the constants added in `MetricsMasterSource` (e.g., `MetricsMasterSource.BALANCER_ENABLED_NAME`, etc.) for these keys.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4253580060
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterWrapperImpl.java:
##########
@@ -267,4 +269,31 @@ public long getOldWALsDirSize() {
}
return master.getMasterWalManager().getOldWALsDirSize();
}
+
+ @Override
+ public Map<String, Integer> getSwitchStates() {
+ if (!master.isActiveMaster() || !master.isInitialized()) {
+ return Collections.emptyMap();
Review Comment:
Thanks for review. You're right that it's more consistent, but I think emitting `-1` during master restart, which can be quite frequent in some cases, could make monitoring more complicated without giving much benefit.
So on second thought, how about removing `-1` state altogether and also omitting `rpcThrottleEnabled` if it's not ready?
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "NihalJain (via GitHub)" <gi...@apache.org>.
NihalJain commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4250408057
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterWrapperImpl.java:
##########
@@ -267,4 +269,31 @@ public long getOldWALsDirSize() {
}
return master.getMasterWalManager().getOldWALsDirSize();
}
+
+ @Override
+ public Map<String, Integer> getSwitchStates() {
+ if (!master.isActiveMaster() || !master.isInitialized()) {
+ return Collections.emptyMap();
Review Comment:
Agree on omitting these on backup masters, but for an active-but-not-yet-initialized master, would it be better to emit -1 for all keys? That matches the documented “unknown” semantics and keeps the series continuous across failover?
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "NihalJain (via GitHub)" <gi...@apache.org>.
NihalJain commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4250562189
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterWrapperImpl.java:
##########
@@ -267,4 +269,31 @@ public long getOldWALsDirSize() {
}
return master.getMasterWalManager().getOldWALsDirSize();
}
+
+ @Override
+ public Map<String, Integer> getSwitchStates() {
+ if (!master.isActiveMaster() || !master.isInitialized()) {
+ return Collections.emptyMap();
+ }
+ int rpcThrottleEnabled = 0;
+ if (QuotaUtil.isQuotaEnabled(master.getConfiguration())) {
Review Comment:
In maintenance mode finishActiveMasterInitialization returns before initQuotaManager(), so with quota enabled rpcThrottleEnabled stays -1 for the lifetime of the Master. The docs say “until the quota manager is ready”, which implies it’s transient. Could you document this case of maintenance, or report 0 like the other switches already do in maintenance mode?
Worth testing out the metrics in MAINTENANCE_MODE = "hbase.master.maintenance_mode"
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "NihalJain (via GitHub)" <gi...@apache.org>.
NihalJain commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4250485712
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java:
##########
@@ -147,6 +149,33 @@ 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);
+ Map<String, Integer> initialStates = new MetricsMasterWrapperImpl(master).getSwitchStates();
+ for (Map.Entry<String, IOExceptionConsumer<Boolean>> entry : switches.entrySet()) {
+ try {
+ for (boolean enabled : new boolean[] { false, true }) {
+ entry.getValue().accept(enabled);
+ metricsHelper.assertGauge(entry.getKey(), enabled ? 1L : 0L, source);
+ }
+ } finally {
+ entry.getValue().accept(initialStates.get(entry.getKey()) == 1);
+ }
+ }
+ metricsHelper.assertGauge("rpcThrottleEnabled", 0L, source);
Review Comment:
nit: worth adding a comment why this is 0 so that other dont get confused later?
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4260694312
##########
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:
b0d816b6f0
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "NihalJain (via GitHub)" <gi...@apache.org>.
NihalJain commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4250533495
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterSwitchMetrics.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Consumer;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.CompatibilityFactory;
+import org.apache.hadoop.hbase.client.MasterSwitchType;
+import org.apache.hadoop.hbase.master.snapshot.SnapshotCleanupStateStore;
+import org.apache.hadoop.hbase.metrics.MetricRegistries;
+import org.apache.hadoop.hbase.quotas.MasterQuotaManager;
+import org.apache.hadoop.hbase.quotas.QuotaUtil;
+import org.apache.hadoop.hbase.quotas.RpcThrottleStateStore;
+import org.apache.hadoop.hbase.test.MetricsAssertHelper;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+@Tag(MasterTests.TAG)
+@Tag(SmallTests.TAG)
+public class TestMasterSwitchMetrics {
+ private static final MetricsAssertHelper METRICS_HELPER =
+ CompatibilityFactory.getInstance(MetricsAssertHelper.class);
+
+ private HMaster master;
+ private Configuration conf;
+ private RpcThrottleStateStore rpcThrottleStateStore;
+ private MetricsMasterWrapperImpl wrapper;
+ private MetricsMasterSourceImpl source;
+
+ @BeforeEach
+ public void setUp() {
+ master = mock(HMaster.class);
+ conf = new Configuration(false);
+ conf.setBoolean(QuotaUtil.QUOTA_CONF_KEY, true);
+ when(master.getConfiguration()).thenReturn(conf);
+ when(master.isActiveMaster()).thenReturn(true);
+ when(master.isInitialized()).thenReturn(true);
+ when(master.getMasterWalManager()).thenReturn(mock(MasterWalManager.class));
+ when(master.getMasterQuotaManager()).thenReturn(mock(MasterQuotaManager.class));
+ master.snapshotCleanupStateStore = mock(SnapshotCleanupStateStore.class);
+ rpcThrottleStateStore = mock(RpcThrottleStateStore.class);
+ when(master.getRpcThrottleStateStore()).thenReturn(rpcThrottleStateStore);
+ wrapper = new MetricsMasterWrapperImpl(master);
+ source = new MetricsMasterSourceImpl(wrapper);
+ }
+
+ @AfterEach
+ public void tearDown() {
+ DefaultMetricsSystem.instance().unregisterSource(MetricsMasterSource.METRICS_JMX_CONTEXT);
+ MetricRegistries.global().remove(source.getMetricRegistryInfo());
+ }
+
+ @Test
+ public void testEachSwitchChangesItsGauge() {
+ Map<String, Consumer<Boolean>> switches = new HashMap<>();
+ switches.put("balancerEnabled", enabled -> when(master.isBalancerOn()).thenReturn(enabled));
Review Comment:
nit: Use MetricsMasterSource.*_NAME constants instead?
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4260691738
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterSwitchMetrics.java:
##########
@@ -0,0 +1,137 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.hadoop.hbase.master;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyNoInteractions;
+import static org.mockito.Mockito.when;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.Set;
+import java.util.function.Consumer;
+import org.apache.hadoop.conf.Configuration;
+import org.apache.hadoop.hbase.CompatibilityFactory;
+import org.apache.hadoop.hbase.client.MasterSwitchType;
+import org.apache.hadoop.hbase.master.snapshot.SnapshotCleanupStateStore;
+import org.apache.hadoop.hbase.metrics.MetricRegistries;
+import org.apache.hadoop.hbase.quotas.MasterQuotaManager;
+import org.apache.hadoop.hbase.quotas.QuotaUtil;
+import org.apache.hadoop.hbase.quotas.RpcThrottleStateStore;
+import org.apache.hadoop.hbase.test.MetricsAssertHelper;
+import org.apache.hadoop.hbase.testclassification.MasterTests;
+import org.apache.hadoop.hbase.testclassification.SmallTests;
+import org.apache.hadoop.metrics2.lib.DefaultMetricsSystem;
+import org.junit.jupiter.api.AfterEach;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Tag;
+import org.junit.jupiter.api.Test;
+
+@Tag(MasterTests.TAG)
+@Tag(SmallTests.TAG)
+public class TestMasterSwitchMetrics {
+ private static final MetricsAssertHelper METRICS_HELPER =
+ CompatibilityFactory.getInstance(MetricsAssertHelper.class);
+
+ private HMaster master;
+ private Configuration conf;
+ private RpcThrottleStateStore rpcThrottleStateStore;
+ private MetricsMasterWrapperImpl wrapper;
+ private MetricsMasterSourceImpl source;
+
+ @BeforeEach
+ public void setUp() {
+ master = mock(HMaster.class);
+ conf = new Configuration(false);
+ conf.setBoolean(QuotaUtil.QUOTA_CONF_KEY, true);
+ when(master.getConfiguration()).thenReturn(conf);
+ when(master.isActiveMaster()).thenReturn(true);
+ when(master.isInitialized()).thenReturn(true);
+ when(master.getMasterWalManager()).thenReturn(mock(MasterWalManager.class));
+ when(master.getMasterQuotaManager()).thenReturn(mock(MasterQuotaManager.class));
+ master.snapshotCleanupStateStore = mock(SnapshotCleanupStateStore.class);
+ rpcThrottleStateStore = mock(RpcThrottleStateStore.class);
+ when(master.getRpcThrottleStateStore()).thenReturn(rpcThrottleStateStore);
+ wrapper = new MetricsMasterWrapperImpl(master);
+ source = new MetricsMasterSourceImpl(wrapper);
+ }
+
+ @AfterEach
+ public void tearDown() {
+ DefaultMetricsSystem.instance().unregisterSource(MetricsMasterSource.METRICS_JMX_CONTEXT);
+ MetricRegistries.global().remove(source.getMetricRegistryInfo());
+ }
+
+ @Test
+ public void testEachSwitchChangesItsGauge() {
+ Map<String, Consumer<Boolean>> switches = new HashMap<>();
+ switches.put("balancerEnabled", enabled -> when(master.isBalancerOn()).thenReturn(enabled));
Review Comment:
Oops, b0d816b6f0 as well.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4254092952
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterWrapperImpl.java:
##########
@@ -267,4 +269,31 @@ public long getOldWALsDirSize() {
}
return master.getMasterWalManager().getOldWALsDirSize();
}
+
+ @Override
+ public Map<String, Integer> getSwitchStates() {
+ if (!master.isActiveMaster() || !master.isInitialized()) {
+ return Collections.emptyMap();
Review Comment:
I've prepared the change to remove `-1` and omit unavailable states, keeping the metrics strictly boolean. Please take a look and let me know what you think.
5d9fd6c4cf
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4245719604
##########
hbase-website/app/pages/_docs/docs/_mdx/(multi-page)/operational-management/metrics-and-monitoring.mdx:
##########
@@ -133,6 +133,39 @@ 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. A value of `-1` means the state is unknown. 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.
+
+Replace `true|false` with `true` to enable or `false` to disable.
+
+| 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` |
Review Comment:
The table rows start with `||` which will render as an extra empty first column in Markdown/MDX tables. Use single leading pipes (`|`) for standard two-column table formatting so the table renders correctly.
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterWrapperImpl.java:
##########
@@ -267,4 +269,31 @@ public long getOldWALsDirSize() {
}
return master.getMasterWalManager().getOldWALsDirSize();
}
+
+ @Override
+ public Map<String, Integer> getSwitchStates() {
+ if (!master.isActiveMaster() || !master.isInitialized()) {
+ return Collections.emptyMap();
+ }
+ int rpcThrottleEnabled = 0;
+ if (QuotaUtil.isQuotaEnabled(master.getConfiguration())) {
+ rpcThrottleEnabled = -1;
+ if (master.getMasterQuotaManager() != null) {
+ rpcThrottleEnabled = master.getRpcThrottleStateStore().get() ? 1 : 0;
+ }
+ }
+ return Map.of(MetricsMasterSource.BALANCER_ENABLED_NAME, master.isBalancerOn() ? 1 : 0,
+ MetricsMasterSource.NORMALIZER_ENABLED_NAME, master.isNormalizerOn() ? 1 : 0,
+ MetricsMasterSource.SPLIT_ENABLED_NAME,
+ master.isSplitOrMergeEnabled(MasterSwitchType.SPLIT) ? 1 : 0,
+ MetricsMasterSource.MERGE_ENABLED_NAME,
+ master.isSplitOrMergeEnabled(MasterSwitchType.MERGE) ? 1 : 0,
+ MetricsMasterSource.CLEANER_CHORE_ENABLED_NAME, master.isCleanerChoreEnabled() ? 1 : 0,
+ MetricsMasterSource.CATALOG_JANITOR_ENABLED_NAME, master.isCatalogJanitorEnabled() ? 1 : 0,
+ MetricsMasterSource.SNAPSHOT_CLEANUP_ENABLED_NAME,
+ master.snapshotCleanupStateStore.get() ? 1 : 0,
+ MetricsMasterSource.REPLICATION_PEER_MODIFICATION_ENABLED_NAME,
+ master.isReplicationPeerModificationEnabled() ? 1 : 0,
+ MetricsMasterSource.RPC_THROTTLE_ENABLED_NAME, rpcThrottleEnabled);
Review Comment:
`getSwitchStates()` allocates a new `Map` on every metrics collection. Since metrics can be scraped frequently, this creates avoidable GC churn. Consider reusing a preallocated mutable map (cleared/refilled per call) or changing the source to emit these gauges directly without constructing an intermediate map.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on PR #8758:
URL: https://github.com/apache/hbase/pull/8758#issuecomment-6183065394
@NihalJain @liuxiaocs7 Thanks for the reviews. I made [a small change](https://github.com/apache/hbase/pull/8758/commits/c1e6eb47c26cca1929995b2e7d694fa6aefb9de5) to address an edge case where `hbase.quota.enabled` is changed by an `update_config`. Could you take another look?
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4253580060
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterWrapperImpl.java:
##########
@@ -267,4 +269,31 @@ public long getOldWALsDirSize() {
}
return master.getMasterWalManager().getOldWALsDirSize();
}
+
+ @Override
+ public Map<String, Integer> getSwitchStates() {
+ if (!master.isActiveMaster() || !master.isInitialized()) {
+ return Collections.emptyMap();
Review Comment:
Thanks for review. You're right that it's more consistent.
> keeps the series continuous across failover?
I think emitting `-1` during master restart, which can be quite frequent in some cases, could make monitoring more complicated without giving much benefit.
So on second thought, how about removing `-1` state altogether and also omitting `rpcThrottleEnabled` if it's not ready?
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4254094293
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java:
##########
@@ -147,6 +149,33 @@ 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);
+ Map<String, Integer> initialStates = new MetricsMasterWrapperImpl(master).getSwitchStates();
+ for (Map.Entry<String, IOExceptionConsumer<Boolean>> entry : switches.entrySet()) {
+ try {
+ for (boolean enabled : new boolean[] { false, true }) {
+ entry.getValue().accept(enabled);
+ metricsHelper.assertGauge(entry.getKey(), enabled ? 1L : 0L, source);
+ }
+ } finally {
+ entry.getValue().accept(initialStates.get(entry.getKey()) == 1);
+ }
+ }
+ metricsHelper.assertGauge("rpcThrottleEnabled", 0L, source);
Review Comment:
Thanks! 6d5fe4081f
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4254092952
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterWrapperImpl.java:
##########
@@ -267,4 +269,31 @@ public long getOldWALsDirSize() {
}
return master.getMasterWalManager().getOldWALsDirSize();
}
+
+ @Override
+ public Map<String, Integer> getSwitchStates() {
+ if (!master.isActiveMaster() || !master.isInitialized()) {
+ return Collections.emptyMap();
Review Comment:
I've prepared the change to remove `-1` and omit unavailable states, keeping the flags strictly boolean. Please take a look and let me know what you think.
5d9fd6c4cf
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn commented on code in PR #8758:
URL: https://github.com/apache/hbase/pull/8758#discussion_r4254099138
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterWrapperImpl.java:
##########
@@ -267,4 +269,31 @@ public long getOldWALsDirSize() {
}
return master.getMasterWalManager().getOldWALsDirSize();
}
+
+ @Override
+ public Map<String, Integer> getSwitchStates() {
+ if (!master.isActiveMaster() || !master.isInitialized()) {
+ return Collections.emptyMap();
+ }
+ int rpcThrottleEnabled = 0;
+ if (QuotaUtil.isQuotaEnabled(master.getConfiguration())) {
Review Comment:
5d9fd6c4cf makes the metrics strictly boolean.
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] HBASE-30473 Expose Master switch states as numeric metrics [hbase]
Posted by "junegunn (via GitHub)" <gi...@apache.org>.
junegunn merged PR #8758:
URL: https://github.com/apache/hbase/pull/8758
--
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: issues-unsubscribe@hbase.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org