Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Omnia Ibrahim]

Thanks for both of your feedback and time, I updated the rejected
alternative section in the KIP and opened a voting thread here
https://lists.apache.org/thread/cy04n445noyp0pqztlp8rk74crvhlrk7
I'll work on the PR in meanwhile so we are ready to go once we get 3
binding votes in order to get into 3.6 release.

Cheers
Omnia

On Fri, Jul 21, 2023 at 3:43 PM Federico Valeri 
wrote:

> Hi, the point that the legacy approach can only be taken once is
> valid, so LGTM. Thanks.
>
> Cheers
> Fede
>
> On Fri, Jul 21, 2023 at 4:28 PM Chris Egerton 
> wrote:
> >
> > Hi Omnia,
> >
> > LGTM, thanks! We may want to note the LegacyReplicationPolicy option in
> the
> > rejected alternatives section in case others prefer that option.
> >
> > Given that we'd like this to land in time for 3.6.0, you may want to
> start
> > a vote thread soon.
> >
> > Cheers,
> >
> > Chris
> >
> > On Fri, Jul 21, 2023 at 10:08 AM Omnia Ibrahim 
> > wrote:
> >
> > > Hi Chris and Federico,
> > > thinking about I think Chris's concern is valid and the bigger concern
> here
> > > is that having this `LegacyReplicationPolicy` will eventually open the
> door
> > > for diversion at some point between this `LegacyReplicationPolicy` and
> the
> > > default one.
> > > For now, let's have the flag properly fix this bug and we can keep it
> as an
> > > option for people to switch between both behaviours. I know having a
> > > bug-fix property is not great but we can treat it as a backward
> > > compatibility property instead in order to keep old mirrors using the
> old
> > > internal topics.
> > >
> > > Hope this is reasonable for the time being.
> > >
> > > Cheers,
> > > Omnia
> > >
> > > On Wed, Jul 19, 2023 at 11:16 PM Chris Egerton  >
> > > wrote:
> > >
> > > > Hi Federico,
> > > >
> > > > Ah yes, sorry about that! You're correct that this would keep the two
> > > > classes in line and largely eliminate the concern I posed about
> porting
> > > > changes to both. Still, I'm a bit hesitant, and I'm not actually
> certain
> > > > that this alternative is more intuitive. The name isn't very
> descriptive,
> > > > and this is the kind of approach we can only really take once; if we
> > > break
> > > > compatibility again, would we introduce a
> LegacyLegacyReplicationPolicy?
> > > > LegacyReplicationPolicy2? Finally, it seems a bit strange to
> introduce a
> > > > new class to implement a change in behavior this small.
> > > >
> > > > That said, I don't think this is worth blocking on and wouldn't be
> > > opposed
> > > > if others felt strongly that a new replication policy class is
> superior
> > > to
> > > > a new property on the existing class.
> > > >
> > > > Cheers,
> > > >
> > > > Chris
> > > >
> > > > On Wed, Jul 19, 2023 at 2:53 PM Federico Valeri <
> fedeval...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi Chris, the KIP says it would be a subclass of
> > > DefaultReplicationPolicy
> > > > > that overrides the ReplicationPolicy.offsetSyncsTopic and
> > > > > ReplicationPolicy.checkpointsTopic. So, not much to maintain and it
> > > would
> > > > > be more intuitive, as you say.
> > > > >
> > > > > On Wed, Jul 19, 2023, 4:50 PM Chris Egerton
> 
> > > > > wrote:
> > > > >
> > > > > > HI all,
> > > > > >
> > > > > > I'm not sure I understand the benefits of introducing a separate
> > > > > > replication policy class, besides maybe being more
> readable/intuitive
> > > > to
> > > > > > users who would want to know when to use one or the other. It
> feels
> > > > like
> > > > > > we've swapped out a "fix the bug" property for an entire "fix the
> > > bug"
> > > > > > class, and it still leaves the problem of graceful migration from
> > > > legacy
> > > > > > behavior to new behavior unsolved. It would also require us to
> > > consider
> > > > > > whether any future changes we make to the
> DefaultReplicationPolicy
> > > > class
> > > > > > would have to be ported over to the LegacyReplicationPolicy
> class as
> > > > > well.
> > > > > >
> > > > > > Perhaps I'm missing something; are there other benefits of
> > > introducing
> > > > a
> > > > > > separate replication policy class?
> > > > > >
> > > > > > Cheers,
> > > > > >
> > > > > > Chris
> > > > > >
> > > > > > On Wed, Jul 19, 2023 at 5:45 AM Omnia Ibrahim <
> > > o.g.h.ibra...@gmail.com
> > > > >
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Federico,
> > > > > > > I like the idea of implementing `LegacyReplicationPolicy` and
> > > > avoiding
> > > > > > bug
> > > > > > > fixes properties. We can drop the idea of the property and
> just go
> > > > > ahead
> > > > > > > with introducing the `LegacyReplicationPolicy` and any user
> upgrade
> > > > > from
> > > > > > > pre-KIP-690 can use this policy instead of default and no
> impact
> > > will
> > > > > > > happen to users upgrading from 3.x to post-KIP-949. We can mark
> > > > > > > `LegacyReplicationPolicy` as deprecated later if we want (but
> not
> > > in
> > > > > 4.0
> > > > > > as
> > > > > > > this is very soon) and we can drop it 

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Federico Valeri]

Hi, the point that the legacy approach can only be taken once is
valid, so LGTM. Thanks.

Cheers
Fede

On Fri, Jul 21, 2023 at 4:28 PM Chris Egerton  wrote:
>
> Hi Omnia,
>
> LGTM, thanks! We may want to note the LegacyReplicationPolicy option in the
> rejected alternatives section in case others prefer that option.
>
> Given that we'd like this to land in time for 3.6.0, you may want to start
> a vote thread soon.
>
> Cheers,
>
> Chris
>
> On Fri, Jul 21, 2023 at 10:08 AM Omnia Ibrahim 
> wrote:
>
> > Hi Chris and Federico,
> > thinking about I think Chris's concern is valid and the bigger concern here
> > is that having this `LegacyReplicationPolicy` will eventually open the door
> > for diversion at some point between this `LegacyReplicationPolicy` and the
> > default one.
> > For now, let's have the flag properly fix this bug and we can keep it as an
> > option for people to switch between both behaviours. I know having a
> > bug-fix property is not great but we can treat it as a backward
> > compatibility property instead in order to keep old mirrors using the old
> > internal topics.
> >
> > Hope this is reasonable for the time being.
> >
> > Cheers,
> > Omnia
> >
> > On Wed, Jul 19, 2023 at 11:16 PM Chris Egerton 
> > wrote:
> >
> > > Hi Federico,
> > >
> > > Ah yes, sorry about that! You're correct that this would keep the two
> > > classes in line and largely eliminate the concern I posed about porting
> > > changes to both. Still, I'm a bit hesitant, and I'm not actually certain
> > > that this alternative is more intuitive. The name isn't very descriptive,
> > > and this is the kind of approach we can only really take once; if we
> > break
> > > compatibility again, would we introduce a LegacyLegacyReplicationPolicy?
> > > LegacyReplicationPolicy2? Finally, it seems a bit strange to introduce a
> > > new class to implement a change in behavior this small.
> > >
> > > That said, I don't think this is worth blocking on and wouldn't be
> > opposed
> > > if others felt strongly that a new replication policy class is superior
> > to
> > > a new property on the existing class.
> > >
> > > Cheers,
> > >
> > > Chris
> > >
> > > On Wed, Jul 19, 2023 at 2:53 PM Federico Valeri 
> > > wrote:
> > >
> > > > Hi Chris, the KIP says it would be a subclass of
> > DefaultReplicationPolicy
> > > > that overrides the ReplicationPolicy.offsetSyncsTopic and
> > > > ReplicationPolicy.checkpointsTopic. So, not much to maintain and it
> > would
> > > > be more intuitive, as you say.
> > > >
> > > > On Wed, Jul 19, 2023, 4:50 PM Chris Egerton 
> > > > wrote:
> > > >
> > > > > HI all,
> > > > >
> > > > > I'm not sure I understand the benefits of introducing a separate
> > > > > replication policy class, besides maybe being more readable/intuitive
> > > to
> > > > > users who would want to know when to use one or the other. It feels
> > > like
> > > > > we've swapped out a "fix the bug" property for an entire "fix the
> > bug"
> > > > > class, and it still leaves the problem of graceful migration from
> > > legacy
> > > > > behavior to new behavior unsolved. It would also require us to
> > consider
> > > > > whether any future changes we make to the DefaultReplicationPolicy
> > > class
> > > > > would have to be ported over to the LegacyReplicationPolicy class as
> > > > well.
> > > > >
> > > > > Perhaps I'm missing something; are there other benefits of
> > introducing
> > > a
> > > > > separate replication policy class?
> > > > >
> > > > > Cheers,
> > > > >
> > > > > Chris
> > > > >
> > > > > On Wed, Jul 19, 2023 at 5:45 AM Omnia Ibrahim <
> > o.g.h.ibra...@gmail.com
> > > >
> > > > > wrote:
> > > > >
> > > > > > Hi Federico,
> > > > > > I like the idea of implementing `LegacyReplicationPolicy` and
> > > avoiding
> > > > > bug
> > > > > > fixes properties. We can drop the idea of the property and just go
> > > > ahead
> > > > > > with introducing the `LegacyReplicationPolicy` and any user upgrade
> > > > from
> > > > > > pre-KIP-690 can use this policy instead of default and no impact
> > will
> > > > > > happen to users upgrading from 3.x to post-KIP-949. We can mark
> > > > > > `LegacyReplicationPolicy` as deprecated later if we want (but not
> > in
> > > > 4.0
> > > > > as
> > > > > > this is very soon) and we can drop it entirely at some point.
> > > > > >
> > > > > > If there's an agreement on this approach I can upgrade the KIP.
> > > > > >
> > > > > > Cheers.
> > > > > > Omnia
> > > > > >
> > > > > > On Wed, Jul 19, 2023 at 8:52 AM Federico Valeri <
> > > fedeval...@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hi, one way to avoid the "fix the bug property" would be to
> > provide
> > > > > > > and document an additional LegacyReplicationPolicy, but still
> > > keeping
> > > > > > > the current DefaultReplicationPolicy as replication.policy.class
> > > > > > > default value, which is basically one of the workarounds
> > suggested
> > > in
> > > > > > > the KIP.
> > > > > > >
> > > > > > > On Tue, Jul 

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Chris Egerton]

Hi Omnia,

LGTM, thanks! We may want to note the LegacyReplicationPolicy option in the
rejected alternatives section in case others prefer that option.

Given that we'd like this to land in time for 3.6.0, you may want to start
a vote thread soon.

Cheers,

Chris

