Hi Chris, thanks for the feedback. 1. regarding the default value I had the same conflict of which version to break the backward compatibility with. We can just say that this KIP gives the release Pre KIP-690 the ability to keep the old behaviour with one config and keep the backwards compatibility from post-KIP-690 the same so we don't break at least the last 3 versions. I will update the KIP to switch the default value to true. 2. For the backporting, which versions can we backport these to? Usually, Kafka supports bugfix releases as needed for the last 3 releases. Now we @ 3.5 so the last 3 are 3.4, 3.3 and 3.2 is this correct? 3. I'll add a Jira for updating the docs for this KIP so we don't forget about it.
Thanks Omnia On Mon, Jul 10, 2023 at 5:33 PM Chris Egerton <chr...@aiven.io.invalid> wrote: > Hi Omnia, > > Thanks for taking this on! I have some thoughts but the general approach > looks good. > > 1. Default value > > One thing I'm wrestling with is what the default value of the new property > should be. I know on the Jira ticket I proposed that it should be false, > but I'm having second thoughts. Technically we'd preserve backward > compatibility with pre-KIP-690 releases by defaulting to false, but at the > same time, we'd break compatibility with post-KIP-690 releases. And if we > default to true, the opposite would be true: compatibility would be broken > with pre-KIP-690 releases, but preserved with post-KIP-690 releases. > > One argument against defaulting to false (which, again, would preserve the > behavior of MM2 before we accidentally broke compatibility with KIP-690) is > that this change could possibly cause a single MM2 setup to break > twice--once when upgrading from a pre-KIP-690 release to an existing > release, and again when upgrading from that existing release to a version > that reverted (by default) to pre-KIP-690 behavior. On the other hand, if > we default to true (which would preserve the existing behavior that breaks > compatibility with pre-KIP-690 releases), then any given setup will only be > broken once. > > In addition, if we default to true right now, then we don't have to worry > about changing that default in 4.0 to a more intuitive value (I hope we can > all agree that, for new clusters, it makes sense to set this property to > true and not to distinguish between internal and non-internal topics). > > With that in mind, I'm now leaning more towards defaulting to true, but > would be interested in your thoughts. > > > 2. Backport? > > It's highly unlikely to backport changes for a KIP, but given the impact of > the compatibility break that we're trying to address here, and the > extremely low risk of the proposed changes, I think we should consider > backporting the proposed fix to all affected release branches (i.e., 3.1 > through 3.5). > > > 3. Extra steps > > I also think we can take these additional steps to try to help prevent > users from being bitten by this change: > > - Add a note to our upgrade instructions [1] for all affected versions that > instructs users on how to safely upgrade to a post-KIP-690 release, for > versions that both do and do not include the changes from this KIP > - Log a warning message on MM2 startup if the config contains an explicit > value for "replication.policy.separator" but does not contain an explicit > value for "replication.policy.internal.topic.separator.enabled" > > These details don't necessarily have to be codified in the KIP, but they're > worth taking into account when considering how to design any functional > changes in order to better try to gauge how well this could go for our > users. > > [1] - https://kafka.apache.org/documentation.html#upgrade > > > Thanks again for the KIP! > > Cheers, > > Chris > > On Fri, Jul 7, 2023 at 10:12 AM Omnia Ibrahim <o.g.h.ibra...@gmail.com> > wrote: > > > Hi everyone, > > I want to start the discussion of the KIP-949. The proposal is here > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-949%3A+Add+flag+to+enable+the+usage+of+topic+separator+in+MM2+DefaultReplicationPolicy > > < > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-949%3A+Add+flag+to+enable+the+usage+of+topic+separator+in+MM2+DefaultReplicationPolicy > > > > > > > Thanks for your time and feedback. > > Omnia > > >
