fix(control-plane): Avoid spawning a new control plan loop on every single actor restart. by fulmicoton-dd · Pull Request #6600 · quickwit-oss/quickwit

fulmicoton-dd · 2026-07-13T11:49:37Z

Description

Bugfix: before a failure of the control plane actor (due to connectivity with the metastore for instance) would trigger a restart of the control plane actor. A chitchat watching task is monitoring for node membership changes and triggering rebalance shards on this change. That task was spawned in initialized, on a recycled mailbox. As a result, every single restart would start a new watching loop.

This PR starts a loop once and for all.

Also, instead of looking for changes in the cluster membership, it looks for change in the ingester pool itself.

Each pool owns a monotonic generation sender that is updated after mutations are applied. The control plane subscribes once for the lifetime of its supervised mailbox, and the ingest controller tracks the last successfully rebalanced generation so bulk or duplicate notifications are coalesced. Periodic control-loop rebalances remain unconditional.

This also removes the obsolete cluster change stream factory and test stream abstractions from the control-plane API.

How was this PR tested?

cargo test -p quickwit-common tower::pool --lib
cargo test -p quickwit-control-plane --lib
cargo check -p quickwit-serve --lib
cargo +nightly fmt --all -- --check

fulmicoton-dd · 2026-07-13T11:50:47Z

does not seem to good to me though

Copilot

Pull request overview

This PR updates the control plane to react directly to IngesterPool mutations (via a monotonic pool generation) instead of deriving shard-rebalance triggers from cluster membership change streams, and removes the now-obsolete cluster change stream factory/test abstractions.

Changes:

Added a monotonic “generation” signal to Pool and used it to trigger control-plane rebalances.
Implemented debounced rebalance handling in the control plane by tracking the last successfully rebalanced ingester-pool generation.
Removed ClusterChangeStreamFactory and associated test helpers, updating call sites accordingly.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
quickwit/quickwit-serve/src/lib.rs	Updates `ControlPlane::spawn` call signature.
quickwit/quickwit-control-plane/src/tests.rs	Updates test helper to match new control-plane spawn signature.
quickwit/quickwit-control-plane/src/ingest/ingest_controller.rs	Tracks last rebalanced pool generation and exposes `rebalance_shards_inner` to support debouncing decisions.
quickwit/quickwit-control-plane/src/control_plane.rs	Switches from cluster-change watching to ingester-pool generation watching and debounced rebalance triggering.
quickwit/quickwit-common/src/tower/pool.rs	Adds pool generation watch channel and increments it on mutations.
quickwit/quickwit-cluster/src/lib.rs	Stops re-exporting removed cluster-change stream factory trait.
quickwit/quickwit-cluster/src/cluster.rs	Removes `ClusterChangeStreamFactory` impl for `Cluster`.
quickwit/quickwit-cluster/src/change.rs	Removes `ClusterChangeStreamFactory` trait and its test implementation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+        let rebalance_performed = match self
            .ingest_controller
-            .rebalance_shards(&mut self.model, ctx.mailbox(), ctx.progress())
+            .rebalance_shards_inner(&mut self.model, ctx.mailbox(), ctx.progress())
            .await
        {
-            return convert_metastore_error::<()>(error).map(|_| ());
+            Ok(num_opened_shards_opt) => num_opened_shards_opt.is_some(),
+            Err(error) => return convert_metastore_error::<()>(error).map(|_| ()),
        };
+        if !rebalance_performed {
+            return Ok(());
+        }


chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: fe1df52d91

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 4705d7bbfd

ℹ️ About Codex in GitHub

Codex has been enabled to automatically 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 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

chatgpt-codex-connector · 2026-07-13T17:14:27Z

+        if observed_pool_generation
+            <= self
+                .ingest_controller
+                .last_rebalanced_ingester_pool_generation
+        {
+            debug!(
+                pool_generation = observed_pool_generation,
+                "skipping rebalance: ingester pool is unchanged"
+            );
+            return Ok(());
+        }


Don't let periodic rebalances consume pool-change notifications

When a scheduled ControlPlanLoop runs after the ingester pool generation advances but before this queued IngesterPoolHasChanged message is handled, rebalance_shards_inner records that generation as last_rebalanced_ingester_pool_generation; this guard then returns before calling rebuild_plan. In that ordering, ready/retiring ingester changes can be rebalanced by the periodic loop while the indexing plan rebuild associated with the pool change is skipped, leaving assignments stale until control_running_plan later notices after its scheduling delay.

Useful? React with 👍 / 👎.

chatgpt-codex-connector · 2026-07-13T17:14:27Z

+            Ok(num_opened_shards_opt) => num_opened_shards_opt.is_some(),
+            Err(error) => return convert_metastore_error::<()>(error).map(|_| ()),
        };
+        if !rebalance_performed {
+            return Ok(());


Retry pool-change notifications when rebalance is busy

When an ingester joins or leaves while a previous rebalance is still holding the rebalance lock, including the delayed close phase, rebalance_shards_inner returns Ok(None) and this handler just drops the notification. Since the watch change for that generation has already been consumed, no follow-up IngesterPoolHasChanged is queued, so shards can remain on a retiring/departed ingester and the indexing-plan rebuild can be skipped until a later periodic control loop happens to acquire the lock.

Useful? React with 👍 / 👎.

guilload · 2026-07-14T17:59:21Z

 }