On Fri, Jul 21, 2023 at 10:08 AM Omnia Ibrahim 
wrote:

> Hi Chris and Federico,
> thinking about I think Chris's concern is valid and the bigger concern here
> is that having this `LegacyReplicationPolicy` will eventually open the door
> for diversion at some point between this `LegacyReplicationPolicy` and the
> default one.
> For now, let's have the flag properly fix this bug and we can keep it as an
> option for people to switch between both behaviours. I know having a
> bug-fix property is not great but we can treat it as a backward
> compatibility property instead in order to keep old mirrors using the old
> internal topics.
>
> Hope this is reasonable for the time being.
>
> Cheers,
> Omnia
>
> On Wed, Jul 19, 2023 at 11:16 PM Chris Egerton 
> wrote:
>
> > Hi Federico,
> >
> > Ah yes, sorry about that! You're correct that this would keep the two
> > classes in line and largely eliminate the concern I posed about porting
> > changes to both. Still, I'm a bit hesitant, and I'm not actually certain
> > that this alternative is more intuitive. The name isn't very descriptive,
> > and this is the kind of approach we can only really take once; if we
> break
> > compatibility again, would we introduce a LegacyLegacyReplicationPolicy?
> > LegacyReplicationPolicy2? Finally, it seems a bit strange to introduce a
> > new class to implement a change in behavior this small.
> >
> > That said, I don't think this is worth blocking on and wouldn't be
> opposed
> > if others felt strongly that a new replication policy class is superior
> to
> > a new property on the existing class.
> >
> > Cheers,
> >
> > Chris
> >
> > On Wed, Jul 19, 2023 at 2:53 PM Federico Valeri 
> > wrote:
> >
> > > Hi Chris, the KIP says it would be a subclass of
> DefaultReplicationPolicy
> > > that overrides the ReplicationPolicy.offsetSyncsTopic and
> > > ReplicationPolicy.checkpointsTopic. So, not much to maintain and it
> would
> > > be more intuitive, as you say.
> > >
> > > On Wed, Jul 19, 2023, 4:50 PM Chris Egerton 
> > > wrote:
> > >
> > > > HI all,
> > > >
> > > > I'm not sure I understand the benefits of introducing a separate
> > > > replication policy class, besides maybe being more readable/intuitive
> > to
> > > > users who would want to know when to use one or the other. It feels
> > like
> > > > we've swapped out a "fix the bug" property for an entire "fix the
> bug"
> > > > class, and it still leaves the problem of graceful migration from
> > legacy
> > > > behavior to new behavior unsolved. It would also require us to
> consider
> > > > whether any future changes we make to the DefaultReplicationPolicy
> > class
> > > > would have to be ported over to the LegacyReplicationPolicy class as
> > > well.
> > > >
> > > > Perhaps I'm missing something; are there other benefits of
> introducing
> > a
> > > > separate replication policy class?
> > > >
> > > > Cheers,
> > > >
> > > > Chris
> > > >
> > > > On Wed, Jul 19, 2023 at 5:45 AM Omnia Ibrahim <
> o.g.h.ibra...@gmail.com
> > >
> > > > wrote:
> > > >
> > > > > Hi Federico,
> > > > > I like the idea of implementing `LegacyReplicationPolicy` and
> > avoiding
> > > > bug
> > > > > fixes properties. We can drop the idea of the property and just go
> > > ahead
> > > > > with introducing the `LegacyReplicationPolicy` and any user upgrade
> > > from
> > > > > pre-KIP-690 can use this policy instead of default and no impact
> will
> > > > > happen to users upgrading from 3.x to post-KIP-949. We can mark
> > > > > `LegacyReplicationPolicy` as deprecated later if we want (but not
> in
> > > 4.0
> > > > as
> > > > > this is very soon) and we can drop it entirely at some point.
> > > > >
> > > > > If there's an agreement on this approach I can upgrade the KIP.
> > > > >
> > > > > Cheers.
> > > > > Omnia
> > > > >
> > > > > On Wed, Jul 19, 2023 at 8:52 AM Federico Valeri <
> > fedeval...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi, one way to avoid the "fix the bug property" would be to
> provide
> > > > > > and document an additional LegacyReplicationPolicy, but still
> > keeping
> > > > > > the current DefaultReplicationPolicy as replication.policy.class
> > > > > > default value, which is basically one of the workarounds
> suggested
> > in
> > > > > > the KIP.
> > > > > >
> > > > > > On Tue, Jul 18, 2023 at 9:49 PM Chris Egerton
> > >  > > > >
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi Federico/Omnia,
> > > > > > >
> > > > > > > Generally I like the idea of deprecating and eventually
> removing
> > > "fix
> > > > > the
> > > > > > > bug" properties like this, but 4.0 may be a bit soon. I'm also
> > > unsure
> > > > > of
> > > > > > > how we would instruct users who are relying on the property
> being
> > > set

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Omnia Ibrahim]

Hi Chris and Federico,
thinking about I think Chris's concern is valid and the bigger concern here
is that having this `LegacyReplicationPolicy` will eventually open the door
for diversion at some point between this `LegacyReplicationPolicy` and the
default one.
For now, let's have the flag properly fix this bug and we can keep it as an
option for people to switch between both behaviours. I know having a
bug-fix property is not great but we can treat it as a backward
compatibility property instead in order to keep old mirrors using the old
internal topics.

Hope this is reasonable for the time being.

Cheers,
Omnia

On Wed, Jul 19, 2023 at 11:16 PM Chris Egerton 
wrote:

> Hi Federico,
>
> Ah yes, sorry about that! You're correct that this would keep the two
> classes in line and largely eliminate the concern I posed about porting
> changes to both. Still, I'm a bit hesitant, and I'm not actually certain
> that this alternative is more intuitive. The name isn't very descriptive,
> and this is the kind of approach we can only really take once; if we break
> compatibility again, would we introduce a LegacyLegacyReplicationPolicy?
> LegacyReplicationPolicy2? Finally, it seems a bit strange to introduce a
> new class to implement a change in behavior this small.
>
> That said, I don't think this is worth blocking on and wouldn't be opposed
> if others felt strongly that a new replication policy class is superior to
> a new property on the existing class.
>
> Cheers,
>
> Chris
>
> On Wed, Jul 19, 2023 at 2:53 PM Federico Valeri 
> wrote:
>
> > Hi Chris, the KIP says it would be a subclass of DefaultReplicationPolicy
> > that overrides the ReplicationPolicy.offsetSyncsTopic and
> > ReplicationPolicy.checkpointsTopic. So, not much to maintain and it would
> > be more intuitive, as you say.
> >
> > On Wed, Jul 19, 2023, 4:50 PM Chris Egerton 
> > wrote:
> >
> > > HI all,
> > >
> > > I'm not sure I understand the benefits of introducing a separate
> > > replication policy class, besides maybe being more readable/intuitive
> to
> > > users who would want to know when to use one or the other. It feels
> like
> > > we've swapped out a "fix the bug" property for an entire "fix the bug"
> > > class, and it still leaves the problem of graceful migration from
> legacy
> > > behavior to new behavior unsolved. It would also require us to consider
> > > whether any future changes we make to the DefaultReplicationPolicy
> class
> > > would have to be ported over to the LegacyReplicationPolicy class as
> > well.
> > >
> > > Perhaps I'm missing something; are there other benefits of introducing
> a
> > > separate replication policy class?
> > >
> > > Cheers,
> > >
> > > Chris
> > >
> > > On Wed, Jul 19, 2023 at 5:45 AM Omnia Ibrahim  >
> > > wrote:
> > >
> > > > Hi Federico,
> > > > I like the idea of implementing `LegacyReplicationPolicy` and
> avoiding
> > > bug
> > > > fixes properties. We can drop the idea of the property and just go
> > ahead
> > > > with introducing the `LegacyReplicationPolicy` and any user upgrade
> > from
> > > > pre-KIP-690 can use this policy instead of default and no impact will
> > > > happen to users upgrading from 3.x to post-KIP-949. We can mark
> > > > `LegacyReplicationPolicy` as deprecated later if we want (but not in
> > 4.0
> > > as
> > > > this is very soon) and we can drop it entirely at some point.
> > > >
> > > > If there's an agreement on this approach I can upgrade the KIP.
> > > >
> > > > Cheers.
> > > > Omnia
> > > >
> > > > On Wed, Jul 19, 2023 at 8:52 AM Federico Valeri <
> fedeval...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi, one way to avoid the "fix the bug property" would be to provide
> > > > > and document an additional LegacyReplicationPolicy, but still
> keeping
> > > > > the current DefaultReplicationPolicy as replication.policy.class
> > > > > default value, which is basically one of the workarounds suggested
> in
> > > > > the KIP.
> > > > >
> > > > > On Tue, Jul 18, 2023 at 9:49 PM Chris Egerton
> >  > > >
> > > > > wrote:
> > > > > >
> > > > > > Hi Federico/Omnia,
> > > > > >
> > > > > > Generally I like the idea of deprecating and eventually removing
> > "fix
> > > > the
> > > > > > bug" properties like this, but 4.0 may be a bit soon. I'm also
> > unsure
> > > > of
> > > > > > how we would instruct users who are relying on the property being
> > set
> > > > to
> > > > > > "false" to migrate to a version of MM2 that doesn't support
> support
> > > it,
> > > > > > beyond simply implementing their own replication policy--at which
> > > > point,
> > > > > > would we really be doing anyone a favor by forcing them to fork
> the
> > > > > default
> > > > > > policy just to preserve a property we removed?
> > > > > >
> > > > > > I guess right now I'd rather play it safe and not immediately
> > > deprecate
> > > > > the
> > > > > > property. If we can find an easy migration path for users who are
> > > > relying
> > > > > > on it, then I'd be happy to deprecate and 

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Chris Egerton]

Hi Federico,

Ah yes, sorry about that! You're correct that this would keep the two
classes in line and largely eliminate the concern I posed about porting
changes to both. Still, I'm a bit hesitant, and I'm not actually certain
that this alternative is more intuitive. The name isn't very descriptive,
and this is the kind of approach we can only really take once; if we break
compatibility again, would we introduce a LegacyLegacyReplicationPolicy?
LegacyReplicationPolicy2? Finally, it seems a bit strange to introduce a
new class to implement a change in behavior this small.

That said, I don't think this is worth blocking on and wouldn't be opposed
if others felt strongly that a new replication policy class is superior to
a new property on the existing class.

Cheers,

Chris

On Wed, Jul 19, 2023 at 2:53 PM Federico Valeri 
wrote:

