You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@kafka.apache.org by Gwen Shapira <gw...@confluent.io> on 2017/09/01 06:36:04 UTC

Re: [VOTE] KIP-183 - Change PreferredReplicaLeaderElectionCommand to use AdminClient

Thank you! +1 (binding).

On Thu, Aug 31, 2017 at 9:48 AM Jun Rao <ju...@confluent.io> wrote:

> Hi, Tom,
>
> Thanks for the KIP. +1. Just one more minor comment. It seems that the
> ElectPreferredLeadersResponse
> should expect at least 3 other types of errors : (1) request timeout
> exception, (2) leader rebalance in-progress exception, (3) can't move to
> the preferred replica exception (i.e., preferred replica not in sync yet).
>
> Jun
>
> On Tue, Aug 29, 2017 at 8:56 AM, Tom Bentley <t....@gmail.com>
> wrote:
>
> > Hi all,
> >
> > I would like to start the vote on KIP-183 which will provide an
> AdminClient
> > interface for electing the preferred replica, and refactor the
> > kafka-preferred-replica-election.sh tool to use this interface. More
> > details here:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-183+-+Change+
> > PreferredReplicaLeaderElectionCommand+to+use+AdminClient
> >
> >
> > Regards,
> >
> > Tom
> >
>

Re: [VOTE] KIP-183 - Change PreferredReplicaLeaderElectionCommand to use AdminClient

Posted by Colin McCabe <cm...@apache.org>.
Thanks, Tom!  Great work.

best,
Colin

On Mon, Jan 28, 2019, at 04:33, Tom Bentley wrote:
> Hi Folks,
> 
> It took a while, but the work for KIP-183 has now been merged. My thanks to
> everyone involved.
> 
> A few details changed between what was voted on and what ultimately got
> merged. I've updated the KIP to reflect what was actually merged. If 
> anyone
> is interested in the gory details they can look at
> https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=73632065&selectedPageVersions=20&selectedPageVersions=18
> and
> https://github.com/apache/kafka/commit/269b65279c746bc54c611141a5a6509f9b310f11
> 
> Kind regards,
> 
> Tom
> 
> On Fri, 8 Sep 2017 at 16:30, Tom Bentley <t....@gmail.com> wrote:
> 
> > Since no one has objected, I conclude that this KIP is again accepted.
> >
> > Thanks,
> >
> > Tom
> >
> > On 7 September 2017 at 22:31, Guozhang Wang <wa...@gmail.com> wrote:
> >
> >> Hi Tom,
> >>
> >> The updated part in "AdminClient:electPreferredLeaders()" looks reasonable
> >> to me. If there is no objections from the voted committer by end of the
> >> day, I think you can mark it as accepted.
> >>
> >>
> >> Guozhang
> >>
> >>
> >> On Wed, Sep 6, 2017 at 7:42 AM, Tom Bentley <t....@gmail.com>
> >> wrote:
> >>
> >> > Unfortunately I've had to make a small change to the
> >> > ElectPreferredLeadersResult, because exposing a Map<TopicPartition,
> >> > KafkaFuture<Void>> was incompatible with the case where
> >> > electPreferredLeaders() was called with a null partitions argument. The
> >> > change exposes methods to access the map which return futures, rather
> >> than
> >> > exposing the map (and crucially its keys) directly.
> >> >
> >> > This is described in more detail in the [DISCUSS] thread.
> >> >
> >> > Please take a look and recast your votes:
> >> >
> >> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-183+-+Change+
> >> > PreferredReplicaLeaderElectionCommand+to+use+AdminClient#KIP-183-
> >> > ChangePreferredReplicaLeaderElectionCommandtouseAdminClient-AdminClient:
> >> > electPreferredLeaders()
> >> >
> >> > Thanks,
> >> >
> >> > Tom
> >> >
> >> > On 4 September 2017 at 10:52, Ismael Juma <is...@juma.me.uk> wrote:
> >> >
> >> > > Hi Tom,
> >> > >
> >> > > You can update the KIP for minor things like that. Worth updating the
> >> > > thread if it's something that is done during the PR review.
> >> > >
> >> > > With regards to exceptions, yes, that's definitely desired. I filed a
> >> > JIRA
> >> > > a while back for this:
> >> > >
> >> > > https://issues.apache.org/jira/browse/KAFKA-5445
> >> > >
> >> > > Ideally, new methods that we add would have this so that we don't
> >> > increase
> >> > > the tech debt that already exists.
> >> > >
> >> > > Ismael
> >> > >
> >> > > On Mon, Sep 4, 2017 at 10:11 AM, Tom Bentley <t....@gmail.com>
> >> > > wrote:
> >> > >
> >> > > > Hi Jun,
> >> > > >
> >> > > > You're correct about those other expected errors. If it's OK to
> >> update
> >> > > the
> >> > > > KIP after the vote I'll add those.
> >> > > >
> >> > > > But this makes me wonder about the value of documenting expected
> >> errors
> >> > > in
> >> > > > the Javadocs for the AdminClient (on the Results class, to be
> >> > specific).
> >> > > > Currently we don't do this, but it would be helpful for people using
> >> > the
> >> > > > AdminClient to know the kinds of errors they should expect, for
> >> testing
> >> > > > purposes for example. On the other hand it's a maintenance burden.
> >> > Should
> >> > > > we start documenting likely errors like this?
> >> > > >
> >> > > > Cheers,
> >> > > >
> >> > > > Tom
> >> > > >
> >> > > > On 4 September 2017 at 10:10, Tom Bentley <t....@gmail.com>
> >> > wrote:
> >> > > >
> >> > > > > I see three +1s, no +0s and no -1, so the vote passes.
> >> > > > >
> >> > > > > Thanks to those who voted and/or commented on the discussion
> >> thread.
> >> > > > >
> >> > > > > On 1 September 2017 at 07:36, Gwen Shapira <gw...@confluent.io>
> >> > wrote:
> >> > > > >
> >> > > > >> Thank you! +1 (binding).
> >> > > > >>
> >> > > > >> On Thu, Aug 31, 2017 at 9:48 AM Jun Rao <ju...@confluent.io>
> >> wrote:
> >> > > > >>
> >> > > > >> > Hi, Tom,
> >> > > > >> >
> >> > > > >> > Thanks for the KIP. +1. Just one more minor comment. It seems
> >> that
> >> > > the
> >> > > > >> > ElectPreferredLeadersResponse
> >> > > > >> > should expect at least 3 other types of errors : (1) request
> >> > timeout
> >> > > > >> > exception, (2) leader rebalance in-progress exception, (3)
> >> can't
> >> > > move
> >> > > > to
> >> > > > >> > the preferred replica exception (i.e., preferred replica not in
> >> > sync
> >> > > > >> yet).
> >> > > > >> >
> >> > > > >> > Jun
> >> > > > >> >
> >> > > > >> > On Tue, Aug 29, 2017 at 8:56 AM, Tom Bentley <
> >> > t.j.bentley@gmail.com
> >> > > >
> >> > > > >> > wrote:
> >> > > > >> >
> >> > > > >> > > Hi all,
> >> > > > >> > >
> >> > > > >> > > I would like to start the vote on KIP-183 which will provide
> >> an
> >> > > > >> > AdminClient
> >> > > > >> > > interface for electing the preferred replica, and refactor
> >> the
> >> > > > >> > > kafka-preferred-replica-election.sh tool to use this
> >> interface.
> >> > > > More
> >> > > > >> > > details here:
> >> > > > >> > >
> >> > > > >> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> >> > > 183+-+Change+
> >> > > > >> > > PreferredReplicaLeaderElectionCommand+to+use+AdminClient
> >> > > > >> > >
> >> > > > >> > >
> >> > > > >> > > Regards,
> >> > > > >> > >
> >> > > > >> > > Tom
> >> > > > >> > >
> >> > > > >> >
> >> > > > >>
> >> > > > >
> >> > > > >
> >> > > >
> >> > >
> >> >
> >>
> >>
> >>
> >> --
> >> -- Guozhang
> >>
> >
> >
>

Re: [VOTE] KIP-183 - Change PreferredReplicaLeaderElectionCommand to use AdminClient

Posted by Tom Bentley <t....@gmail.com>.
Hi Folks,

It took a while, but the work for KIP-183 has now been merged. My thanks to
everyone involved.

A few details changed between what was voted on and what ultimately got
merged. I've updated the KIP to reflect what was actually merged. If anyone
is interested in the gory details they can look at
https://cwiki.apache.org/confluence/pages/diffpagesbyversion.action?pageId=73632065&selectedPageVersions=20&selectedPageVersions=18
and
https://github.com/apache/kafka/commit/269b65279c746bc54c611141a5a6509f9b310f11

