Skip to content

Conversation

@milindl
Copy link
Contributor

@milindl milindl commented Dec 27, 2024

Fixes #222

Thanks @omer-riv for filing this issue and providing a reproducing case!

Details of the issue:
In the cache, next() "gives up" a partition when it tries to fetch something, and returns empty handed. The message returned to the user in this case is also null.

In the cache, nextN "gives up" partition assignment not only when it returns empty handed, but when we consume to the end of the partition (either leftover size = N or with -1 to consume everything). This is problematic because, we're returning a not-null set of messages to the user but not retaining partition ownership for that user.

Solution: Make nextN like next.

Check the cache test for more details.

@milindl milindl requested a review from emasab December 27, 2024 06:54
@milindl milindl requested review from a team as code owners December 27, 2024 06:54
@confluent-cla-assistant
Copy link

🎉 All Contributor License Agreements have been signed. Ready to merge.
Please push an empty commit if you would like to re-run the checks to verify CLA status for all contributors.

Copy link
Contributor

@emasab emasab left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks Milind and Omer!

@milindl milindl merged commit 2f8d293 into master Jan 15, 2025
2 checks passed
@milindl milindl deleted the fix/eachBatch_concurrency_partition_restriction branch January 15, 2025 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

EachBatch called on the same partition at the same time

3 participants