Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to "range" assignor strategy from "cooperative-sticky" #705

Merged
merged 3 commits into from
Jan 15, 2025

Conversation

daniil-quix
Copy link
Collaborator

@daniil-quix daniil-quix commented Jan 14, 2025

Switched the partition assignment strategy to "range" from the previously used "cooperative-sticky".
With "cooperative-sticky", commits during rebalancing may fail, and we always commit the current checkpoint on assign and revoke.

According to this comment, range assignor now provides co-partitioning and ensures that the same partitions are assigned to the same members.

This property will help us to implement #484 too.

I briefly verified that it works this way, but I want to do more tests to be 100% sure.

Main changes

  • Use "range" assignment strategy
  • For recovery, subscribe to changelog topics too (previously subscribed only to input & repartition topics), and rely on assignment to be co-partitioned (same numbers to same consumers).
    • Added a check in case it's not co-partitioned
  • RecoveryManager now doesn't assign/unassign partitions and instead only pauses, seeks and resumes them

tim-quix
tim-quix previously approved these changes Jan 14, 2025
Copy link
Contributor

@tim-quix tim-quix left a comment

Choose a reason for hiding this comment

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

Overall surprisingly simple/minimal changes, nice!

Besides my comments here, I had a very small PR #710 to remove some other redundant stuff that was prob just missed while you cleaned stuff up.

quixstreams/state/recovery.py Show resolved Hide resolved
quixstreams/state/recovery.py Show resolved Hide resolved
@daniil-quix daniil-quix merged commit ec719d4 into main Jan 15, 2025
3 checks passed
@daniil-quix daniil-quix deleted the fix/switch-to-range-assignment branch January 15, 2025 09:46
quentin-quix pushed a commit that referenced this pull request Jan 15, 2025
Co-authored-by: Tim Sawicki <136370015+tim-quix@users.noreply.github.com>
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.

3 participants