Kind regards,

Tom

On Fri, 8 Sep 2017 at 16:30, Tom Bentley <t....@gmail.com> wrote:

> Since no one has objected, I conclude that this KIP is again accepted.
>
> Thanks,
>
> Tom
>
> On 7 September 2017 at 22:31, Guozhang Wang <wa...@gmail.com> wrote:
>
>> Hi Tom,
>>
>> The updated part in "AdminClient:electPreferredLeaders()" looks reasonable
>> to me. If there is no objections from the voted committer by end of the
>> day, I think you can mark it as accepted.
>>
>>
>> Guozhang
>>
>>
>> On Wed, Sep 6, 2017 at 7:42 AM, Tom Bentley <t....@gmail.com>
>> wrote:
>>
>> > Unfortunately I've had to make a small change to the
>> > ElectPreferredLeadersResult, because exposing a Map<TopicPartition,
>> > KafkaFuture<Void>> was incompatible with the case where
>> > electPreferredLeaders() was called with a null partitions argument. The
>> > change exposes methods to access the map which return futures, rather
>> than
>> > exposing the map (and crucially its keys) directly.
>> >
>> > This is described in more detail in the [DISCUSS] thread.
>> >
>> > Please take a look and recast your votes:
>> >
>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-183+-+Change+
>> > PreferredReplicaLeaderElectionCommand+to+use+AdminClient#KIP-183-
>> > ChangePreferredReplicaLeaderElectionCommandtouseAdminClient-AdminClient:
>> > electPreferredLeaders()
>> >
>> > Thanks,
>> >
>> > Tom
>> >
>> > On 4 September 2017 at 10:52, Ismael Juma <is...@juma.me.uk> wrote:
>> >
>> > > Hi Tom,
>> > >
>> > > You can update the KIP for minor things like that. Worth updating the
>> > > thread if it's something that is done during the PR review.
>> > >
>> > > With regards to exceptions, yes, that's definitely desired. I filed a
>> > JIRA
>> > > a while back for this:
>> > >
>> > > https://issues.apache.org/jira/browse/KAFKA-5445
>> > >
>> > > Ideally, new methods that we add would have this so that we don't
>> > increase
>> > > the tech debt that already exists.
>> > >
>> > > Ismael
>> > >
>> > > On Mon, Sep 4, 2017 at 10:11 AM, Tom Bentley <t....@gmail.com>
>> > > wrote:
>> > >
>> > > > Hi Jun,
>> > > >
>> > > > You're correct about those other expected errors. If it's OK to
>> update
>> > > the
>> > > > KIP after the vote I'll add those.
>> > > >
>> > > > But this makes me wonder about the value of documenting expected
>> errors
>> > > in
>> > > > the Javadocs for the AdminClient (on the Results class, to be
>> > specific).
>> > > > Currently we don't do this, but it would be helpful for people using
>> > the
>> > > > AdminClient to know the kinds of errors they should expect, for
>> testing
>> > > > purposes for example. On the other hand it's a maintenance burden.
>> > Should
>> > > > we start documenting likely errors like this?
>> > > >
>> > > > Cheers,
>> > > >
>> > > > Tom
>> > > >
>> > > > On 4 September 2017 at 10:10, Tom Bentley <t....@gmail.com>
>> > wrote:
>> > > >
>> > > > > I see three +1s, no +0s and no -1, so the vote passes.
>> > > > >
>> > > > > Thanks to those who voted and/or commented on the discussion
>> thread.
>> > > > >
>> > > > > On 1 September 2017 at 07:36, Gwen Shapira <gw...@confluent.io>
>> > wrote:
>> > > > >
>> > > > >> Thank you! +1 (binding).
>> > > > >>
>> > > > >> On Thu, Aug 31, 2017 at 9:48 AM Jun Rao <ju...@confluent.io>
>> wrote:
>> > > > >>
>> > > > >> > Hi, Tom,
>> > > > >> >
>> > > > >> > Thanks for the KIP. +1. Just one more minor comment. It seems
>> that
>> > > the
>> > > > >> > ElectPreferredLeadersResponse
>> > > > >> > should expect at least 3 other types of errors : (1) request
>> > timeout
>> > > > >> > exception, (2) leader rebalance in-progress exception, (3)
>> can't
>> > > move
>> > > > to
>> > > > >> > the preferred replica exception (i.e., preferred replica not in
>> > sync
>> > > > >> yet).
>> > > > >> >
>> > > > >> > Jun
>> > > > >> >
>> > > > >> > On Tue, Aug 29, 2017 at 8:56 AM, Tom Bentley <
>> > t.j.bentley@gmail.com
>> > > >
>> > > > >> > wrote:
>> > > > >> >
>> > > > >> > > Hi all,
>> > > > >> > >
>> > > > >> > > I would like to start the vote on KIP-183 which will provide
>> an
>> > > > >> > AdminClient
>> > > > >> > > interface for electing the preferred replica, and refactor
>> the
>> > > > >> > > kafka-preferred-replica-election.sh tool to use this
>> interface.
>> > > > More
>> > > > >> > > details here:
>> > > > >> > >
>> > > > >> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
>> > > 183+-+Change+
>> > > > >> > > PreferredReplicaLeaderElectionCommand+to+use+AdminClient
>> > > > >> > >
>> > > > >> > >
>> > > > >> > > Regards,
>> > > > >> > >
>> > > > >> > > Tom
>> > > > >> > >
>> > > > >> >
>> > > > >>
>> > > > >
>> > > > >
>> > > >
>> > >
>> >
>>
>>
>>
>> --
>> -- Guozhang
>>
>
>

