Skip to content

Commit 4f8eb7c

Browse files
andiclaude
authored and
andimarek
committed
Add deterministic CAS retry tests for ExhaustedDataLoaderDispatchStrategy
The coverage gate was flaking on dispatchImpl's CAS retry branches because they only execute under real thread contention, making coverage non-deterministic. This adds targeted tests that inject simulated CAS failures to deterministically exercise both retry paths (dispatch-setup and early-exit) without relying on thread scheduling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 1d5bb11 commit 4f8eb7c

File tree

2 files changed

+141
-2
lines changed

2 files changed

+141
-2
lines changed

‎src/main/java/graphql/execution/instrumentation/dataloader/ExhaustedDataLoaderDispatchStrategy.java‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
import graphql.execution.incremental.AlternativeCallContext;
1010
import org.dataloader.DataLoader;
1111
import org.dataloader.DataLoaderRegistry;
12+
import graphql.VisibleForTesting;
1213
import org.jspecify.annotations.NullMarked;
1314
import org.jspecify.annotations.Nullable;
1415

@@ -31,7 +32,8 @@ public class ExhaustedDataLoaderDispatchStrategy implements DataLoaderDispatchSt
3132
private final Map<AlternativeCallContext, CallStack> alternativeCallContextMap = new ConcurrentHashMap<>();
3233

3334

