kevin-wu24 commented on code in PR #18304:
URL: https://github.com/apache/kafka/pull/18304#discussion_r1934784295
##########
raft/src/test/java/org/apache/kafka/raft/KafkaRaftClientReconfigTest.java:
##########
@@ -2351,4 +2459,21 @@ private static ApiVersionsResponseData
apiVersionsResponse(Errors error, Support
.setErrorCode(error.code())
.setSupportedFeatures(supportedFeatures);
}
+
+ private static KafkaMetric getMetric(final Metrics metrics, final String
name) {
+ return metrics.metrics().get(metrics.metricName(name, "raft-metrics"));
+ }
+
+ private static void checkLeaderMetricValues(
+ final int expectedVoters,
+ final int expectedObservers,
+ final boolean expectedUncommittedVoterChange,
+ final boolean expectedIgnoredStaticVoters,
Review Comment:
I guess my main question is about the necessity to test this the
`ignored-static-voters` metric in tests that use `RaftClientTestContext`.
Because of the way we initialize the context, this metric will either report
true or false for the entirety of the test. I feel this is not very useful to
test, and that is why I added tests for this metric to
`KRaftControlRecordStateMachineTest`.
My thinking is after version upgrades are implemented I think checking the
metric value in these tests makes more sense (i.e. a case where we initialize
the context in version zero, upgrade to version 1, check the metric is still
false, update the voter set, finally check the metric is true).
--
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]