> Hi Chris, the KIP says it would be a subclass of DefaultReplicationPolicy
> that overrides the ReplicationPolicy.offsetSyncsTopic and
> ReplicationPolicy.checkpointsTopic. So, not much to maintain and it would
> be more intuitive, as you say.
>
> On Wed, Jul 19, 2023, 4:50 PM Chris Egerton 
> wrote:
>
> > HI all,
> >
> > I'm not sure I understand the benefits of introducing a separate
> > replication policy class, besides maybe being more readable/intuitive to
> > users who would want to know when to use one or the other. It feels like
> > we've swapped out a "fix the bug" property for an entire "fix the bug"
> > class, and it still leaves the problem of graceful migration from legacy
> > behavior to new behavior unsolved. It would also require us to consider
> > whether any future changes we make to the DefaultReplicationPolicy class
> > would have to be ported over to the LegacyReplicationPolicy class as
> well.
> >
> > Perhaps I'm missing something; are there other benefits of introducing a
> > separate replication policy class?
> >
> > Cheers,
> >
> > Chris
> >
> > On Wed, Jul 19, 2023 at 5:45 AM Omnia Ibrahim 
> > wrote:
> >
> > > Hi Federico,
> > > I like the idea of implementing `LegacyReplicationPolicy` and avoiding
> > bug
> > > fixes properties. We can drop the idea of the property and just go
> ahead
> > > with introducing the `LegacyReplicationPolicy` and any user upgrade
> from
> > > pre-KIP-690 can use this policy instead of default and no impact will
> > > happen to users upgrading from 3.x to post-KIP-949. We can mark
> > > `LegacyReplicationPolicy` as deprecated later if we want (but not in
> 4.0
> > as
> > > this is very soon) and we can drop it entirely at some point.
> > >
> > > If there's an agreement on this approach I can upgrade the KIP.
> > >
> > > Cheers.
> > > Omnia
> > >
> > > On Wed, Jul 19, 2023 at 8:52 AM Federico Valeri 
> > > wrote:
> > >
> > > > Hi, one way to avoid the "fix the bug property" would be to provide
> > > > and document an additional LegacyReplicationPolicy, but still keeping
> > > > the current DefaultReplicationPolicy as replication.policy.class
> > > > default value, which is basically one of the workarounds suggested in
> > > > the KIP.
> > > >
> > > > On Tue, Jul 18, 2023 at 9:49 PM Chris Egerton
>  > >
> > > > wrote:
> > > > >
> > > > > Hi Federico/Omnia,
> > > > >
> > > > > Generally I like the idea of deprecating and eventually removing
> "fix
> > > the
> > > > > bug" properties like this, but 4.0 may be a bit soon. I'm also
> unsure
> > > of
> > > > > how we would instruct users who are relying on the property being
> set
> > > to
> > > > > "false" to migrate to a version of MM2 that doesn't support support
> > it,
> > > > > beyond simply implementing their own replication policy--at which
> > > point,
> > > > > would we really be doing anyone a favor by forcing them to fork the
> > > > default
> > > > > policy just to preserve a property we removed?
> > > > >
> > > > > I guess right now I'd rather play it safe and not immediately
> > deprecate
> > > > the
> > > > > property. If we can find an easy migration path for users who are
> > > relying
> > > > > on it, then I'd be happy to deprecate and schedule for removal.
> > > > >
> > > > > Cheers,
> > > > >
> > > > > Chris
> > > > >
> > > > > On Tue, Jul 18, 2023 at 12:54 PM Omnia Ibrahim <
> > > o.g.h.ibra...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi Federico,
> > > > > > I don't have any strong opinion one way or the other. The pro of
> > > > > > deprecation is not adding more configs to MM2 as it has too many
> > > > configs
> > > > > > already. However, we need to think about old MM2 migrating their
> > > > internal
> > > > > > topics to 4.0 with less impact.
> > > > > >
> > > > > > @Chris what do you think?
> > > > > >
> > > > > > Cheers
> > > > > > Omnia
> > > > > >
> > > > > > On Fri, Jul 14, 2023 at 2:38 PM Federico Valeri <
> > > fedeval...@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hi Omnia and Chris, I agree with setting
> > > > > > > "replication.policy.internal.topic.separator.enabled=true" by
> > > > default,
> > > > > > > but I was 

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Federico Valeri]

Hi Chris, the KIP says it would be a subclass of DefaultReplicationPolicy
that overrides the ReplicationPolicy.offsetSyncsTopic and
ReplicationPolicy.checkpointsTopic. So, not much to maintain and it would
be more intuitive, as you say.

On Wed, Jul 19, 2023, 4:50 PM Chris Egerton  wrote:

