Hi Colin, Thanks for your input. A couple of comments inline.
On 22 August 2017 at 21:42, Colin McCabe <cmcc...@apache.org> wrote: > Hi, > > Thanks for the KIP. It looks good overall. > > On Tue, Aug 22, 2017, at 08:54, Tom Bentley wrote: > > Hi Jun, > > > > Thanks for the feedback. > > > > I've updated the KIP to mention this new algorithm, which I agree will be > > much better from the AdminClient PoV. > > > > I've also reverted the authorization to be against the cluster resource, > > rather than the topic, since, on re-reading, Ewen wasn't particularly > > advocating that it should be topic-based, merely wondering if that made > > more sense in addition to the cluster-based check. > > > > Hmm. In general I thought we were using Cluster:ClusterAction for > authorizing internal requests made by Kafka itself, and Cluster:Alter > for administrative requests initiated by the admin. For example, the > RPCs added in KIP-140 use Cluster:Alter. > > Nitpick: I think you meant to remove this line from the KIP? > > > TOPIC_AUTHORIZATION_FAILED (29) If the user didn't have Alter access to > the topic. > > You're right, both now fixed. > Also... this command simply starts the leader election, but does not > wait for it to complete, right? What is the preferred method for an > admin to check the final result or identify when the final result has > been achieved? > > As suggested by Jun and mentioned in the section "Broker-side election algorithm" the KIP proposes (as of yesterday) to change how the election is performed. Rather than the current process of writing to the znode and having the controller react to that to perform the election, I am now proposing that the request from the AdminClient to the controller directly starts the election, and the response is not sent to the AdminClient until the election is complete. Thus, the API presented by the AdminClient is synchronous and the tool itself only returns when the elections have finished (or timed-out). Thanks again, Tom
