fix: ensure execution plan initialized after target setup (#49)#92
Merged
bashandbone merged 1 commit intomainfrom Mar 13, 2026
Merged
fix: ensure execution plan initialized after target setup (#49)#92bashandbone merged 1 commit intomainfrom
bashandbone merged 1 commit intomainfrom
Conversation
Adopt upstream bug fix from cocoindex-io/cocoindex#1715 (commit ba2fc4a). The bug allowed the execution plan to be initialized before target setup was complete in certain cases. This race could cause the planner to use outdated or incomplete state, leading to subtle bugs when resources are quickly provisioned or flows reconfigured. Changes: - Add Debug and Clone derives to ExportOpExecutionContext - Refactor TrackingTableSetupChange to store lazy execution plan - Pass execution_plan and export_op_execution_contexts to diff_flow_setup_states - Move tracking table setup to occur AFTER all target setup completes This ensures tracking table initialization only happens after all target contexts exist, preventing race conditions in flow setup. Co-authored-by: Adam Poulemanos <bashandbone@users.noreply.github.com>
bashandbone
added
bug
Contributor
|
🤖 Hi @bashandbone, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
Contributor
There was a problem hiding this comment.
Pull request overview
Adopts an upstream fix to prevent flow setup from initializing/using an execution plan before target setup is complete, by deferring tracking-table setup and threading plan/context data needed for lazy resolution.
Changes:
- Reorder flow setup application so tracking table setup runs after all target setup completes.
- Extend flow diffing to accept a shared/lazy execution plan and export-op execution contexts.
- Add
Debug/Clonederives and refactor tracking table setup change to carry the lazy plan/context.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/recoco-core/src/setup/driver.rs | Adds execution-plan/context params to flow diffing and applies tracking table setup after targets. |
| crates/recoco-core/src/lib_context.rs | Passes execution plan + export contexts into flow diffing during setup context build. |
| crates/recoco-core/src/execution/db_tracking_setup.rs | Refactors TrackingTableSetupChange to store a lazy execution plan and export contexts; custom Debug. |
| crates/recoco-core/src/builder/exec_ctx.rs | Derives Debug and Clone for ExportOpExecutionContext to enable cloning at call sites. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
352
to
363
| pub async fn diff_flow_setup_states( | ||
| desired_state: Option<&FlowSetupState<DesiredMode>>, | ||
| existing_state: Option<&FlowSetupState<ExistingMode>>, | ||
| flow_instance_ctx: &Arc<FlowInstanceContext>, | ||
| execution_plan: Shared< | ||
| BoxFuture< | ||
| 'static, | ||
| std::result::Result<Arc<plan::ExecutionPlan>, recoco_utils::error::SharedError>, | ||
| >, | ||
| >, | ||
| export_op_execution_contexts: Vec<exec_ctx::ExportOpExecutionContext>, | ||
| ) -> Result<FlowSetupChange> { |
This file contains hidden or 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
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.
Adopt upstream bug fix from cocoindex-io/cocoindex#1715 (commit ba2fc4a).
The bug allowed the execution plan to be initialized before target setup was complete in certain cases. This race could cause the planner to use outdated or incomplete state, leading to subtle bugs when resources are quickly provisioned or flows reconfigured.
Changes:
This ensures tracking table initialization only happens after all target contexts exist, preventing race conditions in flow setup.
Closes #49