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
