Repository navigation
IndexerStateReporter task - #6891
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. |
00a405f to
593637a
Compare
![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
Here are some automated review suggestions for this pull request.
Reviewed commit: 00a405f8a4
ℹ️ 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".
| // TODO: implement me | ||
| reply(Ok(ReportIndexerStateResponse {})); |
There was a problem hiding this comment.
Once every indexer enables shard-scaling v2, the new code stops publishing both running tasks and local shard readings through gossip, making this RPC the only update path. This handler discards the entire request and returns success, so the indexer pool retains stale task state and shard throughput never reaches the model or scaling arbiter, disabling correct plan reconciliation and shard scaling for the migrated cluster. Implement both updates, including node/generation validation, before acknowledging the report.
AGENTS.md reference: AGENTS.md:L19-L22
Useful? React with 👍 / 👎.
| /// retry, and we have a short timeout; we don't need either, as the next iteration of the loop | ||
| /// will take place soon anyway. | ||
| async fn send_report(&self, request: ReportIndexerStateRequest) { | ||
| let report_future = self.control_plane_client.report_indexer_state(request); |
There was a problem hiding this comment.
In a cluster with multiple control-plane nodes, this call reaches only one of them: an indexer-only node uses the BalanceChannel built in start_control_plane_if_needed, while an indexer co-located with a control plane always uses its local mailbox. Each control plane owns an independent model, indexer pool, and reconciliation loop, so after the new code disables gossip, control planes that did not receive a given report retain stale shard throughput and running-task state. Broadcast the snapshot to all active control planes or move this state into a shared/single-owner path.
Useful? React with 👍 / 👎.
| // Performs a debounced shard pruning request to the metastore. | ||
| rpc PruneShards(quickwit.metastore.PruneShardsRequest) returns (quickwit.metastore.EmptyResponse); | ||
|
|
||
| rpc ReportIndexerState(ReportIndexerStateRequest) returns (ReportIndexerStateResponse); |
There was a problem hiding this comment.
This adds a new inter-service RPC and changes the cluster-wide source of indexer task and shard-throughput state from gossip to gRPC, including a compatibility-sensitive cutover, but the commit does not update any protocol or architecture documentation. Document the request semantics, generation handling, rollout prerequisites, and cutover behavior so operators and future implementations do not violate the migration contract.
AGENTS.md reference: AGENTS.md:L23-L24
Useful? React with 👍 / 👎.
This branch has not been deployed
1 participant
Description
This is the task that communicates the indexer state - shard throughput readings, and indexing tasks - via gRPC.
Both old gossip tasks continue running until all indexers send enable_shard_scaling_v2. Then, there's a cutover to this task and reporting starts all at once. The old gossip updates then stop.
Both the old gossip updates use the new watch channels used for the gRPC so that the data source remains the same.
How was this PR tested?
Unit tests.