Hi Omnia, Thanks for the KIP. I agree that we should migrate admin APIs to the new pattern.
DJ1: Why do we want to migrate only a subset of the APIs vs migrating all of them? For instance, there are DescribeGroups, ConsumerGroupDescribe, etc. Do we have reasons not to migrate them too? I think that it would be great to have a KIP that establishes the pattern for all the admin APIs. DJ2: I am not a fan of all the new parameters passed to the tools (e.g. --partition-size-limit-per-response). I wonder if we could rather have a config in the admin client to set the default page size for all requests. DJ3: I assume that the Admin client will transparently handle the change. It would be great to call it out in the compatibility section. Thanks, David On Tue, Jul 2, 2024 at 11:17 AM Andrew Schofield <andrew_schofi...@live.com> wrote: > Hi, > Thanks for the response. Makes sense to me. Just one additional comment: > > AS5: The cursor for ListGroupsResponse is called `TransactionalCursor` > which > seems like a copy-paste mistake. > > Thanks, > Andrew > > > On 30 Jun 2024, at 22:28, Omnia Ibrahim <o.g.h.ibra...@gmail.com> wrote: > > > > Hi Andrew thanks for having a look into the KIP > > > >> AS1: Besides topics, the most numerous resources in Kafka clusters in > my experience > >> are consumer groups. Would it be possible to extend the KIP to cover > ListGroups while > >> you’re in here? I’ve heard of clusters with truly vast numbers of > groups. This is also > >> potentially a sign of a misbehaving or poorly written clients. Getting > a page of groups > >> with a massive ItemsLeftToFetch would be nice. > > Yes, I also had few experiences with large cluster where to list > consumer groups can take up to 5min. I update the KIP to include this as > well. > > > >> AS2: A tiny nit: The versions for the added fields are incorrect in > some cases. > > I believe I fixed all of them now > > > >> AS3: I don’t quite understand the cursor for > OffsetFetchRequest/Response. > >> It looks like the cursor is (topic, partition), but not group ID. Does > the cursor > >> apply to all groups in the request, or is group ID missing? > > > > I was thinking that the last one in the response will be the one that > has the cursor while the rest will have null. But if we are moving > NextCursour to the top level of the response then the cursor will need > groupID. > >> AS4: For the remaining request/response pairs, the cursor makes sense > to me, > >> but I do wonder whether `NextCursor` should be at the top level of the > responses > >> instead, like DescribeTopicPartitionsResponse. > > > > Updates the KIP to reflect this now. > > > > Let me know if you have any more feedback on this. > > > > Best > > Omnia > > > >> On 27 Jun 2024, at 17:53, Andrew Schofield <andrew_schofi...@live.com> > wrote: > >> > >> Hi Omnia, > >> Thanks for the KIP. This is a really nice improvement for administering > large clusters. > >> > >> AS1: Besides topics, the most numerous resources in Kafka clusters in > my experience > >> are consumer groups. Would it be possible to extend the KIP to cover > ListGroups while > >> you’re in here? I’ve heard of clusters with truly vast numbers of > groups. This is also > >> potentially a sign of a misbehaving or poorly written clients. Getting > a page of groups > >> with a massive ItemsLeftToFetch would be nice. > >> > >> AS2: A tiny nit: The versions for the added fields are incorrect in > some cases. > >> > >> AS3: I don’t quite understand the cursor for > OffsetFetchRequest/Response. > >> It looks like the cursor is (topic, partition), but not group ID. Does > the cursor > >> apply to all groups in the request, or is group ID missing? > >> > >> AS4: For the remaining request/response pairs, the cursor makes sense > to me, > >> but I do wonder whether `NextCursor` should be at the top level of the > responses > >> instead, like DescribeTopicPartitionsResponse. > >> > >> Thanks, > >> Andrew > >> > >>> On 27 Jun 2024, at 14:05, Omnia Ibrahim <o.g.h.ibra...@gmail.com> > wrote: > >>> > >>> Hi everyone, I would like to start a discussion thread for KIP-1062 > >>> > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1062%3A+Introduce+Pagination+for+some+requests+used+by+Admin+API > >>> > >>> > >>> Thanks > >>> Omnia > > >
