Skip to content

Share one lifted committer per consumer in injectK - #297

Merged
alejandrohdezma merged 1 commit into
mainfrom
fix/lifted-committer-batching
Sep 29, 2026
Merged

alejandrohdezma merged 1 commit into
mainfrom
fix/lifted-committer-batching

Conversation

@alejandrohdezma

Copy link
Copy Markdown
Member

🚀 What's included in this PR?

Follow-up to #293. injectK lifted every record with CommittableConsumerRecord#mapK, and fs2-kafka's KafkaCommitter#mapK allocates a new committer on each call. CommittableOffsetBatch groups offsets by committer identity, so a batch of lifted records held one committer per record: commitBatchWithin (or CommittableOffsetBatch.fromFoldable) issued one parallel commitAsync per record instead of a single merged commit, with no ordering guarantee between them.

The lift now happens once per underlying committer. P.liftK is built once per stream, a mapAccumulate over the unwrapped WriterT keeps a map from each original committer to its lifted counterpart, and every record is rebuilt around the shared one. CommittableOffset.apply is package-private in fs2-kafka, so a small LiftedCommittableOffset object in package fs2.kafka performs the rebuild; it can go once fs2-kafka gives mapK structural equality.

TracedConsumerSpec runs three records over two partitions through injectK and checks the batch has one committer and commits once with the latest offset of each partition. It fails with three committers on the previous implementation.

`injectK` lifted every record with `CommittableConsumerRecord#mapK`, and
fs2-kafka's `KafkaCommitter#mapK` allocates a new committer on each call.
`CommittableOffsetBatch` groups offsets by committer identity, so a batch of
lifted records held one committer per record and `commitBatchWithin` issued one
parallel `commitAsync` per record instead of a single merged commit, with no
ordering guarantee between them.

The lift now happens once per underlying committer: `P.liftK` is built once
per stream, a `mapAccumulate` over the unwrapped `WriterT` keeps a map from
each original committer to its lifted counterpart, and every record is rebuilt
around the shared one. `CommittableOffset.apply` is package-private, so
`LiftedCommittableOffset` in package `fs2.kafka` performs that rebuild.

The new spec runs three records over two partitions through `injectK` and
checks the batch has one committer and commits once with the latest offset of
each partition; it fails with three on the previous implementation.
@github-actions github-actions Bot added 🪲 bug Bug fixes ⚖️ M Size of PR between 100 and 500 lines labels Sep 28, 2026
@alejandrohdezma
alejandrohdezma merged commit 89a7718 into main Sep 29, 2026
5 checks passed
@alejandrohdezma
alejandrohdezma deleted the fix/lifted-committer-batching branch September 29, 2026 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

⚖️ M Size of PR between 100 and 500 lines 🪲 bug Bug fixes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants