jsancio commented on code in PR #18304:
URL: https://github.com/apache/kafka/pull/18304#discussion_r1901076998
##########
raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java:
##########
@@ -3515,6 +3519,14 @@ public Optional<Node> voterNode(int id, ListenerName
listenerName) {
return partitionState.lastVoterSet().voterNode(id, listenerName);
}
+ public int ignoredStaticVoters() {
Review Comment:
Let's return a boolean and let the metrics exporter convert the boolean to a
1 or 0.
Once this value reports true it will continue to report true for the entire
lifetime of the JVM/replica. Did you consider a different implementation that
doesn't increase the number of public methods of `KafkaRaftClient`? Maybe we
can store this value in the metrics objects and update it when handling
`KRaftControlRecordStateMachine#handleBatch`.
##########
raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java:
##########
@@ -3515,6 +3519,14 @@ public Optional<Node> voterNode(int id, ListenerName
listenerName) {
return partitionState.lastVoterSet().voterNode(id, listenerName);
}
+ public int ignoredStaticVoters() {
+ return partitionState.lastVoterSetOffset().isPresent() ? 1 : 0;
+ }
+
+ public int isObserver() {
+ return quorum.isObserver() ? 1 : 0;
+ }
Review Comment:
Let's return a boolean and let the metrics exporter convert the boolean to a
1 or 0.
Same here. Did you consider a different implementation that doesn't increase
the number of public methods of `KafkaRaftClient`? Maybe we don't need to
implement this metric since KRaft's `current-state` metrics already reports
half of this information. I need to see if there is a metrics that returns 1
when the process role includes controller.
##########
raft/src/main/java/org/apache/kafka/raft/internals/KafkaRaftMetrics.java:
##########
@@ -215,6 +227,22 @@ public void maybeUpdateElectionLatency(long currentTimeMs)
{
}
}
+ public <T> void addLeaderMetrics(LeaderState<T> leaderState) {
+ metrics.addMetric(numObserversMetricName, (Gauge<Integer>) (config,
now) -> leaderState.numObservers());
+ metrics.addMetric(uncommittedVoterChangeMetricName, (Gauge<Integer>)
(config, now) -> {
+ if (leaderState.addVoterHandlerState().isEmpty() &&
leaderState.removeVoterHandlerState().isEmpty()) {
+ return 0;
+ } else {
+ return 1;
+ }
+ });
+ }
Review Comment:
This lambda are not thread safe. `LeaderState` assumes that the kraft driver
thread is the only reader and writer.
--
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]
