kevin-wu24 commented on code in PR #20859:
URL: https://github.com/apache/kafka/pull/20859#discussion_r2560048766
##########
raft/src/main/java/org/apache/kafka/raft/KafkaRaftClient.java:
##########
@@ -222,6 +222,8 @@ public final class KafkaRaftClient<T> implements
RaftClient<T> {
private volatile RemoveVoterHandler removeVoterHandler;
private volatile UpdateVoterHandler updateVoterHandler;
+ private volatile boolean canAutoJoin = true;
Review Comment:
It seems to me that the intention of the change is the following semantic:
For each lifetime of the controller process on a KRaft node, the node should
be in the "auto-joining" state at most once (i.e. Observer state &&
`shouldSendAddOrRemoveVoterRequest(state, currentTimeMs)`). This means when we
remove a node and it goes back to Observer state, the removed node will not
automatically join back via `AddVoterRPC`. However, if the user removes the
node as a voter and then the node restarts, the node will still try to
auto-join the voter set.
I think a better name for this boolean is `hasAutoJoined` with a value on
startup of `false`. In `initialize()`, each controller node checks their
`partitionState.lastVoterSet()` and sets `hasAutoJoined = true` if they are a
part of the voter set already. Whenever an observer that has auto-join enabled
successfully joins the voter set, we can set `hasAutoJoined = true`.
The main problem with this functionality is that we don't know if previous
additions to the voter set were a result of auto-join or manually adding the
voter. I'm not sure how big of an issue that is, but there are a lot of edge
cases around this feature that lead to confusing UX (i.e. this node is
auto-joining even though I don't "expect" it to) if we have a semantic like the
above. That is one reason why we wanted to keep the pre-requisite for executing
the auto-join code simple.
--
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]
