Hi, Tom,
Thanks for the KIP. Looks good overall. A few minor comments.
1. In most requests with topic partitions, we nest partitions inside topic.
So, instead of [topic partition_id], we do [topic, [partition_id]] to save
some space.
2. The preferred leader election just tries to move the leader to the
preferred replica. This may not succeed if the preferred replica is not
in-sync yet. It would be useful to return an error code to reflect that.
3. REPLICA_LEADER_ELECTION_IN_PROGRESS: Now that the controller is single
threaded, is that just for timeout? If so, perhaps we can just return the
Timeout error code.
4. Permission wise, it seems this should require ClusterAction on Cluster
resource since it's changing things at the cluster level.
Thanks,
Jun
On Wed, Aug 9, 2017 at 6:16 AM, Ismael Juma <ism...@juma.me.uk> wrote:
> On Wed, Aug 9, 2017 at 11:40 AM, Tom Bentley <t.j.bent...@gmail.com>
> wrote:
> >
> > There are responses with detailed error messages as well as the codes,
> > CreateTopicsResponse, {Describe|Alter}ConfigsResponse, and the responses
> > for managing ACLs for instance. To be honest, I assumed including a
> message
> > was the norm. In this case, however, I don't think there is any extra
> > detail to include beyond the error itself, so I've removed it from the
> KIP.
> >
>
> We started sending back actual error messages when we introduced the Create
> Topic Policy as the errors are custom and much more helpful if a string can
> be sent back.
>
> In general, I think it would be better if we had an optional error message
> for all request types as error codes alone sometimes result in people
> having to check the broker logs. Examples that come to mind are Group
> Coordinator Not Available and Invalid Request. For the latter, we want to
> include what was invalid and it's not practical to have an error code for
> every possible validation error (this becomes more obvious once we start
> adding admin protocol APIs).
>
> Ismael
>
