Hi all, Thanks for the feedback! Replies inline below.
Andrew: AS1: Very good question and to be honest I don't know exactly why. I have noted two things while looking at it. a) The ignorable in the OffsetCommitRequest is basically useless now as the versions 0 and 1 were removed. I suppose that we could remove it. I will check this separately. b) The TxnOffsetCommitRequest builder explicitly throws an unsupported exception when the fields are set but the broker does not support the version. It seems that we wanted to explicitly disallow it while we were fine with ignoring those fields on the non-transactional path. Chia-Ping: chia1: Agreed. KIP-1136 actually does it and uses memberEpoch. There is no point in keeping the old terminology there. Lianet: LM1: Good question. After thinking it through, I'd lean toward just adding the topics to the producer's metadata cache and letting them be evicted naturally once they go idle — no explicit cleanup needed. The reason the consumer uses an explicit transient bucket is that its metadata cache has no notion of idle eviction: topics stay until unsubscribed. The producer's cache works differently — every time a topic is added, its idle timer is refreshed, and once a topic hasn't been touched for `metadata.max.idle.ms` it's dropped on the next metadata update. So for the producer, "add and forget" already gives us transient semantics for free. This also has a small benefit in consume-transform-produce loops with frequent commits: the topics from the offset map stay cached between commits, so subsequent `sendOffsetsToTransaction` calls don't have to wait on a fresh metadata fetch (bounded by `max.block.ms`). Topics that genuinely fall out of use are gone within `metadata.max.idle.ms` (default 5 min) of the last commit. I'll spell this out in the "Producer-Side" section. Lucas: LB1: Correct. UNKNOWN_TOPIC_ID is returned if the topic id does not exist. Best, David On Thu, Apr 23, 2026 at 4:27 PM Chia-Ping Tsai <[email protected]> wrote: > > > chia1: We should keep in mind that this could eventually be renamed to > > just "memberEpoch," and every rename requires actual changes in the > > user code (as opposed to renaming RPCs), so this could factor into > > whether we need the intermediate "generationIdOrMemberEpoch". Not > > strictly against it though. > > Yes, renaming the method multiple times is definitely not ideal. My point was > that the public API is currently stale, and it would be more readable to show > the correct and modern name. Maybe we could introduce memberEpoch() and > deprecate generationId() with some helpful documentation. > > BTW, it could be a separate KIP > > Best, > Chia-Ping > > On 2026/04/23 13:34:12 Lucas Brutschy via dev wrote: > > Thanks David. This is a nice improvement and I am in favour. I don't > > think this is a controversial change, so my points are minor: > > > > chia1: We should keep in mind that this could eventually be renamed to > > just "memberEpoch," and every rename requires actual changes in the > > user code (as opposed to renaming RPCs), so this could factor into > > whether we need the intermediate "generationIdOrMemberEpoch". Not > > strictly against it though. > > > > LB1: Not strictly required, but consider specifying the error order. > > In particular, if a topic does not exist, does the RPC return > > UNKNOWN_TOPIC_ID or UNKNOWN_TOPIC_OR_PARTITION? Based on the > > OffsetCommit v10 handler, I'd expect the former, but making it > > explicit would be beneficial. > > > > Cheers, > > Lucas > > > > On Thu, Apr 23, 2026 at 1:46 PM Lianet Magrans <[email protected]> wrote: > > > > > > Thanks for the KIP David, nice to see this alignment! > > > > > > LM1: About topic ID resolution & adding the topics to the producer > > > metadata. The topics used for the sendOffsetsToTransaction will > > > commonly not be the ones found in the producer metadata (topics being > > > produced to), so agree with the waiting to resolve, otherwise seems we > > > wouldn't hit the topic ID path, but should we keep them around in > > > metadata, or treat them as "transient" topics (removed from metadata > > > after the call completes, similar to what we have in the consumer > > > path). I imagine the latter, mostly to avoid polluting the producer > > > metadata, but thoughts? > > > > > > Agree with chia1 btw (we already have confusing mapping for that > > > generation Id on the client side, to epoch in some places, to > > > generation in others) > > > > > > Best, > > > Lianet > > > > > > > > > On Wed, Apr 22, 2026 at 2:19 PM Chia-Ping Tsai <[email protected]> wrote: > > > > > > > > hi David > > > > > > > > the KIP is excellent. I have only one small question. > > > > > > > > chia1: Regarding the renaming of `GenerationId` to > > > > `GenerationIdOrMemberEpoch`: I'm wondering if we should also align the > > > > naming of `ConsumerGroupMetadata#generationId` to stay consistent? > > > > > > > > Best, > > > > Chia-Ping > > > > > > > > Andrew Schofield <[email protected]> 於 2026年4月22日週三 下午3:32寫道： > > > > > > > > > Hi David, > > > > > Thanks for the KIP. Just one query. > > > > > > > > > > AS1: Is there any reason why GenerationIdOrMemberEpoch is ignorable in > > > > > OffsetCommitRequest but not in TxnOffsetCommitRequest? > > > > > > > > > > Thanks, > > > > > Andrew > > > > > > > > > > On 2026/04/21 20:13:20 David Jacot wrote: > > > > > > Hi, > > > > > > > > > > > > I would like to start the discussion on KIP-1319. This is a minor > > > > > > KIP > > > > > > which brings the TxnOffsetCommit API in line with the OffsetCommit > > > > > > API. > > > > > > > > > > > > https://urldefense.com/v3/__https://cwiki.apache.org/confluence/x/NJI8G__;!!Ayb5sqE7!tu8P8SATZdib2PXwyaGOjmhzmuTByIKc4RunefP3j2stfdWC2eB00wOgGLtgJHiWdP1QNyofmPZdQbCUXr5BVQ$ > > > > > > > > > > > > Thanks, > > > > > > David > > > > > > > > > > > > >
