control-plane: rebuild the plan when the scheduler is stuck reapplying a non-converging plan#6602
Conversation
aanufriev-celonis
force-pushed
the
escalate-reapply-to-rebuild
branch
from
5ad2764 to
b6bb7f0
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: b6bb7f04a3
ℹ️ 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".
| plans_diff=?indexing_plans_diff, | ||
| "reapplying the last plan is not converging: rebuilding the plan from the model" | ||
| ); | ||
| self.rebuild_plan(model); |
There was a problem hiding this comment.
In the stuck shard-placement case this still calls the normal rebuild_plan, which passes self.state.last_applied_physical_plan.as_ref() into build_physical_indexing_plan and the scheduler reuses previous sharded tasks before assigning missing shards. When the model correction is only that a shard is no longer located on the old indexer, the rebuilt plan can be identical to the non-runnable one, so the new threshold path does not actually break the reapply loop it is meant to recover from; this path needs a way to build from the current model without preserving the stale physical assignment.
Useful? React with 👍 / 👎.
aanufriev-celonis
force-pushed
the
escalate-reapply-to-rebuild
branch
from
d188b5a to
e89bbdc
Compare
|
I had a couple more ideas in mind on how to tackle this problem, but decided to go with the least intrusive one. Below are some thoughts about alternative paths:
|
|
@codex review |
|
Codex Review: Didn't find any major issues. Chef's kiss. Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
1 participant
Summary
Make the indexing scheduler recover on its own from a persistently non-converging
control_running_planloop, instead of re-sending the same un-runnable plan forever.Part of #6601.
Problem
See the original description of #6601.
Change
Add
REAPPLY_LOOP_RESCHEDULE_THRESHOLD(10 cycles, ~5 min at the 30s production interval), above the existingREAPPLY_LOOP_WARN_THRESHOLDso the non-convergence warning still fires first.Once
num_consecutive_reappliescrosses it, the loop callsrebuild_plan(model)instead of re-sending the stale plan. If the model has since been corrected, the rebuilt plan is runnable, and the cluster converges (installing a new plan resets the counter via the existing reset-on-new-plan logic).Rebuilds are throttled to once every
REAPPLY_LOOP_RESCHEDULE_THRESHOLDcycles to avoid rebuild storms when the model still describes an unreachable state.Testing
test_control_running_plan_rebuilds_after_persistent_reapplies: drives the reapply branch below the threshold (asserting the un-runnable task stays planned and the counter grows), then asserts the threshold cycle rebuilds from the model, drops the task, and resets the counter.