I have updated the KIP. Best, David
On Fri, Nov 19, 2021 at 3:00 PM David Jacot <dja...@confluent.io> wrote: > > Thank you all for your feedback. Let me address all your points below. > > Luke, > 1. I use a tag field so bumping the request version is not necessary. In > this case, using a tag field does not seem to be the best approach so > I will use a regular one and bump the version. > 2. Sounds good. I will fix that comment. > 3. That is a good question. My intent was to avoid getting weird or cryptic > reasons logged on the broker so I thought that having a standard one is > better. As Sophie suggested something similar for the `enforceRebalance` > API, we could do it for both, I suppose. > > Ismael, > 1. That's a good point. I chose to use a tag field to avoid having to bump > the request version. In this particular case, it seems that bumping the > version does not cost much so it is perhaps better. I will change this. > > Sophie, > 1. That's a pretty good idea, thanks. Let me update the KIP to include > a reason in the JoinGroup request. Regarding the Consumer API, do > you think that there is value for KStreams to expose the possibility to > provide a reason? Otherwise, it might be better to use a default > reason in this case. > 2. I don't fully get your point about providing the reason to the group > leader assignor on the client. Do you think that we should propagate > it to all the consumers or to the leader as well? The user usually has > access to all its client logs so I am not sure that it is really necessary. > Could you elaborate? > > I will update the KIP soon. > > Best, > David > > On Sat, Nov 13, 2021 at 6:00 AM Sophie Blee-Goldman > <sop...@confluent.io.invalid> wrote: > > > > This sounds great, thanks David. > > > > One thought: what do you think about doing something similar for the > > JoinGroup request? > > > > When you only have broker logs and not client logs, it's somewhere between > > challenging and > > impossible to determine the reason for a rebalance that was triggered > > explicitly by the client or > > even the user. For example, when a followup rebalance is requested to > > assign the revoked > > partitions after a cooperative rebalance. Or any of the many reasons we > > trigger a rebalance > > in Kafka Streams, via the #enforceRebalance API. > > > > Perhaps we could add a parameter to that method as such: > > > > public void enforceRebalance(final String reason); > > > > Then we can add that to the JoinGroup request/ConsumerProtocol. Not only > > would it help to > > log this reason on the broker side, the information about who requested the > > rebalance and why > > could be extremely useful to have available to the group leader/partition > > assignor on the client > > side. > > > > Cheers, > > Sophie > > > > On Fri, Nov 12, 2021 at 10:05 AM Ismael Juma <ism...@juma.me.uk> wrote: > > > > > Thanks David, this is a worthwhile improvement. Quick question, why did we > > > pick a tagged field here? > > > > > > Ismael > > > > > > On Thu, Nov 11, 2021, 8:32 AM David Jacot <dja...@confluent.io.invalid> > > > wrote: > > > > > > > Hi folks, > > > > > > > > I'd like to discuss this very small KIP which proposes to add a reason > > > > field > > > > to the LeaveGroupRequest in order to let the broker know why a member > > > > left the group. This would be really handy for administrators. > > > > > > > > KIP: https://cwiki.apache.org/confluence/x/eYyqCw > > > > > > > > Cheers, > > > > David > > > > > > >
