+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 > > > > > > > > > > > > > > >