> HI all,
>
> I'm not sure I understand the benefits of introducing a separate
> replication policy class, besides maybe being more readable/intuitive to
> users who would want to know when to use one or the other. It feels like
> we've swapped out a "fix the bug" property for an entire "fix the bug"
> class, and it still leaves the problem of graceful migration from legacy
> behavior to new behavior unsolved. It would also require us to consider
> whether any future changes we make to the DefaultReplicationPolicy class
> would have to be ported over to the LegacyReplicationPolicy class as well.
>
> Perhaps I'm missing something; are there other benefits of introducing a
> separate replication policy class?
>
> Cheers,
>
> Chris
>
> On Wed, Jul 19, 2023 at 5:45 AM Omnia Ibrahim 
> wrote:
>
> > Hi Federico,
> > I like the idea of implementing `LegacyReplicationPolicy` and avoiding
> bug
> > fixes properties. We can drop the idea of the property and just go ahead
> > with introducing the `LegacyReplicationPolicy` and any user upgrade from
> > pre-KIP-690 can use this policy instead of default and no impact will
> > happen to users upgrading from 3.x to post-KIP-949. We can mark
> > `LegacyReplicationPolicy` as deprecated later if we want (but not in 4.0
> as
> > this is very soon) and we can drop it entirely at some point.
> >
> > If there's an agreement on this approach I can upgrade the KIP.
> >
> > Cheers.
> > Omnia
> >
> > On Wed, Jul 19, 2023 at 8:52 AM Federico Valeri 
> > wrote:
> >
> > > Hi, one way to avoid the "fix the bug property" would be to provide
> > > and document an additional LegacyReplicationPolicy, but still keeping
> > > the current DefaultReplicationPolicy as replication.policy.class
> > > default value, which is basically one of the workarounds suggested in
> > > the KIP.
> > >
> > > On Tue, Jul 18, 2023 at 9:49 PM Chris Egerton  >
> > > wrote:
> > > >
> > > > Hi Federico/Omnia,
> > > >
> > > > Generally I like the idea of deprecating and eventually removing "fix
> > the
> > > > bug" properties like this, but 4.0 may be a bit soon. I'm also unsure
> > of
> > > > how we would instruct users who are relying on the property being set
> > to
> > > > "false" to migrate to a version of MM2 that doesn't support support
> it,
> > > > beyond simply implementing their own replication policy--at which
> > point,
> > > > would we really be doing anyone a favor by forcing them to fork the
> > > default
> > > > policy just to preserve a property we removed?
> > > >
> > > > I guess right now I'd rather play it safe and not immediately
> deprecate
> > > the
> > > > property. If we can find an easy migration path for users who are
> > relying
> > > > on it, then I'd be happy to deprecate and schedule for removal.
> > > >
> > > > Cheers,
> > > >
> > > > Chris
> > > >
> > > > On Tue, Jul 18, 2023 at 12:54 PM Omnia Ibrahim <
> > o.g.h.ibra...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi Federico,
> > > > > I don't have any strong opinion one way or the other. The pro of
> > > > > deprecation is not adding more configs to MM2 as it has too many
> > > configs
> > > > > already. However, we need to think about old MM2 migrating their
> > > internal
> > > > > topics to 4.0 with less impact.
> > > > >
> > > > > @Chris what do you think?
> > > > >
> > > > > Cheers
> > > > > Omnia
> > > > >
> > > > > On Fri, Jul 14, 2023 at 2:38 PM Federico Valeri <
> > fedeval...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hi Omnia and Chris, I agree with setting
> > > > > > "replication.policy.internal.topic.separator.enabled=true" by
> > > default,
> > > > > > but I was wondering if we should also deprecate and remove in
> Kafka
> > > 4.
> > > > > > If there is agreement in having the same behavior for internal
> and
> > > > > > non-internal topics, then it should be fine, and we won't need to
> > > keep
> > > > > > an additional configuration around. Wdyt?
> > > > > >
> > > > > > Cheers
> > > > > > Fede
> > > > > >
> > > > > > On Fri, Jul 14, 2023 at 1:51 PM Omnia Ibrahim <
> > > o.g.h.ibra...@gmail.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > 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
> >  > > >
> > > > > > wrote:
> > > > > > > >
> > > > > > > > Hi Omnia,
> > > > > > > >
> > > > > > > > Yes, I think we ought to state the 

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Chris Egerton]

HI all,

I'm not sure I understand the benefits of introducing a separate
replication policy class, besides maybe being more readable/intuitive to
users who would want to know when to use one or the other. It feels like
we've swapped out a "fix the bug" property for an entire "fix the bug"
class, and it still leaves the problem of graceful migration from legacy
behavior to new behavior unsolved. It would also require us to consider
whether any future changes we make to the DefaultReplicationPolicy class
would have to be ported over to the LegacyReplicationPolicy class as well.

Perhaps I'm missing something; are there other benefits of introducing a
separate replication policy class?

Cheers,

Chris

On Wed, Jul 19, 2023 at 5:45 AM Omnia Ibrahim 
wrote:

> Hi Federico,
> I like the idea of implementing `LegacyReplicationPolicy` and avoiding bug
> fixes properties. We can drop the idea of the property and just go ahead
> with introducing the `LegacyReplicationPolicy` and any user upgrade from
> pre-KIP-690 can use this policy instead of default and no impact will
> happen to users upgrading from 3.x to post-KIP-949. We can mark
> `LegacyReplicationPolicy` as deprecated later if we want (but not in 4.0 as
> this is very soon) and we can drop it entirely at some point.
>
> If there's an agreement on this approach I can upgrade the KIP.
>
> Cheers.
> Omnia
>
> On Wed, Jul 19, 2023 at 8:52 AM Federico Valeri 
> wrote:
>
> > Hi, one way to avoid the "fix the bug property" would be to provide
> > and document an additional LegacyReplicationPolicy, but still keeping
> > the current DefaultReplicationPolicy as replication.policy.class
> > default value, which is basically one of the workarounds suggested in
> > the KIP.
> >
> > On Tue, Jul 18, 2023 at 9:49 PM Chris Egerton 
> > wrote:
> > >
> > > Hi Federico/Omnia,
> > >
> > > Generally I like the idea of deprecating and eventually removing "fix
> the
> > > bug" properties like this, but 4.0 may be a bit soon. I'm also unsure
> of
> > > how we would instruct users who are relying on the property being set
> to
> > > "false" to migrate to a version of MM2 that doesn't support support it,
> > > beyond simply implementing their own replication policy--at which
> point,
> > > would we really be doing anyone a favor by forcing them to fork the
> > default
> > > policy just to preserve a property we removed?
> > >
> > > I guess right now I'd rather play it safe and not immediately deprecate
> > the
> > > property. If we can find an easy migration path for users who are
> relying
> > > on it, then I'd be happy to deprecate and schedule for removal.
> > >
> > > Cheers,
> > >
> > > Chris
> > >
> > > On Tue, Jul 18, 2023 at 12:54 PM Omnia Ibrahim <
> o.g.h.ibra...@gmail.com>
> > > wrote:
> > >
> > > > Hi Federico,
> > > > I don't have any strong opinion one way or the other. The pro of
> > > > deprecation is not adding more configs to MM2 as it has too many
> > configs
> > > > already. However, we need to think about old MM2 migrating their
> > internal
> > > > topics to 4.0 with less impact.
> > > >
> > > > @Chris what do you think?
> > > >
> > > > Cheers
> > > > Omnia
> > > >
> > > > On Fri, Jul 14, 2023 at 2:38 PM Federico Valeri <
> fedeval...@gmail.com>
> > > > wrote:
> > > >
> > > > > Hi Omnia and Chris, I agree with setting
> > > > > "replication.policy.internal.topic.separator.enabled=true" by
> > default,
> > > > > but I was wondering if we should also deprecate and remove in Kafka
> > 4.
> > > > > If there is agreement in having the same behavior for internal and
> > > > > non-internal topics, then it should be fine, and we won't need to
> > keep
> > > > > an additional configuration around. Wdyt?
> > > > >
> > > > > Cheers
> > > > > Fede
> > > > >
> > > > > On Fri, Jul 14, 2023 at 1:51 PM Omnia Ibrahim <
> > o.g.h.ibra...@gmail.com>
> > > > > wrote:
> > > > > >
> > > > > > 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
>  > >
> > > > > 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 

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Omnia Ibrahim]

Hi Federico,
I like the idea of implementing `LegacyReplicationPolicy` and avoiding bug
fixes properties. We can drop the idea of the property and just go ahead
with introducing the `LegacyReplicationPolicy` and any user upgrade from
pre-KIP-690 can use this policy instead of default and no impact will
happen to users upgrading from 3.x to post-KIP-949. We can mark
`LegacyReplicationPolicy` as deprecated later if we want (but not in 4.0 as
this is very soon) and we can drop it entirely at some point.

If there's an agreement on this approach I can upgrade the KIP.

Cheers.
Omnia

On Wed, Jul 19, 2023 at 8:52 AM Federico Valeri 
wrote:

> Hi, one way to avoid the "fix the bug property" would be to provide
> and document an additional LegacyReplicationPolicy, but still keeping
> the current DefaultReplicationPolicy as replication.policy.class
> default value, which is basically one of the workarounds suggested in
> the KIP.
>
> On Tue, Jul 18, 2023 at 9:49 PM Chris Egerton 
> wrote:
> >
> > Hi Federico/Omnia,
> >
> > Generally I like the idea of deprecating and eventually removing "fix the
> > bug" properties like this, but 4.0 may be a bit soon. I'm also unsure of
> > how we would instruct users who are relying on the property being set to
> > "false" to migrate to a version of MM2 that doesn't support support it,
> > beyond simply implementing their own replication policy--at which point,
> > would we really be doing anyone a favor by forcing them to fork the
> default
> > policy just to preserve a property we removed?
> >
> > I guess right now I'd rather play it safe and not immediately deprecate
> the
> > property. If we can find an easy migration path for users who are relying
> > on it, then I'd be happy to deprecate and schedule for removal.
> >
> > Cheers,
> >
> > Chris
> >
> > On Tue, Jul 18, 2023 at 12:54 PM Omnia Ibrahim 
> > wrote:
> >
> > > Hi Federico,
> > > I don't have any strong opinion one way or the other. The pro of
> > > deprecation is not adding more configs to MM2 as it has too many
> configs
> > > already. However, we need to think about old MM2 migrating their
> internal
> > > topics to 4.0 with less impact.
> > >
> > > @Chris what do you think?
> > >
> > > Cheers
> > > Omnia
> > >
> > > On Fri, Jul 14, 2023 at 2:38 PM Federico Valeri 
> > > wrote:
> > >
> > > > Hi Omnia and Chris, I agree with setting
> > > > "replication.policy.internal.topic.separator.enabled=true" by
> default,
> > > > but I was wondering if we should also deprecate and remove in Kafka
> 4.
> > > > If there is agreement in having the same behavior for internal and
> > > > non-internal topics, then it should be fine, and we won't need to
> keep
> > > > an additional configuration around. Wdyt?
> > > >
> > > > Cheers
> > > > Fede
> > > >
> > > > On Fri, Jul 14, 2023 at 1:51 PM Omnia Ibrahim <
> o.g.h.ibra...@gmail.com>
> > > > wrote:
> > > > >
> > > > > 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  >
> > > > 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
> > >  > > > >
> > > > > >> 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 

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Federico Valeri]

Hi, one way to avoid the "fix the bug property" would be to provide
and document an additional LegacyReplicationPolicy, but still keeping
the current DefaultReplicationPolicy as replication.policy.class
default value, which is basically one of the workarounds suggested in
the KIP.

