Thanks Joel. I'll fix up the pics to make them consistent on nomenclature. B
On Fri, Jan 6, 2017 at 2:39 AM Joel Koshy <jjkosh...@gmail.com> wrote: > (adding the dev list back - as it seems to have gotten dropped earlier in > this thread) > > On Thu, Jan 5, 2017 at 6:36 PM, Joel Koshy <jjkosh...@gmail.com> wrote: > > > +1 > > > > This is a very well-written KIP! > > Minor: there is still a mix of terms in the doc that references the > > earlier LeaderGenerationRequest (which is what I'm assuming what it was > > called in previous versions of the wiki). Same for the diagrams which I'm > > guessing are a little harder to make consistent with the text. > > > > > > > > On Thu, Jan 5, 2017 at 5:54 PM, Jun Rao <j...@confluent.io> wrote: > > > >> 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 > >> > > > > >> > > > > >> > > > > >> > > > > >> > > > > >> > > > >> > > >> > > > > >