Re: [VOTE] KIP-183 - Change PreferredReplicaLeaderElectionCommand to use AdminClient

Posted by Tom Bentley <t....@gmail.com>.
Since no one has objected, I conclude that this KIP is again accepted.

Thanks,

Tom

On 7 September 2017 at 22:31, Guozhang Wang <wa...@gmail.com> wrote:

> Hi Tom,
>
> The updated part in "AdminClient:electPreferredLeaders()" looks reasonable
> to me. If there is no objections from the voted committer by end of the
> day, I think you can mark it as accepted.
>
>
> Guozhang
>
>
> On Wed, Sep 6, 2017 at 7:42 AM, Tom Bentley <t....@gmail.com> wrote:
>
> > Unfortunately I've had to make a small change to the
> > ElectPreferredLeadersResult, because exposing a Map<TopicPartition,
> > KafkaFuture<Void>> was incompatible with the case where
> > electPreferredLeaders() was called with a null partitions argument. The
> > change exposes methods to access the map which return futures, rather
> than
> > exposing the map (and crucially its keys) directly.
> >
> > This is described in more detail in the [DISCUSS] thread.
> >
> > Please take a look and recast your votes:
> >
> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-183+-+Change+
> > PreferredReplicaLeaderElectionCommand+to+use+AdminClient#KIP-183-
> > ChangePreferredReplicaLeaderElectionCommandtouseAdminClient-AdminClient:
> > electPreferredLeaders()
> >
> > Thanks,
> >
> > Tom
> >
> > On 4 September 2017 at 10:52, Ismael Juma <is...@juma.me.uk> wrote:
> >
> > > Hi Tom,
> > >
> > > You can update the KIP for minor things like that. Worth updating the
> > > thread if it's something that is done during the PR review.
> > >
> > > With regards to exceptions, yes, that's definitely desired. I filed a
> > JIRA
> > > a while back for this:
> > >
> > > https://issues.apache.org/jira/browse/KAFKA-5445
> > >
> > > Ideally, new methods that we add would have this so that we don't
> > increase
> > > the tech debt that already exists.
> > >
> > > Ismael
> > >
> > > On Mon, Sep 4, 2017 at 10:11 AM, Tom Bentley <t....@gmail.com>
> > > wrote:
> > >
> > > > Hi Jun,
> > > >
> > > > You're correct about those other expected errors. If it's OK to
> update
> > > the
> > > > KIP after the vote I'll add those.
> > > >
> > > > But this makes me wonder about the value of documenting expected
> errors
> > > in
> > > > the Javadocs for the AdminClient (on the Results class, to be
> > specific).
> > > > Currently we don't do this, but it would be helpful for people using
> > the
> > > > AdminClient to know the kinds of errors they should expect, for
> testing
> > > > purposes for example. On the other hand it's a maintenance burden.
> > Should
> > > > we start documenting likely errors like this?
> > > >
> > > > Cheers,
> > > >
> > > > Tom
> > > >
> > > > On 4 September 2017 at 10:10, Tom Bentley <t....@gmail.com>
> > wrote:
> > > >
> > > > > I see three +1s, no +0s and no -1, so the vote passes.
> > > > >
> > > > > Thanks to those who voted and/or commented on the discussion
> thread.
> > > > >
> > > > > On 1 September 2017 at 07:36, Gwen Shapira <gw...@confluent.io>
> > wrote:
> > > > >
> > > > >> Thank you! +1 (binding).
> > > > >>
> > > > >> On Thu, Aug 31, 2017 at 9:48 AM Jun Rao <ju...@confluent.io> wrote:
> > > > >>
> > > > >> > Hi, Tom,
> > > > >> >
> > > > >> > Thanks for the KIP. +1. Just one more minor comment. It seems
> that
> > > the
> > > > >> > ElectPreferredLeadersResponse
> > > > >> > should expect at least 3 other types of errors : (1) request
> > timeout
> > > > >> > exception, (2) leader rebalance in-progress exception, (3) can't
> > > move
> > > > to
> > > > >> > the preferred replica exception (i.e., preferred replica not in
> > sync
> > > > >> yet).
> > > > >> >
> > > > >> > Jun
> > > > >> >
> > > > >> > On Tue, Aug 29, 2017 at 8:56 AM, Tom Bentley <
> > t.j.bentley@gmail.com
> > > >
> > > > >> > wrote:
> > > > >> >
> > > > >> > > Hi all,
> > > > >> > >
> > > > >> > > I would like to start the vote on KIP-183 which will provide
> an
> > > > >> > AdminClient
> > > > >> > > interface for electing the preferred replica, and refactor the
> > > > >> > > kafka-preferred-replica-election.sh tool to use this
> interface.
> > > > More
> > > > >> > > details here:
> > > > >> > >
> > > > >> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > > 183+-+Change+
> > > > >> > > PreferredReplicaLeaderElectionCommand+to+use+AdminClient
> > > > >> > >
> > > > >> > >
> > > > >> > > Regards,
> > > > >> > >
> > > > >> > > Tom
> > > > >> > >
> > > > >> >
> > > > >>
> > > > >
> > > > >
> > > >
> > >
> >
>
>
>
> --
> -- Guozhang
>

Re: [VOTE] KIP-183 - Change PreferredReplicaLeaderElectionCommand to use AdminClient

Posted by Guozhang Wang <wa...@gmail.com>.
Hi Tom,

The updated part in "AdminClient:electPreferredLeaders()" looks reasonable
to me. If there is no objections from the voted committer by end of the
day, I think you can mark it as accepted.


Guozhang


On Wed, Sep 6, 2017 at 7:42 AM, Tom Bentley <t....@gmail.com> wrote:

> Unfortunately I've had to make a small change to the
> ElectPreferredLeadersResult, because exposing a Map<TopicPartition,
> KafkaFuture<Void>> was incompatible with the case where
> electPreferredLeaders() was called with a null partitions argument. The
> change exposes methods to access the map which return futures, rather than
> exposing the map (and crucially its keys) directly.
>
> This is described in more detail in the [DISCUSS] thread.
>
> Please take a look and recast your votes:
>
> https://cwiki.apache.org/confluence/display/KAFKA/KIP-183+-+Change+
> PreferredReplicaLeaderElectionCommand+to+use+AdminClient#KIP-183-
> ChangePreferredReplicaLeaderElectionCommandtouseAdminClient-AdminClient:
> electPreferredLeaders()
>
> Thanks,
>
> Tom
>
> On 4 September 2017 at 10:52, Ismael Juma <is...@juma.me.uk> wrote:
>
> > Hi Tom,
> >
> > You can update the KIP for minor things like that. Worth updating the
> > thread if it's something that is done during the PR review.
> >
> > With regards to exceptions, yes, that's definitely desired. I filed a
> JIRA
> > a while back for this:
> >
> > https://issues.apache.org/jira/browse/KAFKA-5445
> >
> > Ideally, new methods that we add would have this so that we don't
> increase
> > the tech debt that already exists.
> >
> > Ismael
> >
> > On Mon, Sep 4, 2017 at 10:11 AM, Tom Bentley <t....@gmail.com>
> > wrote:
> >
> > > Hi Jun,
> > >
> > > You're correct about those other expected errors. If it's OK to update
> > the
> > > KIP after the vote I'll add those.
> > >
> > > But this makes me wonder about the value of documenting expected errors
> > in
> > > the Javadocs for the AdminClient (on the Results class, to be
> specific).
> > > Currently we don't do this, but it would be helpful for people using
> the
> > > AdminClient to know the kinds of errors they should expect, for testing
> > > purposes for example. On the other hand it's a maintenance burden.
> Should
> > > we start documenting likely errors like this?
> > >
> > > Cheers,
> > >
> > > Tom
> > >
> > > On 4 September 2017 at 10:10, Tom Bentley <t....@gmail.com>
> wrote:
> > >
> > > > I see three +1s, no +0s and no -1, so the vote passes.
> > > >
> > > > Thanks to those who voted and/or commented on the discussion thread.
> > > >
> > > > On 1 September 2017 at 07:36, Gwen Shapira <gw...@confluent.io>
> wrote:
> > > >
> > > >> Thank you! +1 (binding).
> > > >>
> > > >> On Thu, Aug 31, 2017 at 9:48 AM Jun Rao <ju...@confluent.io> wrote:
> > > >>
> > > >> > Hi, Tom,
> > > >> >
> > > >> > Thanks for the KIP. +1. Just one more minor comment. It seems that
> > the
> > > >> > ElectPreferredLeadersResponse
> > > >> > should expect at least 3 other types of errors : (1) request
> timeout
> > > >> > exception, (2) leader rebalance in-progress exception, (3) can't
> > move
> > > to
> > > >> > the preferred replica exception (i.e., preferred replica not in
> sync
> > > >> yet).
> > > >> >
> > > >> > Jun
> > > >> >
> > > >> > On Tue, Aug 29, 2017 at 8:56 AM, Tom Bentley <
> t.j.bentley@gmail.com
> > >
> > > >> > wrote:
> > > >> >
> > > >> > > Hi all,
> > > >> > >
> > > >> > > I would like to start the vote on KIP-183 which will provide an
> > > >> > AdminClient
> > > >> > > interface for electing the preferred replica, and refactor the
> > > >> > > kafka-preferred-replica-election.sh tool to use this interface.
> > > More
> > > >> > > details here:
> > > >> > >
> > > >> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> > 183+-+Change+
> > > >> > > PreferredReplicaLeaderElectionCommand+to+use+AdminClient
> > > >> > >
> > > >> > >
> > > >> > > Regards,
> > > >> > >
> > > >> > > Tom
> > > >> > >
> > > >> >
> > > >>
> > > >
> > > >
> > >
> >
>



-- 
-- Guozhang

Re: [VOTE] KIP-183 - Change PreferredReplicaLeaderElectionCommand to use AdminClient

Posted by Tom Bentley <t....@gmail.com>.
Unfortunately I've had to make a small change to the
ElectPreferredLeadersResult, because exposing a Map<TopicPartition,
KafkaFuture<Void>> was incompatible with the case where
electPreferredLeaders() was called with a null partitions argument. The
change exposes methods to access the map which return futures, rather than
exposing the map (and crucially its keys) directly.

This is described in more detail in the [DISCUSS] thread.

Please take a look and recast your votes:

https://cwiki.apache.org/confluence/display/KAFKA/KIP-183+-+Change+
PreferredReplicaLeaderElectionCommand+to+use+AdminClient#KIP-183-
ChangePreferredReplicaLeaderElectionCommandtouseAdminClient-AdminClient:
electPreferredLeaders()

Thanks,

Tom

On 4 September 2017 at 10:52, Ismael Juma <is...@juma.me.uk> wrote:

> Hi Tom,
>
> You can update the KIP for minor things like that. Worth updating the
> thread if it's something that is done during the PR review.
>
> With regards to exceptions, yes, that's definitely desired. I filed a JIRA
> a while back for this:
>
> https://issues.apache.org/jira/browse/KAFKA-5445
>
> Ideally, new methods that we add would have this so that we don't increase
> the tech debt that already exists.
>
> Ismael
>
> On Mon, Sep 4, 2017 at 10:11 AM, Tom Bentley <t....@gmail.com>
> wrote:
>
> > Hi Jun,
> >
> > You're correct about those other expected errors. If it's OK to update
> the
> > KIP after the vote I'll add those.
> >
> > But this makes me wonder about the value of documenting expected errors
> in
> > the Javadocs for the AdminClient (on the Results class, to be specific).
> > Currently we don't do this, but it would be helpful for people using the
> > AdminClient to know the kinds of errors they should expect, for testing
> > purposes for example. On the other hand it's a maintenance burden. Should
> > we start documenting likely errors like this?
> >
> > Cheers,
> >
> > Tom
> >
> > On 4 September 2017 at 10:10, Tom Bentley <t....@gmail.com> wrote:
> >
> > > I see three +1s, no +0s and no -1, so the vote passes.
> > >
> > > Thanks to those who voted and/or commented on the discussion thread.
> > >
> > > On 1 September 2017 at 07:36, Gwen Shapira <gw...@confluent.io> wrote:
> > >
> > >> Thank you! +1 (binding).
> > >>
> > >> On Thu, Aug 31, 2017 at 9:48 AM Jun Rao <ju...@confluent.io> wrote:
> > >>
> > >> > Hi, Tom,
> > >> >
> > >> > Thanks for the KIP. +1. Just one more minor comment. It seems that
> the
> > >> > ElectPreferredLeadersResponse
> > >> > should expect at least 3 other types of errors : (1) request timeout
> > >> > exception, (2) leader rebalance in-progress exception, (3) can't
> move
> > to
> > >> > the preferred replica exception (i.e., preferred replica not in sync
> > >> yet).
> > >> >
> > >> > Jun
> > >> >
> > >> > On Tue, Aug 29, 2017 at 8:56 AM, Tom Bentley <t.j.bentley@gmail.com
> >
> > >> > wrote:
> > >> >
> > >> > > Hi all,
> > >> > >
> > >> > > I would like to start the vote on KIP-183 which will provide an
> > >> > AdminClient
> > >> > > interface for electing the preferred replica, and refactor the
> > >> > > kafka-preferred-replica-election.sh tool to use this interface.
> > More
> > >> > > details here:
> > >> > >
> > >> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-
> 183+-+Change+
> > >> > > PreferredReplicaLeaderElectionCommand+to+use+AdminClient
> > >> > >
> > >> > >
> > >> > > Regards,
> > >> > >
> > >> > > Tom
> > >> > >
> > >> >
> > >>
> > >
> > >
> >
>

Re: [VOTE] KIP-183 - Change PreferredReplicaLeaderElectionCommand to use AdminClient

Posted by Ismael Juma <is...@juma.me.uk>.
Hi Tom,

You can update the KIP for minor things like that. Worth updating the
thread if it's something that is done during the PR review.

With regards to exceptions, yes, that's definitely desired. I filed a JIRA
a while back for this:

https://issues.apache.org/jira/browse/KAFKA-5445

Ideally, new methods that we add would have this so that we don't increase
the tech debt that already exists.

Ismael

On Mon, Sep 4, 2017 at 10:11 AM, Tom Bentley <t....@gmail.com> wrote:

> Hi Jun,
>
> You're correct about those other expected errors. If it's OK to update the
> KIP after the vote I'll add those.
>
> But this makes me wonder about the value of documenting expected errors in
> the Javadocs for the AdminClient (on the Results class, to be specific).
> Currently we don't do this, but it would be helpful for people using the
> AdminClient to know the kinds of errors they should expect, for testing
> purposes for example. On the other hand it's a maintenance burden. Should
> we start documenting likely errors like this?
>
> Cheers,
>
> Tom
>
> On 4 September 2017 at 10:10, Tom Bentley <t....@gmail.com> wrote:
>
> > I see three +1s, no +0s and no -1, so the vote passes.
> >
> > Thanks to those who voted and/or commented on the discussion thread.
> >
> > On 1 September 2017 at 07:36, Gwen Shapira <gw...@confluent.io> wrote:
> >
> >> Thank you! +1 (binding).
> >>
> >> On Thu, Aug 31, 2017 at 9:48 AM Jun Rao <ju...@confluent.io> wrote:
> >>
> >> > Hi, Tom,
> >> >
> >> > Thanks for the KIP. +1. Just one more minor comment. It seems that the
> >> > ElectPreferredLeadersResponse
> >> > should expect at least 3 other types of errors : (1) request timeout
> >> > exception, (2) leader rebalance in-progress exception, (3) can't move
> to
> >> > the preferred replica exception (i.e., preferred replica not in sync
> >> yet).
> >> >
> >> > Jun
> >> >
> >> > On Tue, Aug 29, 2017 at 8:56 AM, Tom Bentley <t....@gmail.com>
> >> > wrote:
> >> >
> >> > > Hi all,
> >> > >
> >> > > I would like to start the vote on KIP-183 which will provide an
> >> > AdminClient
> >> > > interface for electing the preferred replica, and refactor the
> >> > > kafka-preferred-replica-election.sh tool to use this interface.
> More
> >> > > details here:
> >> > >
> >> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-183+-+Change+
> >> > > PreferredReplicaLeaderElectionCommand+to+use+AdminClient
> >> > >
> >> > >
> >> > > Regards,
> >> > >
> >> > > Tom
> >> > >
> >> >
> >>
> >
> >
>

Re: [VOTE] KIP-183 - Change PreferredReplicaLeaderElectionCommand to use AdminClient

Posted by Tom Bentley <t....@gmail.com>.
Hi Jun,

You're correct about those other expected errors. If it's OK to update the
KIP after the vote I'll add those.

But this makes me wonder about the value of documenting expected errors in
the Javadocs for the AdminClient (on the Results class, to be specific).
Currently we don't do this, but it would be helpful for people using the
AdminClient to know the kinds of errors they should expect, for testing
purposes for example. On the other hand it's a maintenance burden. Should
we start documenting likely errors like this?

Cheers,

Tom

On 4 September 2017 at 10:10, Tom Bentley <t....@gmail.com> wrote:

> I see three +1s, no +0s and no -1, so the vote passes.
>
> Thanks to those who voted and/or commented on the discussion thread.
>
> On 1 September 2017 at 07:36, Gwen Shapira <gw...@confluent.io> wrote:
>
>> Thank you! +1 (binding).
>>
>> On Thu, Aug 31, 2017 at 9:48 AM Jun Rao <ju...@confluent.io> wrote:
>>
>> > Hi, Tom,
>> >
>> > Thanks for the KIP. +1. Just one more minor comment. It seems that the
>> > ElectPreferredLeadersResponse
>> > should expect at least 3 other types of errors : (1) request timeout
>> > exception, (2) leader rebalance in-progress exception, (3) can't move to
>> > the preferred replica exception (i.e., preferred replica not in sync
>> yet).
>> >
>> > Jun
>> >
>> > On Tue, Aug 29, 2017 at 8:56 AM, Tom Bentley <t....@gmail.com>
>> > wrote:
>> >
>> > > Hi all,
>> > >
>> > > I would like to start the vote on KIP-183 which will provide an
>> > AdminClient
>> > > interface for electing the preferred replica, and refactor the
>> > > kafka-preferred-replica-election.sh tool to use this interface. More
>> > > details here:
>> > >
>> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-183+-+Change+
>> > > PreferredReplicaLeaderElectionCommand+to+use+AdminClient
>> > >
>> > >
>> > > Regards,
>> > >
>> > > Tom
>> > >
>> >
>>
>
>

Re: [VOTE] KIP-183 - Change PreferredReplicaLeaderElectionCommand to use AdminClient

Posted by Tom Bentley <t....@gmail.com>.
I see three +1s, no +0s and no -1, so the vote passes.

Thanks to those who voted and/or commented on the discussion thread.

On 1 September 2017 at 07:36, Gwen Shapira <gw...@confluent.io> wrote:

> Thank you! +1 (binding).
>
> On Thu, Aug 31, 2017 at 9:48 AM Jun Rao <ju...@confluent.io> wrote:
>
> > Hi, Tom,
> >
> > Thanks for the KIP. +1. Just one more minor comment. It seems that the
> > ElectPreferredLeadersResponse
> > should expect at least 3 other types of errors : (1) request timeout
> > exception, (2) leader rebalance in-progress exception, (3) can't move to
> > the preferred replica exception (i.e., preferred replica not in sync
> yet).
> >
> > Jun
> >
> > On Tue, Aug 29, 2017 at 8:56 AM, Tom Bentley <t....@gmail.com>
> > wrote:
> >
> > > Hi all,
> > >
> > > I would like to start the vote on KIP-183 which will provide an
> > AdminClient
> > > interface for electing the preferred replica, and refactor the
> > > kafka-preferred-replica-election.sh tool to use this interface. More
> > > details here:
> > >
> > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-183+-+Change+
> > > PreferredReplicaLeaderElectionCommand+to+use+AdminClient
> > >
> > >
> > > Regards,
> > >
> > > Tom
> > >
> >
>