HBASE-23996 Add metric for the split WAL procedure - #8515
Conversation
c3b3ed4 to
bf73a3d
Compare
Assert the new splitWAL procedure metric in two places: TestMasterMetrics#testDefaultMasterMetrics (splitWALSubmittedCount is registered and reads 0 at rest) and TestSplitWALManager#testSplitLogs (increments by one per SplitWALProcedure, verified against a captured baseline on the procedure-based non-ZK split path). Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
Mark the legacy ZooKeeper-coordinated WAL split coordination classes (ZKSplitLogManagerCoordination, ZkSplitLogWorkerCoordination) and the associated MetricsMasterFileSystem source as deprecated, since the procedure-based distributed WAL splitter (SplitWALManager) is the default and these are only reachable under the legacy hbase.split.writer.coordinated.by.zk path. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
bf73a3d to
4bff3da
Compare
There was a problem hiding this comment.
Pull request overview
Adds master metrics for procedure-based WAL splitting and deprecates metrics and coordination classes tied to the legacy ZooKeeper flow.
Changes:
- Adds submitted, failed, and timing metrics for
SplitWALProcedure. - Verifies metric registration and submission counts.
- Marks legacy ZooKeeper WAL-splitting components as deprecated.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestSplitWALManager.java |
Tests split-WAL submission metrics. |
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestMasterMetrics.java |
Verifies default metric registration. |
hbase-server/src/main/java/org/apache/hadoop/hbase/master/procedure/SplitWALProcedure.java |
Connects the procedure to its metrics. |
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterFileSystem.java |
Deprecates legacy filesystem metrics. |
hbase-server/src/main/java/org/apache/hadoop/hbase/master/MetricsMaster.java |
Exposes split-WAL procedure metrics. |
hbase-server/src/main/java/org/apache/hadoop/hbase/coordination/ZkSplitLogWorkerCoordination.java |
Deprecates legacy worker coordination. |
hbase-server/src/main/java/org/apache/hadoop/hbase/coordination/ZKSplitLogManagerCoordination.java |
Deprecates legacy manager coordination. |
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterSourceImpl.java |
Registers split-WAL operation metrics. |
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterSource.java |
Defines the metric prefix and accessor. |
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterFilesystemSourceImpl.java |
Deprecates the legacy metrics implementation. |
hbase-hadoop-compat/src/main/java/org/apache/hadoop/hbase/master/MetricsMasterFileSystemSource.java |
Deprecates the legacy metrics interface. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
The new success-path integration test only verifies |
| assertEquals(0, splitWALManager.getWALsToSplit(metaServer, true).size()); | ||
| assertEquals(1, splitWALManager.getWALsToSplit(metaServer, false).size()); | ||
| // The meta SplitWALProcedure should also have been counted by the split WAL metric. | ||
| METRICS_HELPER.assertCounter(MetricsMasterSource.SPLIT_WAL_METRIC_PREFIX + "SubmittedCount", |
There was a problem hiding this comment.
This validates splitWALSubmittedCount, but the new OperationMetrics also exports splitWALTime and splitWALFailedCount. Could we assert that the time histogram receives one sample for each successfully completed SplitWALProcedure here?
I do not see a deterministic terminal-failure path for SplitWALProcedure, so I would not require a FailedCount integration test in this PR. The generic procedure metrics test covers failure accounting; an assertion that FailedCount remains unchanged on this successful/retryable path would be a useful optional addition.
|
|
||
| /** | ||
| * ZooKeeper based implementation of {@link SplitLogManagerCoordination} | ||
| * ZooKeeper based implementation of {@link SplitLogManagerCoordination}. |
There was a problem hiding this comment.
Could we make the replacement a Javadoc link? SplitWALManager is in org.apache.hadoop.hbase.master, so use {@link org.apache.hadoop.hbase.master.SplitWALManager}. This makes the migration target navigable in generated API docs.
| * ZooKeeper based implementation of {@link SplitLogWorkerCoordination} It listen for changes in | ||
| * ZooKeeper and | ||
| * ZooKeeper based implementation of {@link SplitLogWorkerCoordination}. | ||
| * @deprecated since 2.4.0 and in 3.0.0, to be removed in 4.0.0, replaced by procedure-based |
There was a problem hiding this comment.
Same javadoc comment applies here
|
|
||
| /** | ||
| * @deprecated since 2.4.0 and in 3.0.0, to be removed in 4.0.0, replaced by procedure-based | ||
| * distributed WAL splitter; see SplitWALManager. Implementation of the deprecated |
There was a problem hiding this comment.
Same javadoc comment applies here
We already have deprecated HBASE_SPLIT_WAL_COORDINATED_BY_ZK but some Classes only used by that flow was not marked deprecated. I have marked those deprecated in this PR only. I hope that is fine else I will create another PR for that.
Some screen shots for the old metrics and the new one.
example 1
example 2
Old one