On Tue, Jul 18, 2023 at 9:49 PM Chris Egerton  wrote:
>
> Hi Federico/Omnia,
>
> Generally I like the idea of deprecating and eventually removing "fix the
> bug" properties like this, but 4.0 may be a bit soon. I'm also unsure of
> how we would instruct users who are relying on the property being set to
> "false" to migrate to a version of MM2 that doesn't support support it,
> beyond simply implementing their own replication policy--at which point,
> would we really be doing anyone a favor by forcing them to fork the default
> policy just to preserve a property we removed?
>
> I guess right now I'd rather play it safe and not immediately deprecate the
> property. If we can find an easy migration path for users who are relying
> on it, then I'd be happy to deprecate and schedule for removal.
>
> Cheers,
>
> Chris
>
> On Tue, Jul 18, 2023 at 12:54 PM Omnia Ibrahim 
> wrote:
>
> > Hi Federico,
> > I don't have any strong opinion one way or the other. The pro of
> > deprecation is not adding more configs to MM2 as it has too many configs
> > already. However, we need to think about old MM2 migrating their internal
> > topics to 4.0 with less impact.
> >
> > @Chris what do you think?
> >
> > Cheers
> > Omnia
> >
> > On Fri, Jul 14, 2023 at 2:38 PM Federico Valeri 
> > wrote:
> >
> > > Hi Omnia and Chris, I agree with setting
> > > "replication.policy.internal.topic.separator.enabled=true" by default,
> > > but I was wondering if we should also deprecate and remove in Kafka 4.
> > > If there is agreement in having the same behavior for internal and
> > > non-internal topics, then it should be fine, and we won't need to keep
> > > an additional configuration around. Wdyt?
> > >
> > > Cheers
> > > Fede
> > >
> > > On Fri, Jul 14, 2023 at 1:51 PM Omnia Ibrahim 
> > > wrote:
> > > >
> > > > 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 
> > > 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
> >  > > >
> > > > >> 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
> > > 

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Chris Egerton]

Hi Federico/Omnia,

Generally I like the idea of deprecating and eventually removing "fix the
bug" properties like this, but 4.0 may be a bit soon. I'm also unsure of
how we would instruct users who are relying on the property being set to
"false" to migrate to a version of MM2 that doesn't support support it,
beyond simply implementing their own replication policy--at which point,
would we really be doing anyone a favor by forcing them to fork the default
policy just to preserve a property we removed?

I guess right now I'd rather play it safe and not immediately deprecate the
property. If we can find an easy migration path for users who are relying
on it, then I'd be happy to deprecate and schedule for removal.

Cheers,

Chris

On Tue, Jul 18, 2023 at 12:54 PM Omnia Ibrahim 
wrote:

> Hi Federico,
> I don't have any strong opinion one way or the other. The pro of
> deprecation is not adding more configs to MM2 as it has too many configs
> already. However, we need to think about old MM2 migrating their internal
> topics to 4.0 with less impact.
>
> @Chris what do you think?
>
> Cheers
> Omnia
>
> On Fri, Jul 14, 2023 at 2:38 PM Federico Valeri 
> wrote:
>
> > Hi Omnia and Chris, I agree with setting
> > "replication.policy.internal.topic.separator.enabled=true" by default,
> > but I was wondering if we should also deprecate and remove in Kafka 4.
> > If there is agreement in having the same behavior for internal and
> > non-internal topics, then it should be fine, and we won't need to keep
> > an additional configuration around. Wdyt?
> >
> > Cheers
> > Fede
> >
> > On Fri, Jul 14, 2023 at 1:51 PM Omnia Ibrahim 
> > wrote:
> > >
> > > 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 
> > 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
>  > >
> > > >> 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
> > >  

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Omnia Ibrahim]

Hi Federico,
I don't have any strong opinion one way or the other. The pro of
deprecation is not adding more configs to MM2 as it has too many configs
already. However, we need to think about old MM2 migrating their internal
topics to 4.0 with less impact.

@Chris what do you think?

Cheers
Omnia

On Fri, Jul 14, 2023 at 2:38 PM Federico Valeri 
wrote:

> Hi Omnia and Chris, I agree with setting
> "replication.policy.internal.topic.separator.enabled=true" by default,
> but I was wondering if we should also deprecate and remove in Kafka 4.
> If there is agreement in having the same behavior for internal and
> non-internal topics, then it should be fine, and we won't need to keep
> an additional configuration around. Wdyt?
>
> Cheers
> Fede
>
> On Fri, Jul 14, 2023 at 1:51 PM Omnia Ibrahim 
> wrote:
> >
> > 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 
> 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  >
> > > 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  >
> > >> 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
>  > >>>
> >  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
> > 

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Federico Valeri]

Hi Omnia and Chris, I agree with setting
"replication.policy.internal.topic.separator.enabled=true" by default,
but I was wondering if we should also deprecate and remove in Kafka 4.
If there is agreement in having the same behavior for internal and
non-internal topics, then it should be fine, and we won't need to keep
an additional configuration around. Wdyt?

Cheers
Fede

On Fri, Jul 14, 2023 at 1:51 PM Omnia Ibrahim  wrote:
>
> 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  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 
> > 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 
> >> 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 
> >>> 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  >>>
>  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 

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Omnia Ibrahim]

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

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Chris Egerton]

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

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Omnia Ibrahim]

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

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Chris Egerton]

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

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Omnia Ibrahim]

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

Re: [DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Chris Egerton]

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

[DISCUSS] KIP-949: Add flag to enable the usage of topic separator in MM2 DefaultReplicationPolicy

[Omnia Ibrahim]

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


Thanks for your time and feedback.
Omnia