spring-graphql icon indicating copy to clipboard operation
spring-graphql copied to clipboard

Cascade of DataLoader's in a controller

Open andrey-nakin opened this issue 3 years ago • 6 comments

Hello! In my GraphQL controller I need to call two DataLoaders when the second DataLoader uses the result of the first one.

Here's a simplified example:

@SchemaMapping
public CompletionStage<Boolean> fieldName(
           String param,
           DataLoader<String, Integer> intLoader,
           DataLoader<Integer, Boolean> boolLoader) {
  return intLoader.load(param)
    .thenCompose(
      intVal -> boolLoader.load(intVal)
    );
}

The problem is that the boolLoader.load may never return and the entire operation fails with a timeout error.

Is there a proper way to call data loaders in a cascade?

andrey-nakin avatar Jun 24 '22 12:06 andrey-nakin

Your example works for me. What's the code for your DataLoaders and how did you register them?

@Controller
public class MyController {
  public MyController(BatchLoaderRegistry reg) {
    reg.forTypePair(String.class, Integer.class).registerMappedBatchLoader((values, env) ->
      Mono.just(values.stream().collect(Collectors.toMap(Function.identity(), (String value) > Integer.valueOf(value)))));
    reg.forTypePair(Integer.class, Boolean.class).registerMappedBatchLoader((values, env) ->
      Mono.just(values.stream().collect(Collectors.toMap(Function.identity(), (Integer value) -> value % 2 == 0))));
  }

  @SchemaMapping
  CompletionStage<Boolean> fieldName(DataLoader<String, Integer> dl1, DataLoader<Integer, Boolean> dl2) {
    return dl1.load("1").thenCompose(i -> dl2.load(i));
  }
}

benneq avatar Jun 24 '22 18:06 benneq

@benneq Looks like it stops working with a real multitheading environment. The following example does not work for me:

@Controller
public class DemoController {

    public DemoController(BatchLoaderRegistry reg) {
        reg.forTypePair(String.class, Integer.class)
                .registerMappedBatchLoader((values, env) -> Mono.fromCompletionStage(loadInt(values, env)));
        reg.forTypePair(Integer.class, Boolean.class)
                .registerMappedBatchLoader((values, env) -> Mono.fromCompletionStage(loadBool(values, env)));
    }

    @SchemaMapping
    public CompletionStage<Boolean> myField(MyObject myObject, DataLoader<String, Integer> dl1,
            DataLoader<Integer, Boolean> dl2) {

        return dl1.load(myObject.getInput()).thenCompose(i -> dl2.load(i));
    }

    private CompletionStage<Map<String, Integer>> loadInt(Set<String> keys, final BatchLoaderEnvironment env) {
        return CompletableFuture.supplyAsync(() -> keys.stream()
                .collect(Collectors.toMap(Function.identity(), (String value) -> Integer.valueOf(value))));
    }

    private CompletionStage<Map<Integer, Boolean>> loadBool(Set<Integer> keys, final BatchLoaderEnvironment env) {
        return CompletableFuture.supplyAsync(
                () -> keys.stream().collect(Collectors.toMap(Function.identity(), (Integer value) -> value % 2 == 0)));
    }

    // skipped
}

Here the query returns HTTP error 503 and the following error is found in the logs:

2022-06-27 19:45:38.436  WARN 31203 --- [nio-8080-exec-4] .w.s.m.s.DefaultHandlerExceptionResolver : Resolved [org.springframework.web.context.request.async.AsyncRequestTimeoutException]

The example about starts working if one replaces CompletableFuture.supplyAsync with CompletableFuture.completedFuture in the data loaders.

The whole project demonstrating the problem is here: https://github.com/andrey-nakin/spring-issue-425

andrey-nakin avatar Jun 27 '22 16:06 andrey-nakin

This is probably related to the fact that DataLoader invocation is deferred in order to get the benefits of batching. It's not very clear what the actual scenario is, or if DataLoader's are intended to be used in this way. Could you provide a sample schema that represents the actual use case vs just booleans and ints?

rstoyanchev avatar Jun 30 '22 08:06 rstoyanchev

@rstoyanchev There's nothing special in my GraphQL schema. The simplified examples above illustrate the issue perfectly.

I believe the problem might be in GraphQL Java library, I found a very similar issue: https://githubhot.com/repo/graphql-java/java-dataloader/issues/54

Looks like the second-level DataLoader is never dispatched. Question: can I dispatch is manually somehow? Can DataLoader.dispatchDepth be helpful?

andrey-nakin avatar Jul 01 '22 16:07 andrey-nakin

@andrey-nakin what I'm saying is that I don't think data loaders are meant to be used in this way, and I don't understand why you need to do this. If you provide a more illustrative example, we might be able to discuss alternatives.

rstoyanchev avatar Jul 04 '22 08:07 rstoyanchev

@rstoyanchev ok, here's another example. I have the following Java class:

class Airport {
  String name;
  String cityCode;
}

and the corresponding GraphQL type in my schema:

type Airport {
  name: String
  cityCode: String
  countryName: String
}

Please notice that countryName is not backed by any field in Java class, so I have to write a resolving method for it.

I also have the following dictionaries:

class City {
  String code;
  String name;
  String countryCode;
}

class Country {
  String code;
  String name;
}

So my controller looks like this:

@Controller
class AirportController {

  public AirportController(BatchLoaderRegistry registry) {
    registry.forTypePair(String.class, City.class).registerMappedBatchLoader((cityCodes, env) -> Mono.fromCompletionStage(loadCities(cityCodes)));
    registry.forTypePair(String.class, Country.class).registerMappedBatchLoader((countryCodes, env) -> Mono.fromCompletionStage(loadCountries(countryCodes)));
  }


  public CompletionStage<String> countryName(Airport airport, DataLoader<String, City> cityLoader, DataLoader<String, Country> countryLoader) {
    return cityLoader.load(airport.cityCode).
      .thenCompose(city -> countryLoader.load(city.countryCode))
      .thenApply(country -> country.name);
  }
  
  private CompletionStage<Map<String, City>> loadCities(Set<String> cityCodes) {
    // here goes some asynchronous code
  }

  private CompletionStage<Map<String, Country>> loadCountries(Set<String> countryCodes) {
    // here goes some asynchronous code
  }
  
}

Unfortunately, the countryName method fails as the countryLoader.load method never returns and entire countryLoader does not seem to be dispatched for the loading (loadCountries method is never invoked).

andrey-nakin avatar Jul 05 '22 13:07 andrey-nakin

Thanks for the extra details. Once again I don't believe this is how a DataLoader is meant to be used. You can create the same basic sample without Spring, using just the dataloader library and you'll run into the same thing.

I believe, what you'll want to do is write a loadCountries method that takes a set of city codes as input, and encapsulates the logical join, but I could be wrong. You might want to ask under dataloader-java what the preferred way to achieve this is. Yes, the above snippet uses Spring but it's just syntactic sugar, and in the controller method you're using DataLoader instances from the dataloader-java library.

Closing for now, but feel free to comment with further links or comments.

rstoyanchev avatar Oct 26 '22 18:10 rstoyanchev