Thanks, everyone. I'm going to close the vote tomorrow if there are no more comments or votes.
regards, Colin On Thu, Oct 26, 2017, at 08:09, Manikumar wrote: > Thanks for the KIP. > +1 (non-binding) > > > On Thu, Oct 26, 2017 at 5:58 AM, Jason Gustafson <ja...@confluent.io> > wrote: > > > +1. Thanks for the KIP. > > > > On Mon, Oct 23, 2017 at 11:30 AM, Colin McCabe <cmcc...@apache.org> wrote: > > > > > On Mon, Oct 23, 2017, at 10:29, Jason Gustafson wrote: > > > > Thanks for the KIP. I'm assuming the new behavior only affects > > > > ListOffsets requests from the consumer. > > > > > > That's a very good point. I will add a caveat that we only apply the > > > KIP-207 behavior to requests from clients, not requests from other > > > brokers (such as the ones made by ReplicaFetcherThread). > > > > > > > Might be worth mentioning that in the KIP. > > > > Also, does it affect all ListOffsets requests, or only those that > > specify > > > > the latest offset? > > > > > > I don't feel great about allowing someone to ask for the offset at time > > > T, get back X, and then ask again for the offset at T the next second > > > and get back InvalidOffsetException. So it's probably best just to > > > apply the KIP-207 behavior to all ListOffsets requests from consumers. > > > > > > Thinking about it a bit more, we should disable the KIP-207 behavior > > > when unclean leader elections are enabled on the broker. When unclean > > > leader elections are enabled, data loss is possible. So we cannot > > > guarantee that offsets will always go forwards, even in theory, in this > > > mode. > > > > > > I update the kip-- check it out. > > > > > > best, > > > Colin > > > > > > > > > > > > > > -Jason > > > > > > > > On Wed, Oct 18, 2017 at 9:15 AM, Colin McCabe <cmcc...@apache.org> > > > wrote: > > > > > > > > > On Wed, Oct 18, 2017, at 04:09, Ismael Juma wrote: > > > > > > Thanks for the KIP, +1 (binding). A few comments: > > > > > > > > > > > > 1. I agree with Jun about LEADER_NOT_AVAILABLE for the error code > > for > > > > > > older > > > > > > versions. > > > > > > 2. OffsetNotAvailableException seems clear enough (i.e. we don't > > > need the > > > > > > "ForPartition" part) > > > > > > > > > > Yeah, that is shorter and probably clearer. Changed. > > > > > > > > > > > 3. The KIP seems to be missing the compatibility section. > > > > > > > > > > Added. > > > > > > > > > > > 4. It would be good to mention that it's now possible for a fetch > > to > > > > > > succeed while list offsets will not for a period of time. And for > > > older > > > > > > versions, the latter will return LeaderNotAvailable while the > > former > > > > > > would > > > > > > work fine, which is a bit unexpected. Not much we can do about it, > > > but > > > > > > worth mentioning it in my opinion. > > > > > > > > > > Fair enough > > > > > > > > > > cheers, > > > > > Colin > > > > > > > > > > > > > > > > > Ismael > > > > > > > > > > > > On Tue, Oct 17, 2017 at 9:26 PM, Jun Rao <j...@confluent.io> wrote: > > > > > > > > > > > > > Hi, Colin, > > > > > > > > > > > > > > Thanks for the KIP. +1. Just a minor comment. For the old client > > > > > requests, > > > > > > > would it be better to return a LEADER_NOT_AVAILABLE error > > instead? > > > > > > > > > > > > > > Jun > > > > > > > > > > > > > > On Tue, Oct 17, 2017 at 11:11 AM, Colin McCabe < > > cmcc...@apache.org > > > > > > > > > wrote: > > > > > > > > > > > > > > > Hi all, > > > > > > > > > > > > > > > > I'd like to start the voting process for KIP-207:The Offsets > > > which > > > > > > > > ListOffsetsResponse returns should monotonically increase even > > > > > during a > > > > > > > > partition leader change. > > > > > > > > > > > > > > > > See > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP- > > > > > > > > 207%3A+Offsets+returned+by+ListOffsetsResponse+should+be+ > > > > > > > > monotonically+increasing+even+during+a+partition+leader+change > > > > > > > > for details. > > > > > > > > > > > > > > > > The voting process will run for at least 72 hours. > > > > > > > > > > > > > > > > regards, > > > > > > > > Colin > > > > > > > > > > > > > > > > > > > > > > > > >
