Hi, Ben, Thanks for the updated KIP. +1
1) In OffsetForLeaderEpochResponse, start_offset probably should be end_offset since it's the end offset of that epoch. 3) That's fine. We can fix KAFKA-1120 separately. Jun On Thu, Jan 5, 2017 at 11:11 AM, Ben Stopford <b...@confluent.io> wrote: > Hi Jun > > Thanks for raising these points. Thorough as ever! > > 1) Changes made as requested. > 2) Done. > 3) My plan for handing returning leaders is to simply to force the Leader > Epoch to increment if a leader returns. I don't plan to fix KAFKA-1120 as > part of this KIP. It is really a separate issue with wider implications. > I'd be happy to add KAFKA-1120 into the release though if we have time. > 4) Agreed. Not sure exactly how that's going to play out, but I think we're > on the same page. > > Please could > > Cheers > B > > On Thu, Jan 5, 2017 at 12:50 AM Jun Rao <j...@confluent.io> wrote: > > > Hi, Ben, > > > > Thanks for the proposal. Looks good overall. A few comments below. > > > > 1. For LeaderEpochRequest, we need to include topic right? We probably > want > > to follow other requests by nesting partition inside topic? For > > LeaderEpochResponse, > > do we need to return leader_epoch? I was thinking that we could just > return > > an end_offset, which is the next offset of the last message in the > > requested leader generation. Finally, would OffsetForLeaderEpochRequest > be > > a better name? > > > > 2. We should bump up both the produce request and the fetch request > > protocol version since both include the message set. > > > > 3. Extending LeaderEpoch to include Returning Leaders: To support this, > do > > you plan to use the approach that stores CZXID in the broker > registration > > and including the CZXID of the leader in /brokers/topics/[topic]/ > > partitions/[partitionId]/state in ZK? > > > > 4. Since there are a few other KIPs involving message format too, it > would > > be useful to consider if we could combine the message format changes in > the > > same release. > > > > Thanks, > > > > Jun > > > > > > On Wed, Jan 4, 2017 at 9:24 AM, Ben Stopford <b...@confluent.io> wrote: > > > > > Hi All > > > > > > We’re having some problems with this thread being subsumed by the > > > [Discuss] thread. Hopefully this one will appear distinct. If you see > > more > > > than one, please use this one. > > > > > > KIP-101 should now be ready for a vote. As a reminder the KIP proposes > a > > > change to the replication protocol to remove the potential for replicas > > to > > > diverge. > > > > > > The KIP can be found here: https://cwiki.apache.org/confl > > > uence/display/KAFKA/KIP-101+-+Alter+Replication+Protocol+to+ > > > use+Leader+Epoch+rather+than+High+Watermark+for+Truncation < > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-101+- > > > +Alter+Replication+Protocol+to+use+Leader+Epoch+rather+ > > > than+High+Watermark+for+Truncation> > > > > > > Please let us know your vote. > > > > > > B > > > > > > > > > > > > > > > > > >
