Hi Nicole, Thanks, LGTM!
Regards, Rajini On Mon, Nov 2, 2026 at 11:05 PM Nicole Du <[email protected]> wrote: > Hi Rajini, > > Thanks for the questions! > > For RS01: Done. > > For RS02: Yes, MM2 would need to retry without the option to support older > brokers. It currently uses valid(), so it could preserve its existing > handling of partial results by inspecting errors() on the same > ListGroupsResult. If any error is an UnsupportedVersionException, it would > retry once with only requireCommittedOffsets disabled, keeping the other > filters unchanged. The retry result would replace the initial result. I > have added this adoption guidance to the KIP and clarified that changes to > MM2 itself are not proposed here. > > For RS03: An existence lookup is sufficient. The committed-offsets map > removes a group's entry when its last offset is deleted, and offsets of > uncommitted transactions are stored separately. Checking for the group's > entry against the same committed state as the group listing gives the > required semantics without additional state or a per-partition scan. > > Best, > Nicole > > On Mon, Nov 2, 2026 at 5:40 AM Rajini Sivaram <[email protected]> > wrote: > > > Hi Nicole, > > > > Thanks for the KIP! The KIP looks good, just a few minor comments: > > > > RS01: Can we add a link to the KIP in the KIPs under discussion table in > > > > > https://cwiki.apache.org/confluence/spaces/KAFKA/pages/50859233/Kafka+Improvement+Proposals > > ? > > > > RS02: The KIP gives an example of using `all()` instead of `valid()`. > Since > > we talk about MirrorMaker in the motivation section, it will be good to > > include details about how MirrorMaker can be updated to use this KIP. In > > addition to calling ListGroupsOptions.forConsumerGroups(). > > requireCommittedOffsets(true)).all().get(); , MM2 would also need to > retry > > without requireCommittedOffsets on UnsupportedVersionException? > > > > RS03: The KIP says that the filter "*adds a per-group in-memory existence > > check*". Is this just a boolean check or does it need anything additional > > for the semantics described in the KIP? > > > > Thanks, > > > > Rajini > > > > On Wed, Oct 28, 2026 at 4:28 PM Nicole Du <[email protected]> wrote: > > > > > Hi David, > > > > > > Thanks for taking a look and for the suggestions! > > > > > > For DJ01: My reasoning was to keep the predicate focused on offsets > that > > > have already been committed. Pending transactional offsets may still be > > > aborted, so they do not count on their own. A group with an existing > > > committed offset still matches even if it also has pending offsets. > > > > > > I’ve clarified this in the KIP. One trade-off is that a pending-only > > group > > > that commits after the listing is evaluated needs a subsequent listing > to > > > be discovered. I’d appreciate your thoughts on whether including > > > pending-only groups would be more useful for callers. > > > > > > For DJ02: That makes sense. I’ve replaced the snapshot-specific wording > > > with a definition based on the coordinator’s committed state and > > simplified > > > the table to three cases: at least one committed offset, pending > offsets > > > only, and neither. A separate sentence clarifies that offsets eligible > > for > > > expiration continue to count until their deletion takes effect. > > > > > > I’ve also made two small updates to the existing test plan: explicitly > > > covering pending offsets through transaction commit and abort, and > > > describing the visibility test in terms of committed state. > > > > > > Best, > > > Nicole > > > > > > On Wed, Oct 28, 2026 at 7:51 AM David Jacot <[email protected]> wrote: > > > > > > > Hi Nicole, > > > > > > > > Thanks for the KIP! I have only two comments: > > > > > > > > DJ01: What's the reasoning for not including groups with only pending > > > > offsets? > > > > DJ02: I wonder if we should simplify the "State of the group at the > > > > read snapshot" table. For instance, "is visible in the snapshot" is > an > > > > implementation detail. We should rather say that the condition is > > > > applied to the committed state and simplify the table. > > > > > > > > Best, > > > > David > > > > > > > > > > > > On Mon, Oct 26, 2026 at 11:49 PM Nicole Du <[email protected]> > > wrote: > > > > > > > > > > Hi all, > > > > > > > > > > I'd like to start a discussion on KIP-1388, which proposes adding a > > > > > request-side filter to the ListGroups API to allow Admin.listGroups > > to > > > > > return only groups with at least one committed offset. > > > > > > > > > > Tools that replicate consumer group offsets across clusters, such > as > > > > > MirrorMaker 2 and the group offset sync in KIP-1279, typically list > > all > > > > > groups before fetching their offsets. In large deployments, many > > groups > > > > may > > > > > have active members but no committed offsets, resulting in > > unnecessary > > > > > OffsetFetch requests and additional load on group coordinators. > > > > > > > > > > This KIP proposes filtering these groups on the server side by > > > > introducing > > > > > a new request field in ListGroups v6 and exposing it through > > > > > ListGroupsOptions.requireCommittedOffsets(boolean). > > > > > > > > > > KIP: > > > > > > > > > > > > > > > https://cwiki.apache.org/confluence/display/KAFKA/KIP-1388%3A+Add+a+committed-offsets+filter+to+ListGroups > > > > > > > > > > JIRA: https://issues.apache.org/jira/browse/KAFKA-21366 > > > > > > > > > > I'd appreciate any feedback or suggestions. > > > > > > > > > > Thanks, > > > > > Nicole > > > > > > > > > >
