Repository navigation
[Fusion] Skip the fetch of excluded deferred plans that no nested plan reads - #162
Open
michaelstaib wants to merge 2 commits into
Open
michaelstaib wants to merge 2 commits into
michaelstaib wants to merge 2 commits into
Conversation
…n reads A deferred plan whose fields are all excluded by skip or include still ran its fetches when a nested group lifted a requirement into it, even when that nested group was itself excluded. The plan now completes without starting unless a started nested plan reads its result. No plan output or emitted plan JSON changes.
There was a problem hiding this comment.
🟡 Changes recommended
The enclosing-plan traversal can perform cubic work on every request, including cached plans, delaying the initial payload for deeply nested plan chains.
1 open issue
What changed in this PR
This PR avoids unnecessary Fusion subgraph fetches for excluded deferred plans while preserving fetches needed by nested plans. It builds on #160’s cached include-flag evaluation without changing planner or plan-format versions.
Changes:
- Completes excluded plans without execution when no started nested plan needs their results.
- Checks parent-scope requirements directly from execution nodes.
- Adds request-count and payload snapshots covering skipped groups and required enclosing fetches.
| File | Description |
|---|---|
src/HotChocolate/Fusion/test/Fusion.AspNetCore.Tests/DeferTests.cs |
Adds deferred-fetch regression tests and extracts a shared gateway fixture. |
src/HotChocolate/Fusion/src/Fusion.Execution/Execution/OperationPlanExecutor.cs |
Identifies skippable plans and completes them without executing nodes. |
Review effort: Balanced
Comment on lines
+1040
to
+1042
| foreach (var enclosingPlan in operationPlan.IncrementalPlans) | ||
| { | ||
| if (!deliveryGroupsByPlan[enclosingPlan].DeliveryGroups.Contains(enclosing)) |
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.
A deferred plan whose fields are all excluded by
@skipor@includestill sent its subgraph fetch when a nested deferred group lifted a requirement into it, even when that nested group was excluded too. The gateway now completes such a plan without starting it, unless a started nested plan reads its result.Changes:
FindSkippedIncrementalPlanspicks the excluded plans whose fetch no started nested plan depends on. The executor completes them without running their nodes.No
OperationPlanner.Versionbump, no plan JSON change.Stacked on #160.
Tests: new
DeferTestsin Fusion.AspNetCore.Tests cover a nested group that needs the outer data, enclosing silent groups, and plans that are skipped. The payload streams are snapshotted. The nested test fails without the fix. Fusion.Execution.Tests andDeferTestson net10.0 and net8.0 are green, except one NATS test that needs Docker.No benchmark was run.