Hi Chris, I added a section for backport plan here https://cwiki.apache.org/confluence/display/KAFKA/KIP-949%3A+Add+flag+to+enable+the+usage+of+topic+separator+in+MM2+DefaultReplicationPolicy#KIP949:AddflagtoenabletheusageoftopicseparatorinMM2DefaultReplicationPolicy-Backportingplan
Cheers, Omnia > On 13 Jul 2023, at 19:33, Chris Egerton <chr...@aiven.io.INVALID> wrote: > > Hi Omnia, > > Yes, I think we ought to state the backport plan in the KIP, since it's > highly unusual for KIP changes or new configuration properties to be > backported and we should get the approval of the community (including > binding and non-binding voters) before implementing it. > > Cheers, > > Chris > > On Thu, Jul 13, 2023 at 7:13 AM Omnia Ibrahim <o.g.h.ibra...@gmail.com> > wrote: > >> Hi Chris, >> The implementation should be very small so backporting this to 3.1 and 3.2 >> would be perfect for this case if you or any other committer are okay with >> approving the backporting. Do we need to state this in KIP as well or not? >> >> Also, I’ll open a vote for the KIP today and prepare the pr for it so we >> can merge it as soon as we can. >> >> Thanks, >> >> Omnia >> >> On Wed, Jul 12, 2023 at 4:31 PM Chris Egerton <chr...@aiven.io.invalid> >> wrote: >> >>> Hi Omnia, >>> >>> Thanks for changing the default, LGTM 👍 >>> >>> As far as backporting goes, we probably won't be doing another release >> for >>> 3.1, and possibly not for 3.2 either; however, if we can make the >>> implementation focused enough (which I don't think would be too >> difficult, >>> but correct me if I'm wrong), then we can still backport through 3.1. >> Even >>> if we don't do another release it can make life easier for people who are >>> maintaining parallel forks. Obviously this shouldn't be taken as a >> blanket >>> precedent but in this case it seems like the benefits may outweigh the >>> costs. What are your thoughts? >>> >>> Cheers, >>> >>> Chris >>> >>> On Wed, Jul 12, 2023 at 9:05 AM Omnia Ibrahim <o.g.h.ibra...@gmail.com> >>> wrote: >>> >>>> 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 >>>>>> >>>>> >>>> >>> >>
