kevin-wu24 commented on code in PR #19589:
URL: https://github.com/apache/kafka/pull/19589#discussion_r2076162642
##########
raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java:
##########
@@ -3321,34 +3321,35 @@ private long pollFollowerAsObserver(FollowerState
state, long currentTimeMs) {
if (state.hasFetchTimeoutExpired(currentTimeMs)) {
return maybeSendFetchToAnyBootstrap(currentTimeMs);
} else if (partitionState.lastKraftVersion().isReconfigSupported() &&
followersAlwaysFlush &&
- quorumConfig.autoJoinEnable() &&
state.hasAddRemoveVoterPeriodExpired(currentTimeMs)) {
- var voters = partitionState.lastVoterSet();
+ quorumConfig.autoJoin() &&
state.hasUpdateVoterPeriodExpired(currentTimeMs)) {
+ /* `followersAlwaysFlush` is true when the config `process.roles`
contains "controller".
+ * We require both `followersAlwaysFlush` and `autoJoin` to be
true because
+ * brokers should not be able to add themselves as a KRaft voter.
+ */
var localReplicaKey = quorum.localReplicaKeyOrThrow();
- final boolean resetAddRemoveVoterTimer;
+ final boolean resetUpdateVoterTimer;
final long backoffMs;
-
- Optional<ReplicaKey> oldVoter =
voters.getOldVoterForReplicaKey(localReplicaKey);
- if (oldVoter.isPresent()) {
- var sendResult = maybeSendRemoveVoterRequest(state,
currentTimeMs, oldVoter.get());
- resetAddRemoveVoterTimer = sendResult.requestSent();
- backoffMs = sendResult.timeToWaitMs();
- } else if (voters.doesNotContainReplicaId(localReplicaKey)) {
- var sendResult = maybeSendAddVoterRequest(state,
currentTimeMs);
- resetAddRemoveVoterTimer = sendResult.requestSent();
- backoffMs = sendResult.timeToWaitMs();
+ var voters = partitionState.lastVoterSet();
+ RequestSendResult sendResult;
Review Comment:
Should this also apply to `localReplicaKey` and `voters` (and potentially
many other places)? We do not reassign them nor want to. I read from
https://stackoverflow.com/a/316787 this is in general good practice for
readability + debugging and can allow for the compiler to optimize the class
file since the value is constant.
Java really needs `val` like Scala.
--
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: jira-unsubscr...@kafka.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
