Hi, Alyssa, Thanks for the KIP. A few comments below.
10. "If a server happens to receive multiple VoteResponses from another server for a particular VoteRequest, it can take the first and ignore the rest.": Could you explain why a server would receive multiple responses for the same request? 11. "e.g. S1 in the below diagram pg. 41)": What is pg. 41? 12. "if a server attempts to send out a Pre-Vote request while any other server in the quorum does not understand it, it will get back an UnsupportedVersionException from the network client and knows to default back to the old behavior." 12.1 Based on ApiVersion, a server knows whether a peer supports PreVote or not. If it doesn't, there is no need for the server to send a PreVote request only to be rejected, right? 12.2 What happens when some servers understand PreVote while some others don't? Thanks, Jun On Wed, Nov 29, 2023 at 2:11 AM Luke Chen <show...@gmail.com> wrote: > Hi Alyssa, > > Thanks for the KIP! > This is an important improvement for KRaft quorum. > > Some comments: > 1. Follower transitions to: Prospective: After expiration of the election > timeout > -> Is this the fetch timeout, not election timeout? > > 2. I also agree we don't bump the epoch in prospective state. > A candidate will now send a VoteRequest with the PreVote field set to true > and CandidateEpoch set to its [epoch + 1] when its election timeout > expires. > -> What is "CandidateEpoch"? And I thought you've agreed to not set [epoch > + 1] ? > > Thanks. > Luke > > On Wed, Nov 29, 2023 at 2:06 AM Alyssa Huang <ahu...@confluent.io.invalid> > wrote: > > > Thanks Jose, I've updated the KIP to reflect your and Jason's > suggestions! > > > > On Tue, Nov 28, 2023 at 9:54 AM José Armando García Sancio > > <jsan...@confluent.io.invalid> wrote: > > > > > Hi Alyssa > > > > > > On Mon, Nov 27, 2023 at 1:40 PM Jason Gustafson > > > <ja...@confluent.io.invalid> wrote: > > > > 2. Do you think the pretend epoch bump is necessary? Would it be > > simpler > > > to > > > > change the prevote acceptance check to assert a greater than or equal > > > epoch? > > > > > > I agree with Jason it would be better if all of the requests always > > > sent the current epoch. For the VoterRequest, it should be correct for > > > the prospective node to not increase the epoch and send the current > > > epoch and id. Since there are two states (prospective and candidate) > > > that can send a VoteRequest, maybe we can change the field name to > > > just ReplicaEpoch and ReplicaId. > > > > > > Thanks, > > > -- > > > -José > > > > > >
