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

2023-08-02 Thread Omnia Ibrahim
Thanks, Mickael.
Am closing the voting now with 3 binding votes (from Chris, Greg and
Mickale) and 1 non-binding (from Federico).

On Wed, Aug 2, 2023 at 2:52 PM Mickael Maison 
wrote:

> +1 (binding)
>
> Thanks for the KIP
>
> On Tue, Aug 1, 2023 at 1:26 PM Omnia Ibrahim 
> wrote:
> >
> > Thanks for the binding vote, Greg, We now need one extra binding vote to
> > get this KIP accepted.
> >
> > On Tue, Jul 25, 2023 at 8:10 PM Greg Harris  >
> > wrote:
> >
> > > Hey Omnia,
> > >
> > > Thanks for the KIP!
> > >
> > > I think that MM2 is responsible for providing an upgrade path for
> > > users, even if it isn't backwards-compatible by default due to a
> > > mistake.
> > > The non-configuration-based strategies I could think of aren't viable
> > > due to the danger of inferring the incorrect topic name, and inherent
> > > complexity which makes them hard to backport.
> > > I also support the decision to backport this to 3.1 - 3.5, so that MM2
> > > users can upgrade in minor version increments after those patch
> > > releases go out.
> > >
> > > I'm +1 (binding).
> > >
> > > Thanks,
> > > Greg
> > >
> > > On Mon, Jul 24, 2023 at 7:21 AM Omnia Ibrahim  >
> > > wrote:
> > > >
> > > > Hi Chris, I updated the KIP to address your feedback. Thanks for the
> > > vote.
> > > >
> > > > On Mon, Jul 24, 2023 at 1:30 PM Chris Egerton
> 
> > > > wrote:
> > > >
> > > > > Hi Omnia,
> > > > >
> > > > > I think there's a few clarifications that should still be made on
> the
> > > KIP,
> > > > > but assuming these are agreeable, I'm +1 (binding)
> > > > >
> > > > > - In the description for the
> > > > > replication.policy.internal.topic.separator.enabled property (in
> the
> > > > > "Public Interfaces" section), we should specify that it affects
> only
> > > the
> > > > > checkpoints and offset syncs topic
> > > > > - We can remove the code snippet from the "Proposed Changes"
> section
> > > (right
> > > > > now it's a little buggy; there's two different implementations for
> the
> > > same
> > > > > "internalSuffix" method, and there are references to an
> > > "internalSeparator"
> > > > > method but no implementation for it); since we don't usually
> require
> > > > > specific code changes in KIPs, I think as long as we can describe
> the
> > > > > changes we're proposing in the "Public Interfaces" section, that
> > > should be
> > > > > enough for this KIP
> > > > >
> > > > > Cheers,
> > > > >
> > > > > Chris
> > > > >
> > > > > On Mon, Jul 24, 2023 at 2:04 AM Federico Valeri <
> fedeval...@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > +1 (non binding)
> > > > > >
> > > > > > Thanks
> > > > > > Fede
> > > > > >
> > > > > >
> > > > > > On Sun, Jul 23, 2023 at 6:30 PM Omnia Ibrahim <
> > > o.g.h.ibra...@gmail.com>
> > > > > > wrote:
> > > > > > >
> > > > > > > Hi everyone,
> > > > > > > I would like to open a vote for 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
> > > > > > > Omnia
> > > > > >
> > > > >
> > >
>


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

2023-08-02 Thread Mickael Maison
+1 (binding)

Thanks for the KIP

On Tue, Aug 1, 2023 at 1:26 PM Omnia Ibrahim  wrote:
>
> Thanks for the binding vote, Greg, We now need one extra binding vote to
> get this KIP accepted.
>
> On Tue, Jul 25, 2023 at 8:10 PM Greg Harris 
> wrote:
>
> > Hey Omnia,
> >
> > Thanks for the KIP!
> >
> > I think that MM2 is responsible for providing an upgrade path for
> > users, even if it isn't backwards-compatible by default due to a
> > mistake.
> > The non-configuration-based strategies I could think of aren't viable
> > due to the danger of inferring the incorrect topic name, and inherent
> > complexity which makes them hard to backport.
> > I also support the decision to backport this to 3.1 - 3.5, so that MM2
> > users can upgrade in minor version increments after those patch
> > releases go out.
> >
> > I'm +1 (binding).
> >
> > Thanks,
> > Greg
> >
> > On Mon, Jul 24, 2023 at 7:21 AM Omnia Ibrahim 
> > wrote:
> > >
> > > Hi Chris, I updated the KIP to address your feedback. Thanks for the
> > vote.
> > >
> > > On Mon, Jul 24, 2023 at 1:30 PM Chris Egerton 
> > > wrote:
> > >
> > > > Hi Omnia,
> > > >
> > > > I think there's a few clarifications that should still be made on the
> > KIP,
> > > > but assuming these are agreeable, I'm +1 (binding)
> > > >
> > > > - In the description for the
> > > > replication.policy.internal.topic.separator.enabled property (in the
> > > > "Public Interfaces" section), we should specify that it affects only
> > the
> > > > checkpoints and offset syncs topic
> > > > - We can remove the code snippet from the "Proposed Changes" section
> > (right
> > > > now it's a little buggy; there's two different implementations for the
> > same
> > > > "internalSuffix" method, and there are references to an
> > "internalSeparator"
> > > > method but no implementation for it); since we don't usually require
> > > > specific code changes in KIPs, I think as long as we can describe the
> > > > changes we're proposing in the "Public Interfaces" section, that
> > should be
> > > > enough for this KIP
> > > >
> > > > Cheers,
> > > >
> > > > Chris
> > > >
> > > > On Mon, Jul 24, 2023 at 2:04 AM Federico Valeri 
> > > > wrote:
> > > >
> > > > > +1 (non binding)
> > > > >
> > > > > Thanks
> > > > > Fede
> > > > >
> > > > >
> > > > > On Sun, Jul 23, 2023 at 6:30 PM Omnia Ibrahim <
> > o.g.h.ibra...@gmail.com>
> > > > > wrote:
> > > > > >
> > > > > > Hi everyone,
> > > > > > I would like to open a vote for 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
> > > > > > Omnia
> > > > >
> > > >
> >


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

2023-08-01 Thread Omnia Ibrahim
Thanks for the binding vote, Greg, We now need one extra binding vote to
get this KIP accepted.

On Tue, Jul 25, 2023 at 8:10 PM Greg Harris 
wrote:

> Hey Omnia,
>
> Thanks for the KIP!
>
> I think that MM2 is responsible for providing an upgrade path for
> users, even if it isn't backwards-compatible by default due to a
> mistake.
> The non-configuration-based strategies I could think of aren't viable
> due to the danger of inferring the incorrect topic name, and inherent
> complexity which makes them hard to backport.
> I also support the decision to backport this to 3.1 - 3.5, so that MM2
> users can upgrade in minor version increments after those patch
> releases go out.
>
> I'm +1 (binding).
>
> Thanks,
> Greg
>
> On Mon, Jul 24, 2023 at 7:21 AM Omnia Ibrahim 
> wrote:
> >
> > Hi Chris, I updated the KIP to address your feedback. Thanks for the
> vote.
> >
> > On Mon, Jul 24, 2023 at 1:30 PM Chris Egerton 
> > wrote:
> >
> > > Hi Omnia,
> > >
> > > I think there's a few clarifications that should still be made on the
> KIP,
> > > but assuming these are agreeable, I'm +1 (binding)
> > >
> > > - In the description for the
> > > replication.policy.internal.topic.separator.enabled property (in the
> > > "Public Interfaces" section), we should specify that it affects only
> the
> > > checkpoints and offset syncs topic
> > > - We can remove the code snippet from the "Proposed Changes" section
> (right
> > > now it's a little buggy; there's two different implementations for the
> same
> > > "internalSuffix" method, and there are references to an
> "internalSeparator"
> > > method but no implementation for it); since we don't usually require
> > > specific code changes in KIPs, I think as long as we can describe the
> > > changes we're proposing in the "Public Interfaces" section, that
> should be
> > > enough for this KIP
> > >
> > > Cheers,
> > >
> > > Chris
> > >
> > > On Mon, Jul 24, 2023 at 2:04 AM Federico Valeri 
> > > wrote:
> > >
> > > > +1 (non binding)
> > > >
> > > > Thanks
> > > > Fede
> > > >
> > > >
> > > > On Sun, Jul 23, 2023 at 6:30 PM Omnia Ibrahim <
> o.g.h.ibra...@gmail.com>
> > > > wrote:
> > > > >
> > > > > Hi everyone,
> > > > > I would like to open a vote for 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
> > > > > Omnia
> > > >
> > >
>


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

2023-07-25 Thread Greg Harris
Hey Omnia,

Thanks for the KIP!

