mnpoonia commented on code in PR #8515:
URL: https://github.com/apache/hbase/pull/8515#discussion_r3781388067
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/master/TestSplitWALManager.java:
##########
@@ -251,6 +261,9 @@ private void splitLogsTestHelper(HBaseTestingUtil testUtil)
throws Exception {
ProcedureTestingUtility.submitAndWait(masterPE, procedures.get(0));
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",
Review 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.
--
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]
