You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Jason Gustafson <ja...@confluent.io> on 2019/08/07 18:37:26 UTC
[VOTE] KIP-497: Add inter-broker API to alter ISR
Hi All,
I'd like to start a vote on KIP-497:
https://cwiki.apache.org/confluence/display/KAFKA/KIP-497%3A+Add+inter-broker+API+to+alter+ISR.
+1
from me.
-Jason
Re: [VOTE] KIP-497: Add inter-broker API to alter ISR
Posted by Jason Gustafson <ja...@confluent.io>.
Thanks Jun. Yes, that is probably the simplest way to go.
-Jason
On Tue, Aug 20, 2019 at 2:14 PM Jun Rao <ju...@confluent.io> wrote:
> Hi, Jason,
>
> Thanks for the KIP. Sorry for the late reply. +1 from me too.
>
> Just a comment on the implementation. Perhaps we should only allow one
> pending AlterIsrRequest from a broker to the controller (at least for a
> given partition) in order to set CurrentZkVersion properly?
>
> Jun
>
> On Tue, Aug 20, 2019 at 10:07 AM Jason Gustafson <ja...@confluent.io>
> wrote:
>
> > I'm going to close this vote. The final result is +5 with 3 binding
> votes.
> >
> > -Jason
> >
> > On Wed, Aug 14, 2019 at 10:05 AM Satish Duggana <
> satish.duggana@gmail.com>
> > wrote:
> >
> > > +1 (non-binding), Thanks Jason for nice improvements on ISR
> > > propagation protocol!
> > >
> > > On Wed, Aug 14, 2019 at 8:29 PM David Arthur <mu...@gmail.com> wrote:
> > > >
> > > > +1 binding, this looks great!
> > > >
> > > > -David
> > > >
> > > > On Tue, Aug 13, 2019 at 4:55 PM Guozhang Wang <wa...@gmail.com>
> > > wrote:
> > > >
> > > > > +1 (binding). This is a great KIP, thanks Jason!
> > > > >
> > > > > Regarding the naming of the zkVersion, I'm actually fine to name it
> > > more
> > > > > generally and leave a note that at the moment its value is defined
> as
> > > the
> > > > > zk version.
> > > > >
> > > > >
> > > > > Guozhang
> > > > >
> > > > >
> > > > > On Mon, Aug 12, 2019 at 2:22 PM Jason Gustafson <
> jason@confluent.io>
> > > > > wrote:
> > > > >
> > > > > > Hi Viktor,
> > > > > >
> > > > > > I originally named the field `CurrentVersion`. I didn't have 'Zk'
> > in
> > > the
> > > > > > name in anticipation of KIP-500. I thought about it and decided
> it
> > > makes
> > > > > > sense to keep naming consistent with other APIs. Even if KIP-500
> > > passes,
> > > > > > there will be some time during which it only refers to the zk
> > > version.
> > > > > > Eventually we'll have to decide whether it makes sense to change
> > the
> > > name
> > > > > > or just introduce a new field.
> > > > > >
> > > > > > Thanks,
> > > > > > Jason
> > > > > >
> > > > > > On Fri, Aug 9, 2019 at 9:19 AM Viktor Somogyi-Vass <
> > > > > > viktorsomogyi@gmail.com>
> > > > > > wrote:
> > > > > >
> > > > > > > Hey Jason,
> > > > > > >
> > > > > > > +1 from me too.
> > > > > > > One note though: since it's a new protocol we could perhaps
> > rename
> > > > > > > CurrentZkVersion to something like "IsrEpoch" or "IsrVersion".
> I
> > > think
> > > > > > > that'd reflect its purpose better.
> > > > > > >
> > > > > > > Best,
> > > > > > > Viktor
> > > > > > >
> > > > > > > On Wed, Aug 7, 2019 at 8:37 PM Jason Gustafson <
> > jason@confluent.io
> > > >
> > > > > > wrote:
> > > > > > >
> > > > > > > > Hi All,
> > > > > > > >
> > > > > > > > I'd like to start a vote on KIP-497:
> > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-497%3A+Add+inter-broker+API+to+alter+ISR
> > > > > > > > .
> > > > > > > > +1
> > > > > > > > from me.
> > > > > > > >
> > > > > > > > -Jason
> > > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > > >
> > > > > --
> > > > > -- Guozhang
> > > > >
> > > >
> > > >
> > > > --
> > > > David Arthur
> > >
> >
>
Re: [VOTE] KIP-497: Add inter-broker API to alter ISR
Posted by Jun Rao <ju...@confluent.io>.
Hi, Jason,
Thanks for the KIP. Sorry for the late reply. +1 from me too.
Just a comment on the implementation. Perhaps we should only allow one
pending AlterIsrRequest from a broker to the controller (at least for a
given partition) in order to set CurrentZkVersion properly?
Jun
On Tue, Aug 20, 2019 at 10:07 AM Jason Gustafson <ja...@confluent.io> wrote:
> I'm going to close this vote. The final result is +5 with 3 binding votes.
>
> -Jason
>
> On Wed, Aug 14, 2019 at 10:05 AM Satish Duggana <sa...@gmail.com>
> wrote:
>
> > +1 (non-binding), Thanks Jason for nice improvements on ISR
> > propagation protocol!
> >
> > On Wed, Aug 14, 2019 at 8:29 PM David Arthur <mu...@gmail.com> wrote:
> > >
> > > +1 binding, this looks great!
> > >
> > > -David
> > >
> > > On Tue, Aug 13, 2019 at 4:55 PM Guozhang Wang <wa...@gmail.com>
> > wrote:
> > >
> > > > +1 (binding). This is a great KIP, thanks Jason!
> > > >
> > > > Regarding the naming of the zkVersion, I'm actually fine to name it
> > more
> > > > generally and leave a note that at the moment its value is defined as
> > the
> > > > zk version.
> > > >
> > > >
> > > > Guozhang
> > > >
> > > >
> > > > On Mon, Aug 12, 2019 at 2:22 PM Jason Gustafson <ja...@confluent.io>
> > > > wrote:
> > > >
> > > > > Hi Viktor,
> > > > >
> > > > > I originally named the field `CurrentVersion`. I didn't have 'Zk'
> in
> > the
> > > > > name in anticipation of KIP-500. I thought about it and decided it
> > makes
> > > > > sense to keep naming consistent with other APIs. Even if KIP-500
> > passes,
> > > > > there will be some time during which it only refers to the zk
> > version.
> > > > > Eventually we'll have to decide whether it makes sense to change
> the
> > name
> > > > > or just introduce a new field.
> > > > >
> > > > > Thanks,
> > > > > Jason
> > > > >
> > > > > On Fri, Aug 9, 2019 at 9:19 AM Viktor Somogyi-Vass <
> > > > > viktorsomogyi@gmail.com>
> > > > > wrote:
> > > > >
> > > > > > Hey Jason,
> > > > > >
> > > > > > +1 from me too.
> > > > > > One note though: since it's a new protocol we could perhaps
> rename
> > > > > > CurrentZkVersion to something like "IsrEpoch" or "IsrVersion". I
> > think
> > > > > > that'd reflect its purpose better.
> > > > > >
> > > > > > Best,
> > > > > > Viktor
> > > > > >
> > > > > > On Wed, Aug 7, 2019 at 8:37 PM Jason Gustafson <
> jason@confluent.io
> > >
> > > > > wrote:
> > > > > >
> > > > > > > Hi All,
> > > > > > >
> > > > > > > I'd like to start a vote on KIP-497:
> > > > > > >
> > > > > > >
> > > > > >
> > > > >
> > > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-497%3A+Add+inter-broker+API+to+alter+ISR
> > > > > > > .
> > > > > > > +1
> > > > > > > from me.
> > > > > > >
> > > > > > > -Jason
> > > > > > >
> > > > > >
> > > > >
> > > >
> > > >
> > > > --
> > > > -- Guozhang
> > > >
> > >
> > >
> > > --
> > > David Arthur
> >
>
Re: [VOTE] KIP-497: Add inter-broker API to alter ISR
Posted by Jason Gustafson <ja...@confluent.io>.
I'm going to close this vote. The final result is +5 with 3 binding votes.
-Jason
On Wed, Aug 14, 2019 at 10:05 AM Satish Duggana <sa...@gmail.com>
wrote:
> +1 (non-binding), Thanks Jason for nice improvements on ISR
> propagation protocol!
>
> On Wed, Aug 14, 2019 at 8:29 PM David Arthur <mu...@gmail.com> wrote:
> >
> > +1 binding, this looks great!
> >
> > -David
> >
> > On Tue, Aug 13, 2019 at 4:55 PM Guozhang Wang <wa...@gmail.com>
> wrote:
> >
> > > +1 (binding). This is a great KIP, thanks Jason!
> > >
> > > Regarding the naming of the zkVersion, I'm actually fine to name it
> more
> > > generally and leave a note that at the moment its value is defined as
> the
> > > zk version.
> > >
> > >
> > > Guozhang
> > >
> > >
> > > On Mon, Aug 12, 2019 at 2:22 PM Jason Gustafson <ja...@confluent.io>
> > > wrote:
> > >
> > > > Hi Viktor,
> > > >
> > > > I originally named the field `CurrentVersion`. I didn't have 'Zk' in
> the
> > > > name in anticipation of KIP-500. I thought about it and decided it
> makes
> > > > sense to keep naming consistent with other APIs. Even if KIP-500
> passes,
> > > > there will be some time during which it only refers to the zk
> version.
> > > > Eventually we'll have to decide whether it makes sense to change the
> name
> > > > or just introduce a new field.
> > > >
> > > > Thanks,
> > > > Jason
> > > >
> > > > On Fri, Aug 9, 2019 at 9:19 AM Viktor Somogyi-Vass <
> > > > viktorsomogyi@gmail.com>
> > > > wrote:
> > > >
> > > > > Hey Jason,
> > > > >
> > > > > +1 from me too.
> > > > > One note though: since it's a new protocol we could perhaps rename
> > > > > CurrentZkVersion to something like "IsrEpoch" or "IsrVersion". I
> think
> > > > > that'd reflect its purpose better.
> > > > >
> > > > > Best,
> > > > > Viktor
> > > > >
> > > > > On Wed, Aug 7, 2019 at 8:37 PM Jason Gustafson <jason@confluent.io
> >
> > > > wrote:
> > > > >
> > > > > > Hi All,
> > > > > >
> > > > > > I'd like to start a vote on KIP-497:
> > > > > >
> > > > > >
> > > > >
> > > >
> > >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-497%3A+Add+inter-broker+API+to+alter+ISR
> > > > > > .
> > > > > > +1
> > > > > > from me.
> > > > > >
> > > > > > -Jason
> > > > > >
> > > > >
> > > >
> > >
> > >
> > > --
> > > -- Guozhang
> > >
> >
> >
> > --
> > David Arthur
>
Re: [VOTE] KIP-497: Add inter-broker API to alter ISR
Posted by Satish Duggana <sa...@gmail.com>.
+1 (non-binding), Thanks Jason for nice improvements on ISR
propagation protocol!
On Wed, Aug 14, 2019 at 8:29 PM David Arthur <mu...@gmail.com> wrote:
>
> +1 binding, this looks great!
>
> -David
>
> On Tue, Aug 13, 2019 at 4:55 PM Guozhang Wang <wa...@gmail.com> wrote:
>
> > +1 (binding). This is a great KIP, thanks Jason!
> >
> > Regarding the naming of the zkVersion, I'm actually fine to name it more
> > generally and leave a note that at the moment its value is defined as the
> > zk version.
> >
> >
> > Guozhang
> >
> >
> > On Mon, Aug 12, 2019 at 2:22 PM Jason Gustafson <ja...@confluent.io>
> > wrote:
> >
> > > Hi Viktor,
> > >
> > > I originally named the field `CurrentVersion`. I didn't have 'Zk' in the
> > > name in anticipation of KIP-500. I thought about it and decided it makes
> > > sense to keep naming consistent with other APIs. Even if KIP-500 passes,
> > > there will be some time during which it only refers to the zk version.
> > > Eventually we'll have to decide whether it makes sense to change the name
> > > or just introduce a new field.
> > >
> > > Thanks,
> > > Jason
> > >
> > > On Fri, Aug 9, 2019 at 9:19 AM Viktor Somogyi-Vass <
> > > viktorsomogyi@gmail.com>
> > > wrote:
> > >
> > > > Hey Jason,
> > > >
> > > > +1 from me too.
> > > > One note though: since it's a new protocol we could perhaps rename
> > > > CurrentZkVersion to something like "IsrEpoch" or "IsrVersion". I think
> > > > that'd reflect its purpose better.
> > > >
> > > > Best,
> > > > Viktor
> > > >
> > > > On Wed, Aug 7, 2019 at 8:37 PM Jason Gustafson <ja...@confluent.io>
> > > wrote:
> > > >
> > > > > Hi All,
> > > > >
> > > > > I'd like to start a vote on KIP-497:
> > > > >
> > > > >
> > > >
> > >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-497%3A+Add+inter-broker+API+to+alter+ISR
> > > > > .
> > > > > +1
> > > > > from me.
> > > > >
> > > > > -Jason
> > > > >
> > > >
> > >
> >
> >
> > --
> > -- Guozhang
> >
>
>
> --
> David Arthur
Re: [VOTE] KIP-497: Add inter-broker API to alter ISR
Posted by David Arthur <mu...@gmail.com>.
+1 binding, this looks great!
-David
On Tue, Aug 13, 2019 at 4:55 PM Guozhang Wang <wa...@gmail.com> wrote:
> +1 (binding). This is a great KIP, thanks Jason!
>
> Regarding the naming of the zkVersion, I'm actually fine to name it more
> generally and leave a note that at the moment its value is defined as the
> zk version.
>
>
> Guozhang
>
>
> On Mon, Aug 12, 2019 at 2:22 PM Jason Gustafson <ja...@confluent.io>
> wrote:
>
> > Hi Viktor,
> >
> > I originally named the field `CurrentVersion`. I didn't have 'Zk' in the
> > name in anticipation of KIP-500. I thought about it and decided it makes
> > sense to keep naming consistent with other APIs. Even if KIP-500 passes,
> > there will be some time during which it only refers to the zk version.
> > Eventually we'll have to decide whether it makes sense to change the name
> > or just introduce a new field.
> >
> > Thanks,
> > Jason
> >
> > On Fri, Aug 9, 2019 at 9:19 AM Viktor Somogyi-Vass <
> > viktorsomogyi@gmail.com>
> > wrote:
> >
> > > Hey Jason,
> > >
> > > +1 from me too.
> > > One note though: since it's a new protocol we could perhaps rename
> > > CurrentZkVersion to something like "IsrEpoch" or "IsrVersion". I think
> > > that'd reflect its purpose better.
> > >
> > > Best,
> > > Viktor
> > >
> > > On Wed, Aug 7, 2019 at 8:37 PM Jason Gustafson <ja...@confluent.io>
> > wrote:
> > >
> > > > Hi All,
> > > >
> > > > I'd like to start a vote on KIP-497:
> > > >
> > > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-497%3A+Add+inter-broker+API+to+alter+ISR
> > > > .
> > > > +1
> > > > from me.
> > > >
> > > > -Jason
> > > >
> > >
> >
>
>
> --
> -- Guozhang
>
--
David Arthur
Re: [VOTE] KIP-497: Add inter-broker API to alter ISR
Posted by Guozhang Wang <wa...@gmail.com>.
+1 (binding). This is a great KIP, thanks Jason!
Regarding the naming of the zkVersion, I'm actually fine to name it more
generally and leave a note that at the moment its value is defined as the
zk version.
Guozhang
On Mon, Aug 12, 2019 at 2:22 PM Jason Gustafson <ja...@confluent.io> wrote:
> Hi Viktor,
>
> I originally named the field `CurrentVersion`. I didn't have 'Zk' in the
> name in anticipation of KIP-500. I thought about it and decided it makes
> sense to keep naming consistent with other APIs. Even if KIP-500 passes,
> there will be some time during which it only refers to the zk version.
> Eventually we'll have to decide whether it makes sense to change the name
> or just introduce a new field.
>
> Thanks,
> Jason
>
> On Fri, Aug 9, 2019 at 9:19 AM Viktor Somogyi-Vass <
> viktorsomogyi@gmail.com>
> wrote:
>
> > Hey Jason,
> >
> > +1 from me too.
> > One note though: since it's a new protocol we could perhaps rename
> > CurrentZkVersion to something like "IsrEpoch" or "IsrVersion". I think
> > that'd reflect its purpose better.
> >
> > Best,
> > Viktor
> >
> > On Wed, Aug 7, 2019 at 8:37 PM Jason Gustafson <ja...@confluent.io>
> wrote:
> >
> > > Hi All,
> > >
> > > I'd like to start a vote on KIP-497:
> > >
> > >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-497%3A+Add+inter-broker+API+to+alter+ISR
> > > .
> > > +1
> > > from me.
> > >
> > > -Jason
> > >
> >
>
--
-- Guozhang
Re: [VOTE] KIP-497: Add inter-broker API to alter ISR
Posted by Jason Gustafson <ja...@confluent.io>.
Hi Viktor,
I originally named the field `CurrentVersion`. I didn't have 'Zk' in the
name in anticipation of KIP-500. I thought about it and decided it makes
sense to keep naming consistent with other APIs. Even if KIP-500 passes,
there will be some time during which it only refers to the zk version.
Eventually we'll have to decide whether it makes sense to change the name
or just introduce a new field.
Thanks,
Jason
On Fri, Aug 9, 2019 at 9:19 AM Viktor Somogyi-Vass <vi...@gmail.com>
wrote:
> Hey Jason,
>
> +1 from me too.
> One note though: since it's a new protocol we could perhaps rename
> CurrentZkVersion to something like "IsrEpoch" or "IsrVersion". I think
> that'd reflect its purpose better.
>
> Best,
> Viktor
>
> On Wed, Aug 7, 2019 at 8:37 PM Jason Gustafson <ja...@confluent.io> wrote:
>
> > Hi All,
> >
> > I'd like to start a vote on KIP-497:
> >
> >
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-497%3A+Add+inter-broker+API+to+alter+ISR
> > .
> > +1
> > from me.
> >
> > -Jason
> >
>
Re: [VOTE] KIP-497: Add inter-broker API to alter ISR
Posted by Viktor Somogyi-Vass <vi...@gmail.com>.
Hey Jason,
+1 from me too.
One note though: since it's a new protocol we could perhaps rename
CurrentZkVersion to something like "IsrEpoch" or "IsrVersion". I think
that'd reflect its purpose better.
Best,
Viktor
On Wed, Aug 7, 2019 at 8:37 PM Jason Gustafson <ja...@confluent.io> wrote:
> Hi All,
>
> I'd like to start a vote on KIP-497:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-497%3A+Add+inter-broker+API+to+alter+ISR
> .
> +1
> from me.
>
> -Jason
>