Hi, Kevin, Thanks for the KIP. Overall, it looks reasonable to me.
Regarding "The follower should not send the AddRaftVoter request if the leader doesn't support the version, because we do not want to cause the unavailability scenario described above. Therefore, the new field should not be ignored and the sending replica should handle the unsupported version exception and error. ", it seems that RaftManger uses a NetworkClient with discoverBrokerVersions=true. So, the new controller should be able to send a version of the AddRaftVoter request that the leader supports, right? Jun On Tue, Jun 17, 2025 at 9:41 AM Alyssa Huang <ahu...@confluent.io.invalid> wrote: > Thanks for the clarifications. > > 2. I would say it's also important that operators understand how to use the > RPCs. From my perspective this new RPC field was introduced to address an > 'internal-facing' issue in the sense that you wouldn't expect the average > operator to ever set this field to anything but the default 'true'. For > folks who do not understand the nitty gritty of KRaft, I think it would be > nice if our documentation (doesn't necessarily have to be in the RPC field > description) could note that they should use AckWhenCommitted = true / > explain AckWhenCommitted = false doesn't guarantee the new voter set will > be committed properly and may require retries. > > Best, > Alyssa > > On Tue, Jun 17, 2025 at 7:42 AM Kevin Wu <kevin.wu2...@gmail.com> wrote: > > > Hi Alyssa, > > > > Thanks for the feedback. > > 1. Yeah, I guess I do not state explicitly why this issue does not impact > > controllers that are manually added via the AdminClient. I'll add a > section > > to clarify the difference in the situations. > > 2. I touched on this a bit in the Proposed Changes section, but I agree > > with José on the documentation of this field. How the field is set is up > to > > the implementation, which can change over time (what if the AdminClient > > changes in the future to return a response before commitment?), so > > documenting how we're using it now is not as accurate as documenting what > > changes in the KRaft protocol. > > > > Best, > > Kevin Wu > > > > On Thu, Jun 12, 2025 at 11:49 AM Kevin Wu <kevin.wu2...@gmail.com> > wrote: > > > > > Hi Jose, > > > > > > Thanks for the feedback. I agree with the solution of not ignoring the > > new > > > field, and I see how the current documentation is not descriptive in > > terms > > > of what the flag is actually doing within the protocol. I will update > the > > > KIP to change these things. > > > > > > Best, > > > Kevin > > > > > > On Wed, Jun 11, 2025 at 2:55 PM Kevin Wu <kevin.wu2...@gmail.com> > wrote: > > > > > >> Hello all, > > >> > > >> I wrote a KIP to add a new boolean field to the AddRaftVoterRequest > RPC. > > >> Here is the link: > > >> > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1186%3A+Update+AddRaftVoterRequest+RPC+to+support+auto-join > > >> > > >> > > >> Thanks, > > >> Kevin Wu > > >> > > > > > >