I think that MM2 is responsible for providing an upgrade path for
users, even if it isn't backwards-compatible by default due to a
mistake.
The non-configuration-based strategies I could think of aren't viable
due to the danger of inferring the incorrect topic name, and inherent
complexity which makes them hard to backport.
I also support the decision to backport this to 3.1 - 3.5, so that MM2
users can upgrade in minor version increments after those patch
releases go out.

I'm +1 (binding).

Thanks,
Greg

On Mon, Jul 24, 2023 at 7:21 AM Omnia Ibrahim  wrote:
>
> Hi Chris, I updated the KIP to address your feedback. Thanks for the vote.
>
> On Mon, Jul 24, 2023 at 1:30 PM Chris Egerton 
> wrote:
>
> > Hi Omnia,
> >
> > I think there's a few clarifications that should still be made on the KIP,
> > but assuming these are agreeable, I'm +1 (binding)
> >
> > - In the description for the
> > replication.policy.internal.topic.separator.enabled property (in the
> > "Public Interfaces" section), we should specify that it affects only the
> > checkpoints and offset syncs topic
> > - We can remove the code snippet from the "Proposed Changes" section (right
> > now it's a little buggy; there's two different implementations for the same
> > "internalSuffix" method, and there are references to an "internalSeparator"
> > method but no implementation for it); since we don't usually require
> > specific code changes in KIPs, I think as long as we can describe the
> > changes we're proposing in the "Public Interfaces" section, that should be
> > enough for this KIP
> >
> > Cheers,
> >
> > Chris
> >
> > On Mon, Jul 24, 2023 at 2:04 AM Federico Valeri 
> > wrote:
> >
> > > +1 (non binding)
> > >
> > > Thanks
> > > Fede
> > >
> > >
> > > On Sun, Jul 23, 2023 at 6:30 PM Omnia Ibrahim 
> > > wrote:
> > > >
> > > > Hi everyone,
> > > > I would like to open a vote for 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
> > > > Omnia
> > >
> >


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

2023-07-24 Thread Omnia Ibrahim
Hi Chris, I updated the KIP to address your feedback. Thanks for the vote.

On Mon, Jul 24, 2023 at 1:30 PM Chris Egerton 
wrote:

> Hi Omnia,
>
> I think there's a few clarifications that should still be made on the KIP,
> but assuming these are agreeable, I'm +1 (binding)
>
> - In the description for the
> replication.policy.internal.topic.separator.enabled property (in the
> "Public Interfaces" section), we should specify that it affects only the
> checkpoints and offset syncs topic
> - We can remove the code snippet from the "Proposed Changes" section (right
> now it's a little buggy; there's two different implementations for the same
> "internalSuffix" method, and there are references to an "internalSeparator"
> method but no implementation for it); since we don't usually require
> specific code changes in KIPs, I think as long as we can describe the
> changes we're proposing in the "Public Interfaces" section, that should be
> enough for this KIP
>
> Cheers,
>
> Chris
>
> On Mon, Jul 24, 2023 at 2:04 AM Federico Valeri 
> wrote:
>
> > +1 (non binding)
> >
> > Thanks
> > Fede
> >
> >
> > On Sun, Jul 23, 2023 at 6:30 PM Omnia Ibrahim 
> > wrote:
> > >
> > > Hi everyone,
> > > I would like to open a vote for 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
> > > Omnia
> >
>


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

2023-07-24 Thread Chris Egerton
Hi Omnia,

I think there's a few clarifications that should still be made on the KIP,
but assuming these are agreeable, I'm +1 (binding)

- In the description for the
replication.policy.internal.topic.separator.enabled property (in the
"Public Interfaces" section), we should specify that it affects only the
checkpoints and offset syncs topic
- We can remove the code snippet from the "Proposed Changes" section (right
now it's a little buggy; there's two different implementations for the same
"internalSuffix" method, and there are references to an "internalSeparator"
method but no implementation for it); since we don't usually require
specific code changes in KIPs, I think as long as we can describe the
changes we're proposing in the "Public Interfaces" section, that should be
enough for this KIP

Cheers,

Chris

On Mon, Jul 24, 2023 at 2:04 AM Federico Valeri 
wrote:

> +1 (non binding)
>
> Thanks
> Fede
>
>
> On Sun, Jul 23, 2023 at 6:30 PM Omnia Ibrahim 
> wrote:
> >
> > Hi everyone,
> > I would like to open a vote for 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
> > Omnia
>


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

2023-07-24 Thread Federico Valeri
+1 (non binding)

Thanks
Fede


On Sun, Jul 23, 2023 at 6:30 PM Omnia Ibrahim  wrote:
>
> Hi everyone,
> I would like to open a vote for 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
> Omnia