Copilot commented on code in PR #4037:
URL: https://github.com/apache/cassandra/pull/4037#discussion_r2119006561
##########
src/java/org/apache/cassandra/metrics/HintsServiceMetrics.java:
##########
@@ -39,9 +44,17 @@ public final class HintsServiceMetrics
private static final MetricNameFactory factory = new
DefaultNameFactory(TYPE_NAME);
+ // About the delivery of hints
public static final Meter hintsSucceeded =
Metrics.meter(factory.createMetricName("HintsSucceeded"));
public static final Meter hintsFailed =
Metrics.meter(factory.createMetricName("HintsFailed"));
public static final Meter hintsTimedOut =
Metrics.meter(factory.createMetricName("HintsTimedOut"));
+ public static final Gauge<Long> hintsFileSize =
Metrics.gauge(factory.createMetricName("HintsFileSize"), new
TotalHintsSizeGauge());
+ // Corresponding to the hinted_handoff_throttle_in_kb configuration
+ public static final Counter hintsThrottle =
Metrics.counter(factory.createMetricName("HintsThrottle"));
Review Comment:
Add a comment explaining the unit or scale of `hintsThrottle` (e.g., bytes
or KB) so consumers know what `inc(...)` represents.
##########
test/distributed/org/apache/cassandra/distributed/test/metrics/HintsServiceMetricsTest.java:
##########
@@ -108,15 +108,24 @@ public void testHintsServiceMetrics() throws Exception
dropWritesForNode2.set(true);
for (int i = 0; i < NUM_ROWS / 2; i++)
coordinator.execute(withKeyspace("INSERT INTO %s.t (k, v)
VALUES (?, ?)"), QUORUM, i, i);
+ // some hints have created for node1, so file size must be greater
than 0
+ waitUntilAsserted(() ->
assertThat(countHintsFileSize(node1)).isGreaterThan(0));
dropWritesForNode2.set(false);
// write the second half of the rows with the third node dropping
mutations requests,
// so some hints will be created for that node
dropWritesForNode3.set(true);
for (int i = NUM_ROWS / 2; i < NUM_ROWS; i++)
coordinator.execute(withKeyspace("INSERT INTO %s.t (k, v)
VALUES (?, ?)"), QUORUM, i, i);
+ // another hints have created for node1, so file size must be
greater than 0
Review Comment:
[nitpick] Comment grammar: consider rephrasing to "Additional hints have
been created for node1, so file size must be greater than 0."
```suggestion
// Additional hints have been created for node1, so file size
must be greater than 0
```
##########
src/java/org/apache/cassandra/metrics/CassandraMetricsRegistry.java:
##########
@@ -318,6 +318,11 @@ public Histogram histogram(MetricName name, MetricName
alias, boolean considerZe
return histogram;
}
+ public <T extends Gauge<?>> T gauge(MetricName name, T gauge)
Review Comment:
[nitpick] Consider adding a Javadoc comment to explain how this overload
differs from the alias-based version and when to use each.
--
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: pr-unsubscr...@cassandra.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: pr-unsubscr...@cassandra.apache.org
For additional commands, e-mail: pr-h...@cassandra.apache.org
