Thanks Apurva, On compatibility: I think the proposal makes sense. It's a pity that we can't support idempotence for 0.11.0.0 brokers in the "safe" mode even if it is supported by the broker. I can already imagine users complaining about this, but I guess it's the consequence of missing the impact of that validation check and not thinking through the ultimate goal of enabling idempotence by default. A couple minor comments:
1. Instead of "safe," Ismael suggested "requested" as an alternative. That seems to suggest more clearly that idempotence will only be used when the broker supports it. 2. Should we deprecate the "true" and "false" options? It's a little weird long term to support them in addition to the descriptive names. On the OutOfOrderSequence proposal: high-level, the design makes sense. A couple questions: 1. With this proposal, OutOfOrderSequence means that we must have a last produced offset. Is the idea to expose that in the OutOfOrderSequenceException so that users know which data was lost? 2. Previously we discussed duplicate handling. Currently we raise OutOfOrderSequence if we happen to get a sequence number which is earlier than the sequence numbers we have cached. Alternatively, you suggested we can return a separate DuplicateError for this case, which clients can ignore if they do not care about the offset. I think it might make sense to include that here so that the OutOfOrderSequence error is unambiguous. Finally, do you plan to roll these proposals into the current KIP or do them separately? Probably makes sense to combine them since they both require a bump to the ProduceRequest. Thanks, Jason On Fri, Aug 18, 2017 at 5:18 PM, Apurva Mehta <apu...@confluent.io> wrote: > Thanks Jason and Ismael. > > The message format problem is an acute one: if we enable idempotence by > default, the UnsupportedVersionException when writing to topics with the > older message format would mean that our prescribed upgrade steps would not > work. I have detailed the problems and the solutions on this page (linked > to from the wiki): > > https://cwiki.apache.org/confluence/display/KAFKA/ > Kafka+Exactly+Once+-+Dealing+with+older+message+formats+ > when+idempotence+is+enabled > > It is worth discussing the solution to the problem proposed there. If it is > conceptually sound, it doesnt' seem too hard to implement. > > As far as the other problem of the spurious OutOfOrderSequence problem, I > have documented a proposed solution here: > > https://cwiki.apache.org/confluence/display/KAFKA/ > Kafka+Exactly+Once+-+Solving+the+problem+of+spurious+ > OutOfOrderSequence+errors > > This solution is a bit more involved in terms of effort. > > I think we cannot make the idempotent producer the default unless we solve > the message format compatibility problem. I would also prefer to solve the > second problem before making idempotence the default. > > I would be interested to hear everyone's thoughts on the two solutions > proposed above. > > Thanks, > Apurva > > On Fri, Aug 18, 2017 at 9:24 AM, Jason Gustafson <ja...@confluent.io> > wrote: > > > > > > > so this change will break client backward compatibility while > connecting > > > to 0.10.X brokers. > > > users need to change producer default settings while connecting older > > > brokers. > > > > > > At the moment, I think the answer is yes. The old broker will not support > > the InitProducerId request, so the producer will immediately fail. > Similar > > to the handling of old message formats mentioned above, we probably need > to > > change this so that we just revert to old producer semantics if the > broker > > can't support idempotence. > > > > -Jason > > > > > > On Fri, Aug 18, 2017 at 8:48 AM, Manikumar <manikumar.re...@gmail.com> > > wrote: > > > > > > > > > > 3. The message format requirement is a good point. This should be > > > mentioned > > > > in the compatibility section. Users who are still using the old > message > > > > format will get an error after the upgrade, right? > > > > > > > > > > so this change will break client backward compatibility while > connecting > > > to 0.10.X brokers. > > > users need to change producer default settings while connecting older > > > brokers. > > > > > >