-/// A factory for creating cluster change streams.
-pub trait ClusterChangeStreamFactory: Clone + Send + 'static {


This deletion makes me happy.

guilload · 2026-07-14T17:59:54Z

 pub struct Pool<K, V> {
    pool: Arc<RwLock<HashMap<K, V>>>,
+    /// Publishes a notification after each mutation.
+    pub changes_tx: watch::Sender<()>,


Shouldn't this be private? Only the pool should sent notifications, no?

guilload · 2026-07-14T18:06:23Z

-    tokio::spawn(watcher_indexers(weak_mailbox, cluster_change_stream));
-}
-
-async fn watcher_indexers(


I'm glad we're getting rid of this complex function, however:

we no longer log why we rebalance

we rebalance on every pool change whereas before we would ignore some changes, for instance an ingester joining in initializing status. Rebalance shards is debounced but it's hard to predict whether this change will have undesirable side-effects.

fix(control-plane): avoid duplicate indexer watchers

30f075b

fulmicoton-dd force-pushed the paul.masurel/fix-control-plane-indexer-watcher branch from 7ccf3d7 to fba84d8 Compare July 13, 2026 14:10

fulmicoton-dd changed the title ~~fix(control-plane): avoid duplicate indexer watchers~~ fix(control-plane): debounce ingester pool rebalances Jul 13, 2026

fulmicoton-dd force-pushed the paul.masurel/fix-control-plane-indexer-watcher branch from fba84d8 to 2771635 Compare July 13, 2026 16:06

fulmicoton-dd requested a review from Copilot July 13, 2026 16:19

Copilot started reviewing on behalf of fulmicoton-dd July 13, 2026 16:19 View session

Copilot AI reviewed Jul 13, 2026

View reviewed changes

fulmicoton-dd force-pushed the paul.masurel/fix-control-plane-indexer-watcher branch 3 times, most recently from fb2ba67 to fe1df52 Compare July 13, 2026 16:36

fulmicoton-dd requested a review from fmassot July 13, 2026 16:36

fulmicoton-dd marked this pull request as ready for review July 13, 2026 16:36

fulmicoton-dd requested a review from a team as a code owner July 13, 2026 16:36

fulmicoton mentioned this pull request Jul 13, 2026

Reschedule control loop after metastore overload rejection #6595

Open

chatgpt-codex-connector Bot reviewed Jul 13, 2026

View reviewed changes

Comment thread quickwit/quickwit-common/src/tower/pool.rs Outdated

fulmicoton-dd force-pushed the paul.masurel/fix-control-plane-indexer-watcher branch from fe1df52 to 354f126 Compare July 13, 2026 17:01

fix(control-plane): debounce ingester pool rebalances

4705d7b

fulmicoton-dd force-pushed the paul.masurel/fix-control-plane-indexer-watcher branch from 354f126 to 4705d7b Compare July 13, 2026 17:02

chatgpt-codex-connector Bot reviewed Jul 13, 2026

View reviewed changes

fulmicoton-dd changed the title ~~fix(control-plane): debounce ingester pool rebalances~~ fix(control-plane): Avoid spawning a new control plan loop on every single actor restart. Jul 13, 2026

fulmicoton-dd force-pushed the paul.masurel/fix-control-plane-indexer-watcher branch from aa0676f to e101a83 Compare July 13, 2026 18:07

Removed generation from pool. Now it only advertises for its changes

3732ce1

fulmicoton-dd force-pushed the paul.masurel/fix-control-plane-indexer-watcher branch from e101a83 to 3732ce1 Compare July 13, 2026 18:11

guilload approved these changes Jul 14, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(control-plane): Avoid spawning a new control plan loop on every single actor restart.#6600

fix(control-plane): Avoid spawning a new control plan loop on every single actor restart.#6600
fulmicoton-dd wants to merge 3 commits into
mainfrom
paul.masurel/fix-control-plane-indexer-watcher

fulmicoton-dd commented Jul 13, 2026 •

edited

Loading

Uh oh!

fulmicoton-dd commented Jul 13, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Uh oh!

chatgpt-codex-connector Bot Jul 13, 2026

Uh oh!

chatgpt-codex-connector Bot Jul 13, 2026

Uh oh!

guilload Jul 14, 2026

Uh oh!

guilload Jul 14, 2026

Uh oh!

guilload Jul 14, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

fulmicoton-dd commented Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

How was this PR tested?

Uh oh!

fulmicoton-dd commented Jul 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

Uh oh!

chatgpt-codex-connector Bot left a comment

Choose a reason for hiding this comment

💡 Codex Review

Uh oh!

chatgpt-codex-connector Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

chatgpt-codex-connector Bot Jul 13, 2026

Choose a reason for hiding this comment

Uh oh!

guilload Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

guilload Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

guilload Jul 14, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

fulmicoton-dd commented Jul 13, 2026 •

edited

Loading