Copilot commented on code in PR #8017:
URL: https://github.com/apache/hbase/pull/8017#discussion_r3025844546
##########
hbase-server/src/test/java/org/apache/hadoop/hbase/regionserver/TestMetricsRegionServer.java:
##########
@@ -298,4 +298,9 @@ public void testScannerMetrics() {
HELPER.assertGauge("activeScanners", 0, serverSource);
}
+ @Test
+ public void testSplitRequest() {
+ rsm.incrSplitRequest(null);
+ HELPER.assertCounter("splitRequestCount", 1, serverSource);
Review Comment:
This test only covers the null-table path. Since incrSplitRequest now
accepts a table name and is expected to always increment the RegionServer
counter regardless of table forwarding, it would be better to also call
rsm.incrSplitRequest(<non-null table>) and assert the RegionServer counter
increments again (e.g., becomes 2). This guards against regressions where the
RS counter might accidentally become conditional on the table parameter.
```suggestion
HELPER.assertCounter("splitRequestCount", 1, serverSource);
TableName tn = TableName.valueOf("table1");
rsm.incrSplitRequest(tn);
HELPER.assertCounter("splitRequestCount", 2, serverSource);
```
##########
hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/MetricsRegionServer.java:
##########
@@ -232,16 +232,11 @@ public void updateScanTime(TableName tn, long t) {
userAggregate.updateScanTime(t);
}
- public void updateSplitTime(long t) {
- serverSource.updateSplitTime(t);
- }
-
- public void incrSplitRequest() {
+ public void incrSplitRequest(String table) {
serverSource.incrSplitRequest();
- }
-
- public void incrSplitSuccess() {
- serverSource.incrSplitSuccess();
+ if (table != null) {
+ metricsTable.incrSplitRequest(table);
+ }
}
Review Comment:
This method now explicitly supports null to mean 'RegionServer-level only'
(and tests call it with null). Please document this contract in Javadoc and/or
annotate the parameter as nullable to make the intended usage clear to callers.
--
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]
