Reduce interface expansion for types contained to a single service

[jbaxleyiii]

Purpose

The purpose of this PR is to take a first step towards minimizing query plans and their requests to downstream services. Currently, querying fields that return an abstract type result in large query plans because the planner expands these fields into an inline fragment on every implementing type.

Optimization

This work targets an optimization concerning the over-expansion of abstract types. Previously, any query that asked for a field on an interface would be expanded to a selection of type conditions for all possible types. This would result in overly large operations sent to downstream services. If every field of an interface and its implementors belong to a single service, we can optimize the query plan by not expanding all possible types when building queries for fields that return an interface type.

[delyanr]

Hello @jbaxleyiii. This pull request is related to an issue I opened: #3644

I'm working on a complex architecture that uses multiple abstract types, with services extending the fields of both interfaces and their implementing types. There are significant slow-downs due to:

• Massive (10K+ lines) query plans that simply take too long to prepare and parse.
• Multiple unnecessary calls to the underlying services due to duplicated queries from the explosion of interfaces into their implementing types.

Unfortunately, this pull request crashes queries because of two main reasons:

• Say you extend an interface (and its implementing types) with new fields. When you query those fields on the interface, since the interface itself is not (and cannot be) in the _Entity union, the query will result in "missing fields" error. The query planner with the optimisation in the pull request blindly sends those fields to the base service, rather than the owning service.
• Say there is a union that uses, among other types, all implementing types of an interface. When you try to query the union and use the interface to select particular fields, you would get the same effect as the above, but it can happen even when the query planner sends this to the owning service. (This point is a bit more subtle.)

Based on the above, I think a better optimisation (that requires no changes to other parts of the code) is as follows (and can substitute the entire uniqueServices logic):

const possibleFieldDefs = scope.possibleTypes.map(
  runtimeType => context.getFieldDef(runtimeType, field.fieldNode),
);
const federations = possibleFieldDefs.filter(def => !!def.federation);
if (federations.length === 0) {
  const group = groupForField(field as Field<GraphQLObjectType>);
  group.fields.push(
    completeField(context, scope, group, path, fieldsForResponseName)
  );
  continue;
}

This works because it focuses on the fields themselves. If a field has been extended in any service across one or more of its implementing types, simply explode this field into all possible types. If not, activate the optimisation. This works without errors and has reduced the run-time of some queries by a whooping factor of 10.

Let me know if this makes sense, and whether you would like to change your pull request, or if you want me to open a PR for it? (However, at present, I won't have time to add new tests.)

Regards!

[trevor-scheer]

Hey @delyanr, thanks for the input on this. I'm currently investigating this solution (as well as yours) in hopes of finding the best outcome.

I tried your suggestion, and this seems to fail a (fairly recently added) test concerned with interfaces and preventing expansion of impossible types. Please feel free to take this branch and explore. I'm getting into the weeds with this now, but just wanted to fill you in on what I've found so far in case you're interested in working on this any further.

PRs to this branch will definitely be entertained and welcome! As a side note, I would love to see tests which reproduce the breaking behavior you're concerned with:

this pull request crashes queries because of two main reasons

Thanks again for taking a look at this!

[delyanr]

Hi @trevor-scheer. Thanks for looking into this, since it is a huge issue for complex federations.

In my code above, I've forgotten to include one check in the if statement. Please change

if (federations.length === 0) {

to

if (scope.possibleTypes.length > 0 && federations.length === 0) {

This should now work as expected. (We don't want to explode fields with no possible runtime types.)

I will work to include some additional tests that show how the previous commit was causing problems, but this will take me a couple of days when I'm less busy.

Regards!

[trevor-scheer]

Thanks for revisiting this @delyanr! I've tidied up the code a bit, but your solution is correct and I've pushed it to this branch (via 3c19fae) with co-authorship attributed to you.

[delyanr]

Hi @trevor-scheer! Thanks for making the changes. I still feel a bit uneasy merging this without adding some more tests. Give me a couple of days and I will add the ones I'm thinking about, just to make sure all is working.