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.j.bent...@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 <ism...@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.j.bent...@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.j.bent...@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 <g...@confluent.io> > wrote: > > > > > > > >> Thank you! +1 (binding). > > > >> > > > >> On Thu, Aug 31, 2017 at 9:48 AM Jun Rao <j...@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.bent...@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
