feat(consumers): rust consumers quantized rebalance #6561
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
john-z-yang
reviewed
Nov 14, 2024
john-z-yang
reviewed
Nov 14, 2024
Comment on lines
+298
to
+299
| rebalancing::delay_kafka_rebalance(secs); | ||
| handle.signal_shutdown(); |
There was a problem hiding this comment.
Am I correct in assuming that this closure that is passed in runs in a separate thread, so the main loop is not blocked while it is sleeping before signaling shutdown?
There was a problem hiding this comment.
From the documentation it seems to be the case
https://docs.rs/ctrlc/latest/ctrlc/fn.set_handler.html
Register signal handler for Ctrl-C.
Starts a new dedicated signal handling thread. Should only be called once, typically at the start of your program.
|
This seems reasonable to me, do we plan to have quantized rebalancing on the other end? I.e. Making the consumer wait a little bit before joining the consumer group |
|
@john-z-yang it does wait before joining, see (and I tested that this happens locally as well) |
john-z-yang
approved these changes
Nov 14, 2024
|
PR reverted: 4f8cfa9 |
getsentry-bot
added a commit
that referenced
this pull request
Nov 15, 2024
This reverts commit f5116c1. Co-authored-by: volokluev <[email protected]>
untitaker
pushed a commit
that referenced
this pull request
Nov 22, 2024
Add the ability to do quantized rebalancing for consumers, toggleable by
runtime config.
### What is quantized rebalancing?
Rebalancing events are synchronized to the tick of the clock. Controlled
by the runtime config:
`quantized_rebalance_consumer_group_delay_secs__{consumer_group}`
Let's say:
`SET quantized_rebalance_consumer_group_delay_secs__spans=15`
* When a consumer starts up: delay subscribing to a topic until
`timestamp % 15 == 0`
* when a consumer gets a shutdown signal (ctrl-c): delay leaving the
group until `timestamp % 15 == 0`
### What else is in this PR?
This PR adds the ability to read from redis config directly. This was
mostly done because I couldn't get the python bindings to work on my
machine, it will still fall back to the old implementation if there is
an error in the direct implementation
untitaker
added a commit
that referenced
this pull request
Nov 25, 2024
reapply #6561 but remove the direct redis call. thanks to volo for writing basically all of the code (with tests!) personal opinion: redis is not a great API boundary to have (there's very few things in redis that help you abstract over the raw datastructure), and using it directly from two codebases would be like accessing the same postgres from multiple microservices. it seems to work fine on localhost. for future reference, I did: ``` make watch-rust-snuba # then make my changes # then go to snuba admin and set the setting for consumer group lol4 RUST_LOG=info snuba rust-consumer --log-level info --storage eap_spans --consumer-group lol4 --concurrency 2 --use-rust-processor --max-batch-size 1000000 --max-batch-time-ms 100 --auto-offset-reset latest --no-strict-offset-reset ``` watch the log output say `Delaying rebalance by 4 seconds` --------- Co-authored-by: volokluev <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add the ability to do quantized rebalancing for consumers, toggleable by runtime config.
What is quantized rebalancing?
Rebalancing events are synchronized to the tick of the clock. Controlled by the runtime config:
quantized_rebalance_consumer_group_delay_secs__{consumer_group}Let's say:
SET quantized_rebalance_consumer_group_delay_secs__spans=15timestamp % 15 == 0timestamp % 15 == 0What else is in this PR?
This PR adds the ability to read from redis config directly. This was mostly done because I couldn't get the python bindings to work on my machine, it will still fall back to the old implementation if there is an error in the direct implementation