N+1 problem with fetchEntities

[klys-equinix]

N+1 problem with fetchEntities

[dariuszkuc]

Do we need to guarantee that the list of entities we are returning come in the same order as list of representations?

Yes.

But i cannot see this mentioned anyhow in the docs for this module. Is this issue handled somehow else by the library?

Its an implementation detail. fed-jvm is one of many subgraph libraries and they all should adhere to the same spec -> see docs for details.

[klys-equinix]

Ok, i agree that this is an implementation detail, and that fed-jvm is one of many subgraph libraries.
But the design of this particular library encourages breaking the rule of preserving order - that is why i asked to mention in docs that the fetchEntities should maintain order.

Furthermore, do you have any hints how to integrate fetchEntities with Spring's BatchLoaderRegistry used for batch fetching entities?

[dariuszkuc]

But the design of this particular library encourages breaking the rule of preserving order - that is why i asked to mention in docs that the fetchEntities should maintain order.

Can you elaborate what part of the design encourages breaking the rule of preserving order? Entity data fetcher gets a list of entity representations and list is an ordered collection of items (GraphQL does not support sets). Hence I am curious how would you map a list to an out-of-order list?

Furthermore, do you have any hints how to integrate fetchEntities with Spring's BatchLoaderRegistry used for batch fetching entities?

DISCLAIMER: I am not an expert on data loaders as I haven't used it much, answer below is based on my understanding of how it works together.

In general, using the data loader should be the same as with any other data fetcher/resolver -> i.e. you can access data loader through DataFetchingEnvironment#getDataLoader(<name>).

If your service resolves single entity type then it is pretty simple as you can just use single loadMany data loader call to return appropriate future. Otherwise, it becomes problematic as you would need to dispatch multiple data loaders which hits the problem described here. It is possible to work around it but it is somewhat messy.

One potential alternative is to NOT use the data loader when fetching entities -> simply create POJOs in your entity data fetcher using provided representation details and then use the data loader for resolving the heavy computation fields.

Also an FYI, using Spring GraphQL 1.3 is going to simplify this integration (docs). Unsure whether it addresses the multiple data loader calls though.

@EntityMapping
public Book book(DataFetchingEnvironment environment, @Argument int id) {
  return environment.getDataLoader("book").load(id);
}

[klys-equinix]

By encouraging i mean the fact that library makes one work with the list of representations, not with individual representation - which naturally makes one want to group and batch those representations.
But you are right that this 1.3 version is going to fix the exact problem i am having - I am unable to find a way to delegate the representation loading to those dataloaders, since to handle the representations i would need to call multiple data loaders

 @Override
  public Object get(DataFetchingEnvironment env) throws Exception {
    List<Map<String, Object>> representations = env.getArgument(_Entity.argumentName);
    return representations
        .stream()
        .map(
            representation ->
                Optional.ofNullable(env.getDataLoader((String) representation.get(TYPENAME)))
                    .orElseThrow(() -> new NoDataLoaderForTypeException((String) representation.get(TYPENAME)))
                    .load(representation)
        )
        .collect(Collectors.toSet());
  }

[klys-equinix]

So I ended writing a DataFetcher like this

@RequiredArgsConstructor
public class FederatedEntitiesDataFetcher implements DataFetcher<CompletableFuture<List<Object>>> {

  private static final String TYPENAME = "__typename";

  private final String modelPackage;

  @Override
  public CompletableFuture<List<Object>> get(DataFetchingEnvironment env) throws Exception {
    List<Map<String, Object>> representations = env.getArgument(_Entity.argumentName);
    var representationLoaders =
        representations.stream()
            .map(
                representation ->
                    getDataLoader(env, representation).load(getKeyFieldValue(representation)))
            .toList();
    return CompletableFuture.allOf(representationLoaders.toArray(new CompletableFuture<?>[0]))
        .thenApply(v -> representationLoaders.stream().map(CompletableFuture::join).toList());
  }

  private DataLoader<Object, Object> getDataLoader(
      DataFetchingEnvironment env, Map<String, Object> representation) {
    return Optional.ofNullable(env.getDataLoader(modelPackage + "." + representation.get(TYPENAME)))
        .orElseThrow(() -> new NoDataLoaderForTypeException((String) representation.get(TYPENAME)));
  }

  private static Object getKeyFieldValue(Map<String, Object> representation) {
    return representation.entrySet().stream()
        .filter(e -> !e.getKey().equals(TYPENAME))
        .findFirst()
        .map(Entry::getValue)
        .orElseThrow(
            () -> new MalformedRepresentationException((String) representation.get(TYPENAME)));
  }
}

It is important to return a single CompletableFuture from a DataFetcher, and also to verify that keys in Map returned in registerMappedBatchLoader match the types for keys that are the input to that method.

This works with representations of different entities in the same query.

Posting this for anyone who might have a similar problem