Skip to content

Comments

Fix dataloader dispatch in @defer when multiple deferred fragments exist#4270

Open
timward60 wants to merge 2 commits intographql-java:masterfrom
timward60:fix/defer-dataloader-dispatch-field-count
Open

Fix dataloader dispatch in @defer when multiple deferred fragments exist#4270
timward60 wants to merge 2 commits intographql-java:masterfrom
timward60:fix/defer-dataloader-dispatch-field-count

Conversation

@timward60
Copy link
Contributor

@timward60 timward60 commented Feb 24, 2026

Title

Fix dataloader dispatch in @defer when multiple deferred fragments exist (#4269)

Description

Problem

When a query contains multiple @defer fragments at the same level, dataloaders invoked inside those deferred fragments are never dispatched. The deferred payloads hang until the request times out.

This was introduced in #3980 ("make dataloader work inside defer blocks").

Root Cause

In DeferredExecutionSupportImpl.createDeferredFragmentCall(), the AlternativeCallContext is constructed with deferredFields.size() — the total field count across all @defer fragments — instead of the field count for the specific fragment:

// BUG: deferredFields.size() is the total across ALL @defer fragments
AlternativeCallContext alternativeCallContext = new AlternativeCallContext(level, deferredFields.size());

// But mergedFields is only the fields for THIS specific fragment
List<MergedField> mergedFields = deferredExecutionToFields.get(deferredExecution);

Each DeferredFragmentCall only invokes its own fields, so the fetch counter will only reach the count of fields in that fragment, never the inflated total. Both PerLevelDataLoaderDispatchStrategy.fieldFetched() and ExhaustedDataLoaderDispatchStrategy.deferFieldFetched() compare against this inflated count to decide when to dispatch, so dispatch never triggers.

Example: 2 @defer fragments with 1 field each → expectedFirstLevelFetchCount = 2, but each fragment only fetches 1 field → dispatch condition 1 == 2 is never satisfied → dataloaders hang.

Fix

Pass mergedFields.size() (the per-fragment field count) instead of deferredFields.size():

 private DeferredFragmentCall createDeferredFragmentCall(DeferredExecution deferredExecution) {
     int level = parameters.getPath().getLevel() + 1;
-    AlternativeCallContext alternativeCallContext = new AlternativeCallContext(level, deferredFields.size());
-
-    List<MergedField> mergedFields = deferredExecutionToFields.get(deferredExecution);
+    List<MergedField> mergedFields = deferredExecutionToFields.get(deferredExecution);
+    AlternativeCallContext alternativeCallContext = new AlternativeCallContext(level, mergedFields.size());

This is safe for mixed deferred/non-deferred selection sets because the constructor already partitions fields — non-deferred fields are routed into nonDeferredFieldNames and never added to deferredExecutionToFields.

Tests Added

Two new test cases in DeferWithDataLoaderTest:

  • multiple separate defer fragments with dataloader fields dispatch correctly — Two separate @defer blocks (departments and expensiveDepartments) on shops, each backed by dataloaders. Without the fix, all 3 dispatch strategy variants time out at ~10s each. With the fix, they complete in ~1s total.
  • mixed deferred and non-deferred fields with dataloaders dispatch correctly — Non-deferred departments alongside a single deferred expensiveDepartments, confirming the fix correctly handles mixed selection sets.

Both tests cover all three dispatch strategy modes: default (PerLevel), ENABLE_DATA_LOADER_CHAINING, and ENABLE_DATA_LOADER_EXHAUSTED_DISPATCHING.

timward60 and others added 2 commits February 17, 2026 09:53
Add two test cases to DeferWithDataLoaderTest:
- Multiple separate @defer fragments with dataloader fields
- Mixed deferred and non-deferred fields with dataloaders

Both tests verify dispatch works correctly across all three dispatch
strategy modes. Without the corresponding fix, the first test times out
because dataloaders are never dispatched.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Use mergedFields.size() (per-fragment field count) instead of
deferredFields.size() (total across all fragments) when constructing
AlternativeCallContext. The inflated count prevented dispatch strategies
from ever triggering dispatch since each fragment only fetches its own
fields.

Fixes the bug introduced in graphql-java#3980.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant