Hi Magnus, Thanks for the review, I've added //moved and explanation as requested.
Thanks Tom On Wed, Jan 27, 2021 at 12:05 PM Magnus Edenhill <mag...@edenhill.se> wrote: > Hey Thomas, > > I'm late to the game. > > It looks like the "top level" ErrorCode moved from the top-level to the > Group array, which makes sense, > but it would be good if it was marked as // MOVED in the KIP and also a > note that top level errors that > are unrelated to the group will be returned as per-group errors. > > > Regards, > Magnus > > > Den tis 26 jan. 2021 kl 15:42 skrev Thomas Scott <t...@confluent.io>: > > > Thanks David I've updated it. > > > > On Tue, Jan 26, 2021 at 1:55 PM David Jacot <dja...@confluent.io> wrote: > > > > > Great. That answers my question! > > > > > > Thomas, I suggest adding a Related/Future Work section in the > > > KIP to link KIP-699 more explicitly. > > > > > > Thanks, > > > David > > > > > > On Tue, Jan 26, 2021 at 1:30 PM Thomas Scott <t...@confluent.io> wrote: > > > > > > > Hi Mickael/David, > > > > > > > > I feel like the combination of these 2 KIPs gives the complete > > solution > > > > but they can be implemented independently. I have added a description > > and > > > > links to KIP-699 to KIP-709 to this effect. > > > > > > > > Thanks > > > > > > > > Tom > > > > > > > > > > > > On Tue, Jan 26, 2021 at 11:44 AM Mickael Maison < > > > mickael.mai...@gmail.com> > > > > wrote: > > > > > > > > > Hi Thomas, > > > > > Thanks, the KIP looks good. > > > > > > > > > > David, > > > > > I started working on exactly that a few weeks ago: > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-699%3A+FindCoordinators > > > > > I hope to complete my draft and start a discussion later on this > > week. > > > > > > > > > > Thanks > > > > > > > > > > On Tue, Jan 26, 2021 at 10:06 AM David Jacot <dja...@confluent.io> > > > > wrote: > > > > > > > > > > > > Hi Thomas, > > > > > > > > > > > > Thanks for the KIP. Overall, the KIP looks good to me. > > > > > > > > > > > > I have only one question: The FindCoordinator API only supports > > > > > > resolving one group id at the time. If we want to get the offsets > > for > > > > > > say N groups, that means that we have to first issue N > > > FindCoordinator > > > > > > requests, wait for the responses, group by coordinators, and then > > > > > > send a OffsetFetch request per coordinator. I wonder if we should > > > > > > also extend the FindCoordinator API to support resolving multiple > > > > > > groups as well. This would make the implementation in the admin > > > > > > client a bit easier and would ensure that we can handle multiple > > > > > > groups end-to-end. Have you thought about this? > > > > > > > > > > > > Best, > > > > > > David > > > > > > > > > > > > On Tue, Jan 26, 2021 at 10:13 AM Rajini Sivaram < > > > > rajinisiva...@gmail.com > > > > > > > > > > > > wrote: > > > > > > > > > > > > > Hi Thomas, > > > > > > > > > > > > > > Thanks for the KIP, this is a useful addition for admin use > > cases. > > > It > > > > > may > > > > > > > be worth starting the voting thread soon if we want to get this > > > into > > > > > 2.8.0. > > > > > > > > > > > > > > Regards, > > > > > > > > > > > > > > Rajini > > > > > > > > > > > > > > > > > > > > > On Mon, Jan 25, 2021 at 1:52 PM Thomas Scott <t...@confluent.io > > > > > > wrote: > > > > > > > > > > > > > > > Thanks Ismael, that's a lot better. I've updated the KIP with > > > this > > > > > > > > behaviour instead. > > > > > > > > > > > > > > > > On Mon, Jan 25, 2021 at 11:42 AM Ismael Juma < > > ism...@juma.me.uk> > > > > > wrote: > > > > > > > > > > > > > > > > > Thanks for the KIP, Thomas. One question below: > > > > > > > > > > > > > > > > > > Should an Admin client with this new functionality be used > > > > against > > > > > an > > > > > > > old > > > > > > > > > > broker that cannot handle these requests then the methods > > > will > > > > > throw > > > > > > > > > > UnsupportedVersionException as per the usual pattern. > > > > > > > > > > > > > > > > > > > > > > > > > > > Did we consider automatically falling back to the single > > group > > > id > > > > > > > request > > > > > > > > > if the more efficient one is not supported? > > > > > > > > > > > > > > > > > > Ismael > > > > > > > > > > > > > > > > > > On Mon, Jan 25, 2021 at 3:34 AM Thomas Scott < > > t...@confluent.io > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Hi, > > > > > > > > > > > > > > > > > > > > I'm starting this thread to discuss KIP-709 to extend > > > > OffsetFetch > > > > > > > > > requests > > > > > > > > > > to accept multiple group ids. Please check out the KIP > > here: > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=173084258 > > > > > > > > > > > > > > > > > > > > Any comments much appreciated. > > > > > > > > > > > > > > > > > > > > thanks, > > > > > > > > > > > > > > > > > > > > Tom > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >
