Thanks for the KIP, Apurva. In general, I think it's a good idea to strengthen the guarantees we provide by default. And people who are willing to trade correctness for performance can then change the configs to suit them. I will comment on the KIP specifics in more detail later, but one additional comment inline:
On Wed, Aug 9, 2017 at 7:11 AM, Ewen Cheslack-Postava <e...@confluent.io> wrote: > 3. The acks=all change is actually unrelated to the title of the KIP and > orthogonal to all the other changes. It's also the most risky since > acks=all needs more network round trips. And while I think it makes sense > to have the more durable default, this seems like it's actually fairly > likely to break things for some people (even if a minority of people). This > one seems like a setting change that needs more sensitive handling, e.g. > both release notes and log notification that the default is going to > change, followed by the actual change later. > The issue is that with acks=1 and idempotence, OutOfOrderSequenceException may be thrown, which is a fatal error for the Producer (it needs to be closed and restarted). I'll leave it to Apurva to explain this in more detail. I wanted to comment on the "log notification" suggestion. Why do you think this is needed since users can just change the config back to `acks=1` (or 0)? We haven't done this in the past when changing defaults, so it would be good to understand it. Given that the next release is 1.0.0, I think it would be OK to just change it and advertise it well. Logging warnings for deprecated configs makes sense because: 1. The config will go away and there may not be an exact replacement, so good to give some time for users to transition 2. Users should not be using the config, so it's OK to spam their logs Neither of those is true when we change defaults. Having said that, Git does what you are suggesting and I agree that the impact can be negative if people don't read the upgrade notes. Not sure what's the best way to solve that. Ismael
