Can't use dataloader twice within a single field

The data loader instrumentation only allows one data loader to resolve per field. Complex queries may require more than a single data loader to resolve.

Would it be better to change the dispatcher instrumentation to something like the following:
when dataloader.load(key) is called - if (numberOfIdleFetches++ == numberOfExpectedFetches) dispatchAll();
when load is resolved: numberOfIdleFetches--

Can you please give a more concrete example of the type of fetchers you are doing that require multiple data loaders.

Psudeo java code is ok

The data loader instrumentation code watches fields fetches not data loader calls. So off hand I would expect that given fieldX if its data fetcher called 2 data loaders it would not care that 2 calls are made

Or are you chaining them?? where the results of dataloader A feeds into a new dataloader B?

Hi,

Thanks for your reply. A common (?) use-case would be when the graphql schema does not match the database schema. A single object can require data from multiple tables to be fetched.

One example is a many-to-many relationship. A "USER" reads many "BOOKS". A "BOOK" has many "READERS". Right now, if we want to use dataloaders, the results of one dataloader must feed into another dataloader. We must include the intermediary associative table "USERS_BOOKS" as a GraphQLObjectType, which may not be desirable.

ideally we can do this (pardon my kotlin):
val userBooks = userBooksLoader.load(x).await() val books = userBooks.map { booksLoader.load(it.books).await() }

Another example is a relational database where one table represents a "main" object and there are many other tables which are subclasses of that table, there is a table for "PERSON" that has name and id, but stores information on other tables like "USER_WITH_OCCUPATION" and "USER_WITH_HEALTHCARE_INFORMATION". Sometimes we don't want to expose the subclasses as fragments.
val user = usersLoader.load(x).await() val userWithOccupation = userWithOccupationLoader.load(user.occupation).await() val userWithAllTheInformation = new UserWithAllTheInformation(user, userWithOccupation, ...)

And a third example is as you mentioned if a dataloader needs to chain from the results of another.

I think we just need to wrap the load function:

in DataLoader:
suspend fun asyncLoad(id: K, callback: {with onAwait() and onComplete() callbacks}) { //wrap the original CompletableFuture with a CompletableDeferred (if kotlin) / CompletableFuture (if java) that has the aforementioned callbacks. }

class WrappedCompletable(original: CompletableDeferred, callbacks) { suspend fun await() { onAwait() return original.await().also { onCompleted() } } }
Then pass the callbacks to the instrumentation to count the number of delayed fields.

A side benefit is that execution can be performed asynchronously instead of level by level.

Thanks for outlining more in detail.

One of the challenges we have today in Java is knowing when to dispatch the data loaders. Today we track fields and levels and dispatch once we have exhausted all fields.

However with chained data loaders, we will kick it off but we will not know when to dispatch again since we don't see the chaining in place

This is not a problem in JavaScript because they have a tick loop and it fires when node.js has not more work to do. Hence they hooked data loader into it

You could manually call dataLoaderRegistry.dispatch() inside chained calls but the it would be TOO eager I think.

Thank you for your reply.

I'm using graphql-java on a Kotlin project. Turns out that it's not too difficult in Kotlin to implement a node.js-style nextTick() function to queue dispatches. Kotlin has its own async framework.

Then I just have to re-write AsyncExecutionStrategy and DataLoader to use Kotlin's async framework rather than the Java standard CompletableFuture framework. The resulting dataloader would function just as intuitively as the node.js version.

I guess my solution is more relevant for people who use Kotlin. Not sure how many of us are here.

I saw that you reopened this issue and thought to chime in.

If the desired eventual outcome is to implement the node.js functionality, I would suggest a centralised dispatcher. It makes sense to make DataLoaderDispatchInstrumentation a first-class entity rather than an optional one.

Deprecate using CompletableFuture static methods like CompletableFuture.supplyAsync() in DataFetchers.
Move the above methods into DataFetchingEnvironment. Eg. DataFetchingEnvironment.supplyAsync().
The new methods spawn an extension of CompletableFuture that holds a reference to the Instrumentation. methods such as whenComplete() would also notify the centralised Instrumentation.
The ExecutionStrategy would also have to use the centralised dispatcher to execute asynchronous operations.
The DataLoaderDispatchInstrumentation instance is now able to monitor all asynchronous operations and implement process.nextTick().

I'm using a similar solution but with Kotlin's coroutine framework. I'm not very familiar with Java so please forgive me for mistakes.

Hey all,

Is there any solution in Java (graphql) for chained DataLoaders ?
Anyone done it and can share (don't want to reinvent the wheel) ?

Thanks Guy.

You can chain data loaders BUT you cant do it with ULTIMATE efficiency

graphql-java tracks fields and hence how many dataLoader.dispatch() calls it needs to make

if it does it too much then it will not be as efficient as it could be - if it does it too little the threads will block on promise that will never finished

You could do this

DataFetcher df  = env -> {
        CompletableFuture cf = firstDataLoader.load(env.getArgument("id")):
       cf.thenApply( resultFromFirst -> secondDataLoader.load(resultFromFirst.otherId();
           // important step next
          secondDataLoader.dispatch();
));

The graphql field tracking will cause the first data loader to dispatch() while the second needs manual calling since graphql did not know that a chain CF was in play

This might mean you dispatch some other fields too early and hence less optimally

Its not optimal but it is possible.

I have exactly the same problem: I'm fetching a set of objects, and then need to check permissions for each of these objects via a separate call to another service. I can't do that efficiently with the current data loaders. As you said, I could hack something together that would kind of work, but it isn't nearly as efficient, adds quite a bit of complexity and requires a deeper understanding of how queries are resolved in GraphQL, and how graphql-java orchestrates the loaders. I don't feel comfortable going down that route given that many different teams at my company will be contributing to the GraphQL schema, and I can't expect everyone to go through the trouble of wiring up data fetchers and dispatching them manually. Ideally, whether or not batching is done should only need to be known at the resolver (data fetcher) level. If batching is desired, implement a batched data fetcher. If batching is not desired, implement a normal data fetcher.

So, how hard would it be to implement something along the lines of Sangria's DeferredResolver, i.e. DeferredDataFetcher? I think that's a much better abstraction than having to add a second concept of loaders, having to construct them for every query, passing them through the context and then calling them in the resolver.

Deferred data fetchers could have a method called getBatch, whose signature could be just like get of a normal data fetcher, except that it would take a list of DataFetchingEnvironment. To provide extra convenience for the common use-case where only the source is needed (eg. fetching a list of related objects by their id), a function could be provided to extract the sources from the BatchDataFetchingEnvironment.

If the resolver is inherently batched, all the problems of figuring out when to dispatch and whether to dispatch manually go away, because inside the resolver everything is batched, so firing off a second batch async request is as simple as chaining the promise.

I'm not familiar with the internals of graphql-java's execution engine, but I assume that since it's already aware of levels there is a place in the code where the executor is aware of all the fields at a given level, and could batch together all calls to a given resolver (aka data fetcher) if the field has a DeferredDataFetcher.

@Togrias

Turns out that it's not too difficult in Kotlin to implement a node.js-style nextTick() function to queue dispatches. Then I just have to re-write AsyncExecutionStrategy and DataLoader to use Kotlin's async framework rather than the Java standard CompletableFuture framework. The resulting dataloader would function just as intuitively as the node.js version.

@Togrias - Did this end up working in Kotlin? And if so would you mind sharing how you did it?

@stevenheidel

@stevenheidel
I have a node.js-ish implementation written in Kotlin. I assume you are familiar with the functionality of Kotlin coroutines, including the CoroutineDispatcher and CoroutineContext, as well as the original node.js reference implementation of DataLoaders (including process.nextTick()). Here are roughly my steps from memory:

General idea: Dataloaders delay execution until their dispatch() method is called. The GraphQL query execution strategy executes datafetchers concurrently as it traverses the query graph. Whenever it calls a dataloader.load() method, it suspends indefinitely (gets stuck). We want to call dispatch() only when all current coroutines are suspended.

Create a new CoroutineDispatcher wrapping the CommonPool (the default coroutine dispatcher - or whatever dispatcher your app is using) I call it the QueueableDispatcher. It has an atomic integer counter and a mutable list of callbacks.
1a) Override the QueueableDispatcher's "execute" method to the following:
i) increment the atomic counter, then;
ii) call the "execute" method of the wrapped inner dispatcher (CommonPool), then;
ii) decrement the atomic counter, then if it's zero, execute and remove the first callback on the list.
1b) A nextTick() method takes a callback and adds it to the aforementioned mutable list. You can optionally return a Deferred from it, but it's not necessary for the implementation.

Explanation: The atomic counter tracks the number of suspendable coroutines that are currently being executed and not suspended. When it reaches zero, we know it's time to fire the DataLoader dispatch() function (i.e. nextTick()).

Create new KDataLoader class and KBatchLoader interface that has uses suspend functions rather than futures. For instance, the Java BatchLoader is List -> Future<List>, we should use suspend List -> List instead.
2a) Now that we have the nextTick() method as above, we can implement the Facebook DataLoader reference implementation. You can access the QueueableDispatcher in suspending functions using either CoroutineContext or the GraphQL Context.
2b) Make sure that your KDataLoader's load(key) and dispatch() methods are suspending functions and have QueueableDispatcher inside their CoroutineContext.
Rewrite portions of the graphql.executions package: ExecutionStrategy, AsyncExecutionStrategy, AsyncSerialExecutionStrategy (or whatever it's called - the one that GraphQL.java uses for mutations), and the Async object. For each method that produces a future, rewrite it such that the Future created uses the Kotlinx coroutines future() api. Pass QueueableDispatcher into the context. Also pay attention to .whenComplete and .thenXX (thenAccept, etc) calls. Replace them all.

I use kotlin extension functions so they're easier to replace and the code reads similarly to the graphql.java implementation so I can maintain them more easily.

The goal is to force graphql.java to use the CoroutineDispatcher to create all its Futures.

I find that Dataloaders are quite difficult to test. But I'd be more worried that they get stuck indefinitely (overbatch), and not that they fetch too quickly (underbatch). So far, the above coroutines-based approach hasn't caused me any problems on the former.

It's not only dataloader which is impossible to use twice: you also can't use single dataloader which depends on any async result:

        return CompletableFuture.supplyAsync {
            Thread.sleep(100L)
            123 // simulate API call which returns some key
        }.thenCompose { key ->
            dataLoader.load(key)
        }

Things will work only if you call dataLoader.load() exactly when preparing CompletableFuture returned from DataFetcher. If dataLoader.load() is called after DataFetcher is done, then it will be never dispatched. Yes, I can move async API call to a new dataLoader, but this will greatly limit the batching ability in case of such schema:

type Query {
    field1: Type
    field2: Type
    // ....
    fieldN: Type     
}

In this case I would have N different dataloaders for one single type, and they wouldn't batch together. This is quite significant limitation.

@bbakerman

@bbakerman Ironically, while there's all this business going on with Futures and AsyncExecutionStrategy to make it work with a modern async concurrency system, we have to resort to good old sync code with threads when fetching cases get a little bit more complicated than most simple ones.

@Togrias

@Togrias - thanks for the help so far, really appreciate the detailed explanation of your Kotlin implementation.

Here's what I've got so far with the dispatcher:

import kotlinx.coroutines.*
import java.util.*
import java.util.concurrent.atomic.AtomicInteger
import kotlin.coroutines.CoroutineContext

class QueueableDispatcher(
    private val wrappedDispatcher: CoroutineDispatcher
) : CoroutineDispatcher() {
    private val counter: AtomicInteger = AtomicInteger(0)
    private val queue: Queue<Runnable> = LinkedList()

    override fun dispatch(context: CoroutineContext, block: Runnable) {
        counter.incrementAndGet()
        val wrappedBlock = Runnable {
            block.run()

            if (counter.decrementAndGet() == 0) {
                queue.poll()?.run()
            }
        }

        wrappedDispatcher.dispatch(context, wrappedBlock)
    }

    fun nextTick(block: suspend () -> Unit) {
        queue.add(Runnable { block() }) // *****
    }
}

Unfortunately now I'm stuck since it doesn't make sense to convert a suspend fun into a Runnable for use in the queue. If the nextTick function makes an async call as in the reference implementation it's not clear what to do next.

@lennyburdette

Unsure if this is useful here, but while working at Carbon Health I amended @lennyburdette's debounce strategy for use in DGS, which is built on graphql-java.

@muratakbal was nice enough to give me permission to extract and publish it. I have done that here. The debounce code specifically is in this file, with tests for all the batching cases here.

We used these in prod without seeing any issues, though at relatively small scale.

Hello, this issue has been inactive for 60 days, so we're marking it as stale. If you would like to continue this discussion, please comment within the next 30 days or we'll close the issue.

@samuelAndalon

I think this is probably resolved by the enableDataLoaderChaining and enableDataLoaderExhaustedDispatching flags added in GraphQL-Java v25 (though I haven't tried them myself yet), though you do still have to explicitly enable one of them. (The latter is based on @samuelAndalon's solution above.)

@glasser

@glasser - yes this is now our best mechanism. @andimarek spent a lot of time on the new "exhausted dispatching strategy" in graphql-java and he his happy with the outcome.

We have vague plans to bring this back into java-dataloader itself should there be people using dataloader outside a graphql-java context (and there are some) but at this stage its slowly happening.

@bbakerman

@bbakerman @andimarek That's pretty exciting and I'd love to do this. However, I'm finding that when I switch over from my janky thing above to enableDataLoaderExhaustedDispatching, my dataloaders don't get called (and yes, I am making sure the registry is in the Execution Input).

What caveats still exist with the new strategy? eg, does the Async Calls On Your Batch Loader Function Only caveat still apply here or should you be able to not worry about this any more with this enabled?

@bbakerman

@bbakerman @andimarek That's pretty exciting and I'd love to do this. However, I'm finding that when I switch over from my janky thing above to enableDataLoaderExhaustedDispatching, my dataloaders don't get called (and yes, I am making sure the registry is in the Execution Input).

What caveats still exist with the new strategy? eg, does the Async Calls On Your Batch Loader Function Only caveat still apply here or should you be able to not worry about this any more with this enabled?

The restriction doesn't apply anymore with the exhausted strategy.

Two reason I can think of why it doesn't work for you:
You need to access all data loader via the DataFetchingEnvironment.getDataLoader method.

You can't block any thread that is used by the engine to execute.

If it is still failing we would need a more concrete example to look into more.

@andimarek

@andimarek Aha, I am calling getDataLoader directly on my DataLoaderRegistry. That will probably fix it! Definitely something to add when the docs get updated :)

Yep, that did the trick.

hey everybody ... I am closing this now because the new Exhausted DataLoader Strategy makes sure everything is dispatched.

Please try it in 25 or the 26.0.beta-2 and open a new issue if you find any problem.

@andimarek

@andimarek I do think it's a bit surprising that this isn't the default (or more strongly documented), FWIW — though I understand slow-rolling out a complex change, do we expect this to eventually be the default (or at least recommended on the batching page)?

@glasser

@glasser it will very likely not be ever the default as this would break to many existing users.

But we should document this much more clearer and we will probably recommend this as default.

Navigation Menu

Repository navigation

Can't use dataloader twice within a single field #1198

22 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Can't use dataloader twice within a single field #1198

Description

Activity

bbakerman commented on Sep 3, 2018

Togrias commented on Sep 3, 2018

bbakerman commented on Sep 7, 2018

Togrias commented on Sep 8, 2018

Togrias commented on Oct 5, 2018

guy-klebanov commented on Dec 17, 2018

bbakerman commented on Mar 15, 2019

helfer commented on Mar 17, 2019

stevenheidel commented on Dec 1, 2019

Togrias commented on Dec 1, 2019

haizz commented on Dec 4, 2019

haizz commented on Dec 4, 2019

stevenheidel commented on Dec 5, 2019

22 remaining items

fluff-shark commented on Jan 7, 2024

github-actions commented on May 5, 2024

glasser commented on Feb 3, 2026

bbakerman commented on Feb 7, 2026

glasser commented on Feb 11, 2026

andimarek commented on Feb 11, 2026

glasser commented on Feb 11, 2026

glasser commented on Feb 11, 2026

andimarek commented on Feb 23, 2026

glasser commented on Feb 23, 2026

andimarek commented on Feb 23, 2026

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions