Repository navigation
Refactor out legacy parts of ingest controller - #6895
nadav-govari wants to merge 1 commit into
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
https://github.com/quickwit-oss/quickwit/blob/46422eb7eb18925e8343dd1fb7c24caa121dc81f/quickwit-control-plane/src/ingest/legacy_scaling_controller.rs#L34
This newly added file is 1,045 lines, exceeding the repository's mandatory 500-line limit by more than double. Split the controller implementation and its large test module by responsibility so the new source files comply with the repository requirement.
AGENTS.md reference: AGENTS.md:L136-L136
https://github.com/quickwit-oss/quickwit/blob/46422eb7eb18925e8343dd1fb7c24caa121dc81f/quickwit-ingest/src/ingest_v2/broadcast/local_shards.rs#L107-L108
When a shard's measured throughput fluctuates within the same rounded-up MiB/s bucket, these exact ByteSize values make LocalShardsSnapshot::diff report a change even though Serialize produces the same gossip value. In mixed-version clusters where this legacy broadcaster remains active, that causes every such source to call set_self_key_value again on each broadcast interval, creating unnecessary Chitchat state/version updates and control-plane notification churn; retain the rounded values in the snapshot or compare the serialized representation instead.
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
This branch has not been deployed
1 participant
Description
Pulls out the legacy parts related to scaling up/down and handling local shards from the ingest controller and puts them into a LegacyScalingController. The ScalingArbiter also gets renamed. This is ahead of introducing the new logic.
This is a straight refactor. The logic simply moves. IngestController becomes Arc, as it's shared between 3 components (control plane, legacy scaling controller, new scaling controller). To accommodate this, a few methods that were previously
mutno longer are (they didn't need to be in the first place). The stats struct on the ingest controller goes away and becomes atomic instead so it remains Send.The
Fromimplementation for RateMibPerSec from ByteSize ceils to Mib, so the readings are still consistent. ShardStats are moved to ByteSize ahead of the new logic.How was this PR tested?
Unit tests.