34-
private static class CallStack {
35+
// VisibleForTesting - package-private to allow test subclassing for CAS contention tests
36+
static class CallStack {
3537

3638
// 30 bits for objectRunningCount
3739
// 1 bit for dataLoaderToDispatch
@@ -127,7 +129,12 @@ public void clear() {
127129
}
128130

129131
public ExhaustedDataLoaderDispatchStrategy(ExecutionContext executionContext) {
130-
this.initialCallStack = new CallStack();
132+
this(executionContext, new CallStack());
133+
}
134+
135+
@VisibleForTesting
136+
ExhaustedDataLoaderDispatchStrategy(ExecutionContext executionContext, CallStack callStack) {
137+
this.initialCallStack = callStack;
131138
this.executionContext = executionContext;
132139

133140
this.profiler = executionContext.getProfiler();

‎src/test/groovy/graphql/execution/instrumentation/dataloader/ExhaustedDataLoaderDispatchStrategyTest.groovy‎

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -298,4 +298,136 @@ class ExhaustedDataLoaderDispatchStrategyTest extends Specification {
298298
// The deferred call stack dispatches independently
299299
batchLoaderInvocations.get() == 1
300300
}
301+
302+
/**
303+
* A CallStack subclass that forces CAS failures to deterministically exercise
304+
* the retry paths in dispatchImpl's CAS loop. Without this, CAS retries only
305+
* happen under real thread contention, making coverage non-deterministic.
306+
*
307+
* Failures are targeted: only CAS attempts matching a specific state transition
308+
* (identified by the newState pattern) are failed, so other CAS users like
309+
* incrementObjectRunningCount/decrementObjectRunningCount are not affected.
310+
*/
311+
static class ContendedCallStack extends ExhaustedDataLoaderDispatchStrategy.CallStack {
312+
// The newState value that should trigger a simulated CAS failure
313+
volatile int failOnNewState = -1
314+
final AtomicInteger casFailuresRemaining = new AtomicInteger(0)
315+
316+
@Override
317+
boolean tryUpdateState(int oldState, int newState) {
318+
if (newState == failOnNewState && casFailuresRemaining.getAndDecrement() > 0) {
319+
return false
320+
}
321+
return super.tryUpdateState(oldState, newState)
322+
}
323+
}
324+
325+
private void setupStrategyWithCallStack(BatchLoader<String, String> batchLoader, ExhaustedDataLoaderDispatchStrategy.CallStack callStack) {
326+
dataLoaderRegistry = new DataLoaderRegistry()
327+
def dataLoader = DataLoaderFactory.newDataLoader(batchLoader)
328+
dataLoaderRegistry.register("testLoader", dataLoader)
329+
330+
def executionInput = ExecutionInput.newExecutionInput()
331+
.query("{ dummy }")
332+
.build()
333+
def engineRunningState = new EngineRunningState(executionInput, Profiler.NO_OP)
334+
335+
def executionStrategy = new AsyncExecutionStrategy()
336+
executionContext = new ExecutionContextBuilder()
337+
.executionId(ExecutionId.generate())
338+
.graphQLSchema(GraphQLSchema.newSchema().query(
339+
graphql.schema.GraphQLObjectType.newObject()
340+
.name("Query")
341+
.field({ f -> f.name("dummy").type(GraphQLString) })
342+
.build()
343+
).build())
344+
.queryStrategy(executionStrategy)
345+
.mutationStrategy(executionStrategy)
346+
.subscriptionStrategy(executionStrategy)
347+
.graphQLContext(GraphQLContext.newContext().build())
348+
.coercedVariables(CoercedVariables.emptyVariables())
349+
.dataLoaderRegistry(dataLoaderRegistry)
350+
.executionInput(executionInput)
351+
.profiler(Profiler.NO_OP)
352+
.engineRunningState(engineRunningState)
353+
.build()
354+
355+
strategy = new ExhaustedDataLoaderDispatchStrategy(executionContext, callStack)
356+
357+
rootParams = newParameters()
358+
.executionStepInfo(newExecutionStepInfo()
359+
.type(GraphQLString)
360+
.path(ResultPath.rootPath())
361+
.build())
362+
.source(new Object())
363+
.fields(graphql.execution.MergedSelectionSet.newMergedSelectionSet().build())
364+
.nonNullFieldValidator(new NonNullableFieldValidator(executionContext))
365+
.build()
366+
}
367+
368+
def "CAS retry in dispatchImpl dispatch path is exercised under contention"() {
369+
given:
370+
def contendedCallStack = new ContendedCallStack()
371+
setupStrategyWithCallStack(simpleBatchLoader(), contendedCallStack)
372+
dataLoaderRegistry.getDataLoader("testLoader").load("key1")
373+
374+
when:
375+
strategy.executionStrategy(executionContext, rootParams, 1)
376+
strategy.newDataLoaderInvocation(null)
377+
// The dispatch-setup CAS in dispatchImpl sets currentlyDispatching=true and
378+
// dataLoaderToDispatch=false. With objectRunningCount=0, the target newState is:
379+
// currentlyDispatching(bit0)=1, dataLoaderToDispatch(bit1)=0, objectRunningCount(bits2+)=0
380+
// = 0b01 = 1
381+
contendedCallStack.failOnNewState = ExhaustedDataLoaderDispatchStrategy.CallStack.setCurrentlyDispatching(
382+
ExhaustedDataLoaderDispatchStrategy.CallStack.setDataLoaderToDispatch(0, false), true)
383+
contendedCallStack.casFailuresRemaining.set(1)
384+
strategy.finishedFetching(executionContext, rootParams)
385+
386+
Thread.sleep(200)
387+
388+
then:
389+
batchLoaderInvocations.get() == 1
390+
}
391+
392+
def "CAS retry in dispatchImpl early-exit path is exercised under contention"() {
393+
given:
394+
def doneLatch = new CountDownLatch(1)
395+
AtomicInteger roundCount = new AtomicInteger()
396+
def contendedCallStack = new ContendedCallStack()
397+
398+
def chainedBatchLoader = new BatchLoader<String, String>() {
399+
@Override
400+
CompletionStage<List<String>> load(List<String> keys) {
401+
int round = roundCount.incrementAndGet()
402+
if (round == 1) {
403+
// During first batch, load another key to trigger second dispatch round
404+
dataLoaderRegistry.getDataLoader("testLoader").load("key2")
405+
strategy.newDataLoaderInvocation(null)
406+
}
407+
if (round == 2) {
408+
// Inject a CAS failure targeting the early-exit path. After round 2
409+
// completes, the recursive dispatchImpl sees dataLoaderToDispatch=false
410+
// and tries to set currentlyDispatching=false. The target newState is 0
411+
// (all bits cleared: no dispatching, no dataLoader pending, objectRunning=0).
412+
contendedCallStack.failOnNewState = ExhaustedDataLoaderDispatchStrategy.CallStack.setCurrentlyDispatching(0, false)
413+
contendedCallStack.casFailuresRemaining.set(1)
414+
doneLatch.countDown()
415+
}
416+
return CompletableFuture.completedFuture(keys)
417+
}
418+
}
419+
setupStrategyWithCallStack(chainedBatchLoader, contendedCallStack)
420+
dataLoaderRegistry.getDataLoader("testLoader").load("key1")
421+
422+
when:
423+
strategy.executionStrategy(executionContext, rootParams, 1)
424+
strategy.newDataLoaderInvocation(null)
425+
strategy.finishedFetching(executionContext, rootParams)
426+
427+
def completed = doneLatch.await(2, TimeUnit.SECONDS)
428+
429+
then:
430+
completed
431+
roundCount.get() == 2
432+
}
301433
}

0 commit comments

Comments
 (0)