Clients can't reliably distinguish if defer/stream was inlined or not

[IvanGoncharov]

The current proposal allows the inlining of the result of @defer directly into data of the initial response.
This makes it impossible to figure out if certain fields have already been delivered or not.

For example, with the below query:

{
  someAbstractType {
    name
    ... @defer(label: "deferLabel") {
      ... on SomeConcreteType {
        someField
      }
    }
  }

  ... @defer(label: "otherLabel") {
    someUnrelatedField
  }
}

and below response:

{
  "data": {
    "someAbstractType": {
      "name": "something"
    }
  },
  "hasNext": true,
}

It's impossible to know if deferred "deferLabel" is inlined or not.
So client code that waits for this particular label is locked until hasNext: false.

Stream Example:

{
  myList @stream(initialCount: 0) {
    id
  }
  ... @defer(label: "otherLabel") {
    someUnrelatedField
  }
}

and below response:

{
  "data": {
    "myList": []
  },
  "hasNext": true,
}

It's impossible to know if myList has additional items being prepared, or if myList is actually an empty array.

Proposed fix

Allow implementations to fully ignore @stream/defer at once (no incremental and hasNext either absent or false).
In all other cases, steam/defer results can be batched into incremental of the initial response.

[glasser]

I like that fix but I'd like to go even stronger and not have the choice between absent and false.

ie, the proposal would be, either there is a single map that has no incremental or hasNext field at all, or the data/errors in the first message only contains non-deferred data, and any "immediately available deferred data" comes in the first message's incremental.

[mjmahone]

This sounds like a very reasonable path. So the client has to handle servers that just ignore @defer but also knows if a server has deferred anything, then all defers are in separate payloads.

The downside is field duplication (the server can't "intelligently" determine an entire defer payload is duplicated in non-deferred data, and just not re-send the duplicated sub-tree again). But I think that's a reasonable tradeoff.

One alternative (I do not recommend going with this alternative, but want to lay it out for completeness) would be adding a meta-field like __isFulfilled(tag: String): boolean. This is one way one of our clients at Meta handles defer: it transforms the request above to:

{
  someAbstractType {
    name
    ... @defer(label: "deferLabel") {
      __fulfilledDeferLabel: __isFulfilled(tag: "deferLabel")
      ... on SomeConcreteType {
        someField
      }
    }
  }

  ... @defer(label: "otherLabel") {
    __fulfilledOtherLabel: __isFulfilled(tag: "otherLabel")
    someUnrelatedField
  }
}

in order to distinguish what has come back without looking into the extra key.

[yaacovCR]

For what purpose then are we still allowing clients to completely ignore defer? I suppose when the server sees that the entire result is cached? In that case, if it is a large result, clients have to wait to process the entire result before being able to access the initial payload?

Basically, I think now that we have the incremental map and a simple form of batching implemented, we should require servers to always serve incremental results as requested.

[yaacovCR]

Or is this suggestion possibly only with regard to defer and not in regard to stream?

[michaelstaib]

What we at the moment do from the client side is something very similar to what @mjmahone suggestes...

{
  someAbstractType {
    name
    ... @defer(label: "deferLabel") {
      __fulfilledDeferLabel: __typename
      ... on SomeConcreteType {
        someField
      }
    }
  }

  ... @defer(label: "otherLabel") {
    __fulfilledOtherLabel:__typename
    someUnrelatedField
  }
}

while a bit hacky it helps us identify which fragment is fullfilled in which payload.

[benjie]

Allow implementations to fully ignore @stream/defer at once (no incremental
and hasNext either absent or false). In all other cases, steam/defer results
can be batched into incremental of the initial response.

I'm concerned that this may lead to response explosion. Take for example this
'friends of friends' query:

{
  me {
    friends {
      id
      friends {
        id
        ... @defer(label: "1") { name bio }
        ... @defer(label: "2") { age bio }
        ... @defer(label: "3") { dob bio }
        ... @defer(label: "4") { avatarUrl bio }
        ... @defer(label: "5") { id bio }
        ... @defer(label: "6") { joinedAt bio }
        ... @defer(label: "7") { lastSeen bio }
        ... @defer(label: "8") { location bio }
      }
    }
  }
}

In a non-deferred query, we'd end up with effectively this query:

{
  me {
    friends {
      id
      friends {
        id
        name
        bio
        age
        dob
        avatarUrl
        joinedAt
        lastSeen
        location
      }
    }
  }
}

(Lets assume that the friends field always returns N records.)

{data:{me:{
  friends:[
    N x {id:..., friends: [
      N x {id:..., name:..., bio:..., age:..., dob:..., avatarUrl: ..., joinedAt:..., lastSeen:..., location:...}
    ] }
  ]
}}}

Most likely we already have all these fields thanks to DataLoader, so defer
wouldn't really give us any benefits and lets assume the server can determine
this.

---

In a forced-incremental context we'd get something more like:

{
  data:{me:{friends:[
    N x {id: ..., friends: [
      N x {id: ...}
    ]}
  ]}},
  incremental:[
    N x N x {label:"1",path:["me","friends",n,"friends",m],data:{name:..., bio:...}},
    N x N x {label:"2",path:["me","friends",n,"friends",m],data:{age:..., bio:...}},
    N x N x {label:"3",path:["me","friends",n,"friends",m],data:{dob:..., bio:...}},
    N x N x {label:"4",path:["me","friends",n,"friends",m],data:{avatarUrl:..., bio:...}},
    N x N x {label:"5",path:["me","friends",n,"friends",m],data:{id:..., bio:...}},
    N x N x {label:"6",path:["me","friends",n,"friends",m],data:{joinedAt:..., bio:...}},
    N x N x {label:"7",path:["me","friends",n,"friends",m],data:{lastSeen:..., bio:...}},
    N x N x {label:"8",path:["me","friends",n,"friends",m],data:{location:..., bio:...}},
  ]
}

(And this is without using fragments to expand the amount of data needed in each
payload!)

Imagine that the bio is a fairly standard Twitter-length bio of 160 bytes, and
that our friends field returns its cap of 100 results. In the no-defer
response we get N*N*160 bio bytes, which comes out at 1.6MB. In the deferred
response we get 8x as many bytes - 12.8MB, not to mention the additional data
encoded in the paths. If we were to alias those fields to be, say, 100
characters long, then that'd be an additional (100+100+100)*N*N*8 bytes just
in the paths (another 24MB minimum). Our "big" GraphQL query that returned
1.6MB of bio data without @defer now is well over 36.8MB with @defer.

This is highly redundant data and will compress well on the wire via
gzip/deflate; however for servers that build this string in memory before
compressing it (the majority of servers, I would guess - certainly any that use
an API analogous to JSON.stringify) it's still a significant memory burden. If
we ramp the bio up to 5kB of HTML our no-defer payload would be 50MB putting our
defer payload at well over 436MB prior to compression.

[glasser]

I get that, but isn't the problem the same if you actually do defer the payloads? It seems like the root cause of the issue here is "we don't merge fields across @defer payloads".

[benjie]

If we actually defer them (to a later payload) then the same amount of data is serialized in total, but it’s not done all at once - it’s spread out over multiple payloads, so the memory pressure is reduced and has lower spikes. But yes, I also worry that this label requirement forces this request amplification issue and merging all the defers on the same selection set would significantly reduce the attack surface. I’d also like fields that exist both outside the defer and inside it to be omitted from the defer because they’ve already been satisfied. But I’m sure there are good reasons for the label, which forces our hand.

[michaelstaib]

There are even worse scenarios where you can create so many patches that the execution actually degrades. Defer like that also opens attack vectors to the GraphQL server. We are at the moment working on mitigating this for hot chocolate with a couple of options. That is also where I still think the GraphQL server needs a way to optimize and ignore some of the defer/stream directives.

[yaacovCR]

See #58 which suggests to require fragment aliasing

[yaacovCR]

For defer, see graphql/graphql-spec#998 which alternatively suggests soft aliases or references that don't wrap the fragment's fields, but rather add another "reference" alongside them to indicate the state of the fragment.

[yaacovCR]

For stream, mentioned at the 11/17/2022 graphql-wg meeting, we could suggest the following response:

{
  "data": {
    "stream1": [],
    "stream2": [],
  },
  "incremental": [{
    "streamHasStarted": true,
    "path": ["stream1"],
    "label": "Stream1",
  }]
}

[yaacovCR]

Another suggestion for stream would be to introduce the idea of references for fields -- I believe @mjmahone mentioned something like this.

ie for a query like this:

{
   &referenceForStream1: aliasForStream1: stream1
   &referenceForStream2: aliasForStream2: stream2
}

We could have the initial response:

{
  "data": {
    "aliasForStream1": [],
    "referenceForStream1": "streaming",
    "aliasForStream2": [],
    "referenceForStream2": "completed",
  },
}

[robrichard]

Adding this idea as a jumping off point for discussion. This would be very straightforward to implement, and I think it could cover the all of the currently ambiguous cases.

We add a new array field pendingPayloads to the top level response object (name subject to workshopping). This array contains an object with a label and path field and is directly derived from subsequentPayloads right before the response is sent to the client.

Example

query {
  person(id: "1") {
    ...HomeWorldFragment @defer(label: "homeWorldDefer")
    name
    films @stream(initialCount: 0, label: "filmsStream") {
      title
    }
  }
}
fragment HomeWorldFragment on Person {
  homeWorld {
    name
  }
}

Response 1:

{
  "data": {
    "person": {
      "name": "Luke Skywalker",
      "films": []
    }
  },
  "pendingPayloads": [
    { "label": "filmsStream", "path": ["person", "films", 0] },
    { "label": "homeWorldDefer", "path": ["person"] }
  ],
  "hasNext": true
}

Response 2:

{
  "incremental": [{
    "label": "filmsStream",
    "path": ["person", "films", 0],
    "items": [
      { "title": "A New Hope" }
    ]
  }],
  "pendingPayloads": [
    { "label": "filmsStream", "path": ["person", "films", 1] },
    { "label": "homeWorldDefer", "path": ["person"] }
  ],
  "hasNext": true
}

Response 3:

{
  "incremental": [{
    "label": "homeWorldDefer",
    "path": ["person"],
    "data": {
      "homeWorld": { "name": "Tatooine" }
    }
  }],
  "pendingPayloads": [
    { "label": "filmsStream", "path": ["person", "films", 1] }
  ],
  "hasNext": true
}

Caveats

• label is optional. If the user doesn't provide a label, pendingPayloads is much less useful.
• All pending payloads may not be surfaced in the initial payload. For example, if a defer is nested under a stream, the executor is not aware of the deferred payloads until the streamed item is processed and sent. However, this won't lead to ambiguous results for the client as those payloads will be known by the time the stream payload is sent.
• Pending payloads may not eventually be sent. This can happen in the case of null bubbling that cancels a pending payload (See Cancel defer/stream execution when path is nulled from error bubbling #45)
• A pending stream payload may be sent then removed. If the stream is backed by an AsyncIterable (or similar data structure where the end of the results is not known synchronously), the executor will report a pending payload for the next item in the stream. Once the stream ends, that pendingPayload will be removed from the next response.

[IvanGoncharov]

Variation of @robrichard suggestion #52 (comment)

Instead of adding new top-level pendingPayloads we can just add hasNext field to every item of incremental.
So using @robrichard example it will look like:

Response 1:

{
  "data": {
    "person": {
      "name": "Luke Skywalker",
      "films": []
    }
  },
  "incremental": [
    { "label": "filmsStream", "path": ["person", "films", 0], "hasNext": true },
    { "label": "homeWorldDefer", "path": ["person"], "hasNext": true }
  ],
  "hasNext": true
}

Response 2:

{
  "incremental": [
    {
      "label": "filmsStream",
      "path": ["person", "films", 0],
      "hasNext": false,
      "items": [
        { "title": "A New Hope" }
      ]
    },
    { "label": "filmsStream", "path": ["person", "films", 1], "hasNext": true }
  ],
  "hasNext": true
}

Response 3:

{
  "incremental": [
    {
      "label": "homeWorldDefer",
      "path": ["person"],
      "data": {
        "homeWorld": { "name": "Tatooine" }
      }
    },
  ],
  "hasNext": true
}

Pending payloads may not eventually be sent. This can happen in the case of null bubbling that cancels a pending payload

It also solves this issue you just send:

{
  "incremental": [
    {
      "label": "homeWorldDefer",
      "path": ["person"],
      "data": {
        "homeWorld": { "name": "Tatooine" }
      }
    },
    { "label": "filmsStream", "path": ["person", "films", 1], "hasNext": false }
  ],
  "hasNext": true
}

[robrichard]

@martinbonnin tracking received payloads instead of pending would force the server to return a large amount of data in the case of a query like this with many list items:

{ list { list { …@defer { stuff } } } }

[martinbonnin]

@robrichard are you talking about Benjie's response explosion above? If yes then I think merging everything inside the initial "data" field workarounds this specific issue? (i.e. the bio field is merged and not replicated x8). Taking back @benjie example:

Response1 with inlined payload

{
  data:{me:{friends:[
    // The defered data is merged here
    N x {id: ..., friends: [
      // bio is present here
      N x {id: ..., bio: ..., name:.., }
    ]}
  ]}},
  // theses payloads do not contain data because it was merged above
  incremental:[
    N x N x {label:"1",path:["me","friends",n,"friends",m]},
    N x N x {label:"2",path:["me","friends",n,"friends",m]},
    N x N x {label:"3",path:["me","friends",n,"friends",m]},
    N x N x {label:"4",path:["me","friends",n,"friends",m]},
    N x N x {label:"5",path:["me","friends",n,"friends",m]},
    N x N x {label:"6",path:["me","friends",n,"friends",m]},
    N x N x {label:"7",path:["me","friends",n,"friends",m]},
    N x N x {label:"8",path:["me","friends",n,"friends",m]},
  ]
}

Of course it means there is 8 * N * N * (labels + path) transmitted but if we're thinking in terms of "pending" payloads these would be transmitted anyways in the "not inlined initial response" case?

[robrichard]

@martinbonnin the case is a client requests thousands of defer payloads and the server wants to optimize by inlining most of them.
If we specify received payloads, it would still have to send a large amount of data to inform the clients which payloads were inlined.

Executing a large amount of defer/stream payloads is bad for server performance and we discussed adding guidelines to the spec to instruct servers to inline in these cases. So I think its better that we don't force the server to generate a lot of data when it does choose to inline.

[martinbonnin]

I see, thanks! Just to make sure I understand, it'd still be valid to inline half of a list of payloads, right? If I query a list of 8 users:

query {
  users {
     ...userDetails @defer(label: "userDetails")
  }
}

fragment userDetails on User {
  name
}

Response 1 with half of the payloads inlined (possibly out of order):

{
  data: {
    users: [
      {
        name: "name0"
      },
      {}, // empty user, not received yet
      {
        name: "name2"
      },
      {}, // empty user, not received yet
      {
        name: "name4"
      },
      {}, // empty user, not received yet
      {
        name: "name6"
      },
      {}, // empty user, not received yet
    ]
  },
  "pendingPayloads": [
    { "label": "userDetails", "path": ["users",  1] },
    { "label": "userDetails", "path": ["users",  3] },
    { "label": "userDetails", "path": ["users",  5] },
    { "label": "userDetails", "path": ["users",  7] },
  ],
}

From a client point of view, this means we have to infer the received (labels + path) by substracting the pending ones. Might be doable but it's more involved than just checking the presence of a (label + path)

[robrichard]

Yes, clients need to be able to handle potentially any defer or stream being ignored by the server

[benjie]

Expanding on all of the previous discussions (thank you to @robrichard, @IvanGoncharov, @yaacovCR and everyone else who has helped iterate on these solutions), I've proposed an alternative on Discord which I'll reflect here.

My proposal (after todays on-call iteration - thanks to everyone's input) is to introduce the new pending field that tracks newly pending actions, and a completed field that tracks completed (or failed) previous pendings. Think of them as promises/async iterables that have just started. If there's no entry in pending for something from the given selection set then that thing was not @stream'd or @defer'd.

No label on @stream:

All fields with @stream should be mergeable, and removing the label increases the chance of this going through fine:

{
  me { ...UserFrag ...UserDetailFrag }
}
fragment UserFrag on User {
  friends @stream { ... }
}
fragment UserDetailFrag on User {
  friends @stream { ... }
}

The @stream can be identified by its path in the query (and any parent @defer labels), so the @stream label is unnecessary for identification.

Label on @defer uniquely identifies a set of deferred selections at that path

All @defer's with the same label within the same selection set (including traversing fragment spreads) will be treated as a single defer (their selections will be merged). label: null and no label given will be treated as equivalent for this case.

You'll then be able to identify an individual @defer promise or @stream async iterable by its path in the response and the list of @defer labels that have been seen up to and including it.

For the following query operation:

{
  me {
    ... @defer {
      name
    }
    ... @defer(label: "a") {
      favouriteColor
    }
    ... @defer(label: "b") {
      favouriteAuthor
    }
    ... on Node {
      ... on User {
        ... @defer(label: "a") {
          height
        }
      }
    }
    ... @defer {
      lastLoginAt
    }
  }
  ... @defer(label: "a") {
    me { id }
  }
}

there can be up to 4 defers created:

• path: ['me'], labels: [null] with selection set { name lastLoginAt }
• path: ['me'], labels: ["a"] with selection set { favouriteColor height }
• path: ['me'], labels: ["b"] with selection set { favouriteAuthor }
• path: [], labels: ["a"] with selection set { me { id } }

pending

When evaluating a selection set creates a @defer'd selection set or a @stream'd iterable, that defer or stream (promise or async iterable) will be given a unique identifier by the server. Each of these newly created identifiers (and only the newly created identifiers) will then be detailed in the pending field on the next payload the server returns, with the id, a reason ("stream" or "defer"), a path, and a list of labels that have been seen up to that point (if an @defer without a label is seen, then its label will be null, @streams don't have labels).

Once a @defer or @stream has been detailed in pending that id should never appear in pending in any future payloads for the same GraphQL request.

When a @stream completes, an entry will be added to completed in the next payload, with the id of the stream that completed. If the stream errored then in addition to the id there can be an error (singular) field.

A @defer's completion is implicit when it's id is seen in the incremental field. (Note: this also covers the case of errors, where {id: 7, errors: [ { ... }, ... ]} would be returned inside of incremental.)

For this query:

{
  me {
    name
    ... @defer {
      friends @stream {
	    ... @defer(label: "a") {
	      favouriteColor
	    }
      }
    }
    ... @defer(label: "d2") {
      friends @stream {
	    ... @defer(label: "a") {
	      name
	    }
      }
    }
  }
}

we might get the following pendings (in different payloads):

• id: 1, path: ['me'], labels: [null], reason: "defer"
• id: 2, path: ['me'], labels: ["d2"], reason: "defer"
• id: 3, path: ['me', 'friends'], labels: [null], reason: "stream"
• id: 4, path: ['me', 'friends'], labels: ["d2"], reason: "stream"
• id: 5, path: ['me', 'friends', 0], labels: [null, "a"], reason: "defer"
• id: 6, path: ['me', 'friends', 0], labels: ["d2", "a"], reason: "defer"
• id: 7, path: ['me', 'friends', 1], labels: [null, "a"], reason: "defer"
• id: 8, path: ['me', 'friends', 1], labels: ["d2", "a"], reason: "defer"
• etc

Note:

• the null labels are because the first @defer had no label
• no label is added for the @streams
• each @defer promise or @stream async iterable can be uniquely identified by the combination of its path and its labels
• a @stream that is not inside any @defers would have labels: [] - this is true even in a stream of a stream

No incremental in first payload

There's no need - all deferred/streamed items will be mentioned in pending and everything already satisfied should be directly inlined into the response.

The identifier

I'm currently proposing this identifier is a monotonically increasing integer (scoped to the operation), but honestly it could be a string or anything else - I'm open to suggestions.

By using the id we get two major advantages:

• no need for repeated path in the payloads, which reduces memory overhead during serialization and parsing, and also reduces network traffic
• no need for expensive path-traversal or array comparisons on the client, instead the client can keep references to the places to be patched in memory and can apply the patches directly and release the references when appropriate.

Example

For the following operation:

{
  users @stream {
    userId
    ... @defer { something }
  }
  users @stream {
    ... @defer { somethingElse }
  }
  me {
    userId
    ... @defer { gravatar }
  }
}

(which essentially becomes { users @stream { userId ... @defer { something somethingElse } } me { userId ... @defer { gravatar } } once all the fields/selections have been merged)

we might expect the following payloads:

{
  data: { users: [], me: { userId: 1 } },
  pending: [
    {path: ["users"], labels: [] reason: "stream", id: 1},
    {path: ["me"], labels: [null], reason: "defer", id: 2}
  ],
  hasNext: true
},

{
  // More items for the stream, it's not explicitly complete yet.
  incremental: [{
    id: 1,
    items: [
      { userId: 1 },
      { userId: 2, something: "...", somethingElse: "..." }
    ]
  }],
  // This `@stream`'d item triggered a new defer, but only for the first result
  // (the second result inlined everything):
  pending: [
    {path: ["users", 0], labels: [null], reason: "defer", id: 3}
  ],
  hasNext: true
},

{
  // id: 2 was a "defer", so giving it data (or errors) is sufficient to terminate it
  incremental: [
    {
      id: 2,
      data: { gravatar: null },
      errors: [
        {path: ["me", "gravatar"], message: "Non-null violation" }
      ]
    }
  ],
  hasNext: true
},

{
  // Stream 1 is complete
  completed: [{id: 1}],
  // Or: [{id: 1, error: {...}}]
  hasNext: true
},

{
  // Defer 3 is complete now
  incremental: [{id: 3, data: { something: "...", somethingElse: "..." }],
  // Done
  hasNext: false
}

hasNext

Technically this could be inferred by the presence and lack of resolution of pending, but it probably makes sense to keep it in to make the client's life easier.

[yaacovCR]

@benjie et all, some quick initial feedback to above proposal, two points:

Stream, Labels, and Label Hierarchies

A clarification above, because stream can be nested under defer (which currently branches execution), it seems multiple streams may indeed be created at the same path.

Example

query {
  me {
    ... @defer(label: "A") {
      name
      posts ... @stream {
        createdAt
        text
      }
    }
    ... @defer(label: "B") {
      occupation
      posts ... @stream {
        comments
      }
    }
}

A label on stream wouldn't actually help, of course, because the label is currently required to be the same! It's the parent branch labels that differ ("A" vs "B").

What would allow the stream to be identified uniquely is the entire execution branch/label hierarchy (reminiscent of earlier discussion re: info argument) graphql/graphql-js#2319 (comment)).

So the pending section in the scheme above for streams might need to be updated to include the entire label hierarchy, which would be more verbose in the pending section, but at least not in each payload, considering the introduction of id.

Note that a label hierarchy issue can also affect deferred payloads:

Example

query {
  me {
    id
    ... @defer(label: "A") {
      name
      ...UserData
    }
    ... @defer(label: "B") {
      occupation
      ...UserData
    }
  }
}
fragment on User {
  friends {
    id
    ... @defer(label: "Identical") {
      otherField
   }
  }
}

An execution branch with label "Identical" is currently created under both "A" and "B", duplicating fields unnecessarily and also making distinguishing between the arriving payloads impossible. The first problem mitigates the second, however. Whenever "Identical" shows up, it will be...identical, so the client will not care which branch actually completed. In fact, one could argue that the specification in permitting algorithm optimization does not require the second "Identical" payload to ever be sent.

Multiple Deferred Fragments with Identical Labels

This proposal allows merging of deferred payloads with identical labels encountered at a given level of field collection. This seems like a reasonable idea, but something that can be extracted from this proposal and iterated on later. Reasonable, as it will encourage fewer branches. This may be somewhat brittle when done manually, as it requires knowledge of existing labels that in theory could be spread about a code base. Perhaps this is something that would be utilized primarily by query compilers like Relay or Grafast.

I am pushing a bit back on it mostly to understand if this change is more deeply connected to the rest of the proposal than I currently appreciate.

Note that if we extend the merging suggestion even to nested selection sets, we could explicitly mitigate the label hierarchy issue just mentioned with respect to deferred fragments automatically. It does not sound like your proposal for merging fragments extends to subselections, however.

[benjie]

because stream can be nested under defer (which currently branches execution), it seems multiple streams may indeed be created at the same path.

Good point; I think we can solve this by removing label from the pending entries and instead having labels: Array<string | null> for all pending entries (in addition to path: Array<string | number>). Only the @defers would add to the labels. This would of course undo your proposed 'Identical' optimisation, but I'm okay with that.

[benjie]

I've updated the top comment to use labels instead of label to address the concern that @yaacovCR raised.

[leebyron]

Re: @yaacovCR's highlighted issue above, it does still seem underwhelming that this situation can result in two duplicate streams running at the same time. Is it really true that we can't avoid this situation, and therefore also avoid the need to separately identify them?

For example, could a service keep track of the open streams and an attempt to create a new one check for existing ones in flight (or completion?) Would that end up with a worse end state?

[yaacovCR]

For example, could a service keep track of the open streams and an attempt to create a new one check for existing ones in flight (or completion?) Would that end up with a worse end state?

graphql/graphql-js#3830

[mjmahone]

One of the concerns with the "pending" payloads is that we need to duplicate the field path: we can have the field path once by providing a pointer for future payloads, but that feels a little weird: it's a 2N cost instead of an N cost.

If we wanted to make things as easy as possible for clients, having an inline handle to point to future incremental responses would completely eliminate the need for a field path.

Inline Pending Pointers

Similar to above, @defer and @stream would merge to a single pointer per label (accepting that perhaps @stream would not even allow a label: arg).

I'd propose using a "#<label>": <ID> in the response key to indicate to the client what the pointer for the future response at that point in the tree is. For stream, we could either add the pointer as the last element of the list, or add a parallel-to-the-list-field pointer.

@defer example

Query:

{
  me {
    userId
    ... @defer { 
      something
      ... @defer { innerMerged }
      ... on User @defer(label: "a") { innerUnique }
    }
    ... @defer(label: "a") { cheapField }
  }
}

Responses might look like:

{
  data: { 
    me: {
      userId: 123,
      // this is the inline pointer for an unlabelled @defer
      #: 0,
      // At this point, the two "a" defers maybe don't merge? The innerUnique one is hidden by the unlabelled defer.
      // I'm not sure what the correct merging behavior is.
      cheapField: true
    },
  },
  hasNext: true
}

then

{
  incremental: {
    0: {
      data: {
        something: "x",
        innerMerged: "y",
        #a: 1
      },
      errors: [...]
    }
  },
  hasNext: true
}

and finally:

{
  incremental: {
    1: {
      data: {
        innerUnique: "z",
      },
    }
  },
  hasNext: false
}

@stream examples

Query:

{
  users @stream {
    userId
  }
}

Response option 1: pointer is last element of list

{
  data: { 
    users: [
      {userId: 123},
      "#:0",
    ],
  },
  hasNext: true
}

{
  incremental: {
    0: {
      items: [
        {userId: 456},
        "#:1",
      ]
    }
  },
  hasNext: true
}

{
  incremental: {
    1: {
      items: [
        {userId: 789},
      ]
    }
  },
  hasNext: false
}

Response option 2: pointer lives parallel to streamed field

{
  data: { 
    users: [{userId: 123}],
    // the field *is* the label: the key must begin with # otherwise I think we could have clashes with real fields
    #users: 0
  },
  hasNext: true
}

{
  incremental: {
    0: {
      items: [{userId: 456}],
      #users: 1
    }
  },
  hasNext: true
}

{
  incremental: {
    1: {
      items: [{userId: 789}]
    }
  },
  hasNext: false
}

[mjmahone]

As an addenda: if we're making a pointer type, it likely makes sense to make it an object so we can stuff whatever we want into it.
In which case the stream example might look like:

users: [{userId: 123}],
#users: { id: 0, index: 1 }

while a defer example might just be:

#: {id: 0}

This would still likely be cheaper than using field paths in an extra payload.

Another alternative would be to use a single # field to collect all promises at that level:

{
  me {
    userId
    ... @defer(label: "a") { something }
  }
  users @stream { userId }
  ... @defer { somethingElse }
}

me: {
  userId: 4,
  #: [{kind: "defer", label: "a", id: 1}]
},
users: [],
#: [
  {kind: "stream", field: "users", id: 0},
  {kind: "defer", id: 2},
]

[michaelstaib]

I like the idea ... but not keen on misusing comments here.

I mean do we need to have all that metadata in this case? since we have now clear pointers?

[michaelstaib]

what if the pointer is more like an object itself ...

{
  incremental: {
    0: {
      items: [
        {userId: 456},
        { "#1": "stream"  },
      ]
    }
  },
  hasNext: true
}

Not sure ... feels all hacky... but it would be super efficient to get a pointer approach working.

[benjie]

@michaelstaib if you edit your example to use json5 rather than json then the highlighting errors will go away :)

[robrichard]

We decided not to pursue this inline pointer proposal any further

from @mjmahone:

I’m becoming more convinced this inline proposal is kludgy and there’s no real elegant way to do it without fragment response keys. In which case IMO we should probably spend time figuring out Benjie’s format more exactly (specifically, how do we simplify the path to the least complex possible)

[yaacovCR]

I appreciate the advantages that the pending approach give us => there is a clear translation between what is happening on the server and what is communicated to the client. It is easy to reason about (if not necessarily easy for the server to decide what to do!)

I do wonder whether it is so GraphQL-esque. The promise of GraphQL is "ask for what you need, get exactly what you want." Flowing from this principle, we should provide for the client what the client is interested in, rather than what we have available. The client wants to know which portions of the result tree have been delivered; not a description of what is pending.

So, in general, I favor metadata that labels what we have already, whether we call this a metafield, fragment aliases, fragment references, etc. But can we push this further?

Working off of @mjmahone 's example above:

{
  me {
    userId
    ... @defer(label: "1") { 
      something
      ... @defer(label: "2") { innerMerged }
      ... on User @defer(label: "3") { innerUnique }
    }
    ... @defer(label: "4") { cheapField }
  }
}

Could yield:

{
  data: { 
    me: {
      #fulfilled: [["4"]],
      userId: 123,
      cheapField: true
    },
  },
  hasNext: true
}

and then

{
  data: {
    me: {
      #fulfilled: [["1"], ["1", "3"]], 
      something: "x",       # "something" completely fulfills the fragment labelled 1
      innerMerged: "y",   # and the fragment labelled 3 (nested under 1) is completely fulfilled as well
    },
  errors: [...]
  hasNext: true
}

and finally:

{
  data: {
    #fulfilled: [["1", "2"]], 
    me {
      innerUnique: "z",
    }
  },
  hasNext: false
}

How would stream work? Probably differently, more along the lines of the pending approach.

[yaacovCR]

Of note, wildcards or some other syntax might help us indicate that an entire set of deferred fragments on a list have been inlined.

[michaelstaib]

what you have posted reminds me of what @mjmahone posted a couple of days ago with his pointers proposal. What I really dislike is that we essentially try to misuse comments in JSON

[yaacovCR]

what you have posted reminds me of what @mjmahone posted a couple of days ago with his pointers proposal. What I really dislike is that we essentially try to misuse comments in JSON

We could easily set aside a separate fulfilled tree section akin to pending section if that bothers u

[benjie]

This feels to me more of a divergence from GraphQL's response shape matching the request shape. If we were to do something like that, I'd rather it followed GraphQL syntax, e.g. using a "__defer" field rather than an @defer directive:

{
  me {
    userId
    __defer { 
      something
      __defer { innerMerged }
      ... on User {
        __defer { innerUnique }
      }
    }
    defer2: __defer(label: "2") { cheapField }
  }
}

(This would also lend itself naturally to the @defer-merging that my proposal addresses since all the __defers with the same label would be merged by field-merging.)

I'd have to think more about what the intermediate states would be though. Could even be {"__defer":{"__typename": "__Pending"}} union-style value...

... But either way, this does not solve the stream issue - how would you indicate that a stream is incomplete? I'm not sure this can be solved in an attractive way with inline data.

[robrichard]

Adding a new proposal that further iterates on @benjie and @IvanGoncharov's suggestions

Proposal

1. Remove label from both defer and stream
2. All deferred fragments on the same object are merged into a single selection set that is deferred together.
   1. Server can choose to inline deferred fragments, but if it does inline, it must inline all of the deferred fragments on this object.
3. Update OverlappingFieldsCanBeMergedRule so a field with stream cannot be merged with any other field. Aliases can be used to support multiple streams on the same field. This prevents multiple streams from executing on the same field when one is inside a defer and the other is not.
4. Add simplified version of @benjie's pending proposal
   1. a pending object is sent once when promise begins executing. It contains only a path key.
   2. This path points to the list field for stream, not a specific index.
   3. No pending object is sent if the associated defer/stream is inlined by server, or inlined by client by passing if: false.
5. When a stream is completed an incremental payload is sent with completed: true. This could come with the final items if the stream is closed synchronously or without items if closed asynchronously.

Caveats

• It's no longer possible to have separate deferred fragments to arrive at different times. A future GraphQL spec with fragment aliases would allow that. Aliasing the parent field of the deferred fragments is a workaround that could be used to enable this behavior, (but that's not possible if the deferred fragments are directly under the query type).

Example A: Multiple defers on the same object are merged

query {
  friendList {
    ... @defer {
      ...FriendFrag
      ... @defer {
        ...FriendFrag
        ... @defer {
          ...FriendFrag
          ... @defer {
            ...FriendFrag
          }
        }
      }
    }
  }
}
fragment FriendFrag on Friend {
  id
  name
}

// Responses
[
  {
    "data": { "friendList": [{}, {}, {}] },
    "pending": [
      { "path": ["friendList", 0] },
      { "path": ["friendList", 1] },
      { "path": ["friendList", 2] }
    ],
    "hasNext": true
  },
  {
    "hasNext": false,
    "incremental": [
      { "data": { "id": "1", "name": "Luke" }, "path": ["friendList", 0] },
      { "data": { "id": "2", "name": "Han" }, "path": ["friendList", 1] },
      { "data": { "id": "3", "name": "Leia" }, "path": ["friendList", 2] }
    ]
  }
]

Example B: Defer inside of stream, stream closes synchronously

Although the incremental payloads for defer and stream have the same path, they are not ambiguous because the defer payload contains data and the stream payload contains items.

query {
  friendList @stream {
    id
    ... @defer {
      name
    }
  }
}

// Responses
[
  {
    "data": {
      "friendList": []
    },
    "pending": [{ "path": ["friendList"] }], // stream is pending
    "hasNext": true
  },
  {
    "hasNext": true,
    "pending": [{ "path": ["friendList", 0] }], // defer on first list item is pending
    "incremental": [{ "path": ["friendList", 0], "items": [{ "id": "1" }] }]
  },
  {
    "hasNext": true,
    "pending": [{ "path": ["friendList", 1] }],
    "incremental": [{ "path": ["friendList", 0], "data": { "name": "Luke" } }]
  },
  {
    "hasNext": true,
    "incremental": [{ "path": ["friendList", 1], "items": [{ "id": "2" }, "completed": true] }] // stream is complete
  },
  {
    "hasNext": false,
    "incremental": [{ "path": ["friendList", 1], "data": { "name": "Han" } }]
  }
]

Example C: Stream inside of defer, stream closes asynchronously

query {
  ... @defer {
    friendList @stream {
      id
    }
  }
}

// Responses
[
  {
    "data": {},
    "pending": [{ "path": [] }], // defer is pending
    "hasNext": true
  },
  {
    "hasNext": true,
    "pending": [{ "path": ["friendList"] }], // stream is pending
    "incremental": [{ "path": [], "data": { "friendList": [] } }]
  },
  {
    "hasNext": true,
    "incremental": [{ "path": ["friendList", 0], "items": [{ "id": "1" }] }]
  },
  {
    "hasNext": true,
    "incremental": [{ "path": ["friendList", 1], "items": [{ "id": "2" }] }]
  },
  {
    "hasNext": true,
    "incremental": [{ "path": ["friendList", 2], "items": [{ "id": "3" }] }]
  },
  {
    "hasNext": false,
    "incremental": [{ "path": ["friendList"], "completed": true }] // stream is completed
  }
]

Open question

How are fields merged that are part of both a deferred and non-deferred selection set?

CollectFields currently only analyzes and merges the selection set at the current object level. Would CollectFields need to deeply analyze the entire selection set before it can decide which fields are deferred or not? If we do remove fields out of deferred fragments, we can relax the validation rule preventing streamed fields from merging.

Example D: Nested objects with overlapping deferred and non-deferred fields

{    
  f2 { a b c { d e f { h i } } }
  ... @defer {
    f2 { a b c { d e f { h j } } }
  }
}

Can we make this equivalent to this?

{  
  f2 { a b c { d e f { h i } } }
  ... @defer {
    f2 { c { f { j } } }
  }
}

Example E: With a naive implementation there is still the opportunity for multiple defers pointing to the same path

query { 
  nestedObject {
    ... @defer {
      foo
    }
  }
  ... @defer {
    nestedObject {
      ... @defer {
        bar
      }
    }
  }
}

Results from naive implementation:

Note that there are two objects each in pending and incremental with a path of ["nestedObject"].

[
  {
    "data": { "nestedObject": {} },
    "pending": [
      { "path": [] },
      { "path": ["nestedObject"] }
    ],
    "hasNext": true
  },
  {
    "pending": [
      { "path": ["nestedObject"] }
    ],
    "incremental": [
      { 
        "path": [], 
        "data": { "nestedObject": {} }
      }
    ],
    "hasNext": true
  },
  {
    "incremental": [
      { 
        "path": ["nestedObject"],
        "data": { "foo": "foo" } 
       }
    ],
    "hasNext": true
  },
  {
    "incremental": [
      {
        "path": ["nestedObject"],
        "data": { "bar": "bar" }
      }
    ],
    "hasNext": false
  }
]

Improved implementation

An improved merging algorithm, as proposed in graphql/graphql-js#3820 can give a result that does not do any deduplication, but guarantees all payloads have unique paths:

[
  {
    "data": { "nestedObject": {} },
    "hasNext": true,
    "pending": [
      {"path": []},
      {"path": ["nestedObject"]}
    ]
  },
  {
    "incremental": [
      {
        "data": { "nestedObject": {} },
        "path": []
      },
      {
        "data": { "foo": "foo", "bar": "foo" },
        "path": ["nestedObject"]
      }
    ],
    "hasNext": false
  }
]

Example F: Optimally deduplicating fields could depend on the order which data is resolved

If the first defer is resolved first, a.b.e.f can be removed from the second defer. If the second defer is resolved first, it cannot be removed, and the second defer can be removed entirely.

{
  a {
    b {
      c {
        d
      }
      ... @defer {
        e {
          f
        }
      }
    }
  }
  ... @defer {
    a {
      b {
        e {
          f
        }
      }
    }
    g {
      h
    }
  }
}

[yaacovCR]

My implementation deduplicates leaf fields, but not object fields.

Example

Query:

query {
  hero {
    nestedObject { foo }
    anotherNestedObject { foo }
    ... @defer {
      nestedObject { bar }
      anotherNestedObject { foo }
    }
  }
}

[
  {
    "data": {
      "hero": {
        "nestedObject": {
          "foo": "foo",
        },
        "anotherNestedObject": {
          "foo": "foo",
        },
      },
    },
    "hasNext": true,
  },
  {
    "incremental": [
      {
        "data": {
          "nestedObject": {
            "bar": "bar",
          },
          "anotherNestedObject": {},  ## this is extra, but we didn't know that until we went deeper, so we couldn't deduplicate
        },
        "path": ["hero"],
      },
    ],
    "hasNext": false,
  },
]

I think this is ok! We can add a normative note that says that implementations are encouraged to deduplicate as much as possible, but this is not required, explaining that the defers are to be interpreted as patches, and so an empty field as in the above, would just be a partially empty patch, but is not incorrect / invalid / non-compliant.

[yaacovCR]

Regarding the removal of the label: this is problematic for clients that generate parsers which need to know which fields are expected in any given payload.

If we look at example E above, given the last 2 payloads, I don’t think there’s a way for a parser to know when to read foo vs bar.

I think @BoD is correct that the response in Example E should not have two paths at the same level. I understood that our merging strategy would cause defers to be merged at the same level, even if some of the defers are nested under defers at higher levels. As long as we return only one deferred payload at each path, we can safely remove label even without resorting to a __fulfilled metafield like solution.

In fact, adapted from the test cases from my implementation, the operation:

query {
  nestedObject {
    ... @defer {
      foo
    }
  }
  ... @defer {
    nestedObject {
      ... @defer {
        bar
      }
    }
  }
}

is largely equivalent to:

query {
  nestedObject {
    ... @defer {
      foo
      bar
    }
  }
}

as desired -- putting aside some failure of non-leaf field deduplication! -- but all payloads at the path of "nestedObject" are delivered together:

[
  {
    "data": {
      "nestedObject": {},
    },
    "hasNext": true,
  },
  {
    "incremental": [
      {
        "data": {
          "foo": "foo",
          "bar": "bar",
        },
        "path": ["nestedObject"],
      },
      {
        "data": {
          "nestedObject": {},  ### this is the annoying lack of non-leaf deduplication
        },
        "path": [],
      },
    ],
    "hasNext": false,
  },
]

[yaacovCR]

Take the following example:

query {
  a {
    b {
      c {
        d
      }
      ... @defer {
        e {
          f
        }
      }
    }
  }
  ... @defer {
    a {
      b {
        e {
          f
          slow
        }
      }
    }
    g {
      h
    }
  }
}

@robrichard pointed out a problem:

[
  {
    "data": {
      "a": {
        "b": {
          "c": {
            "d": "d",
          },
        },
      },
    },
    "hasNext": true,
  },
  {
    "incremental": [
      {
        "data": {
          "e": {},    ### <== this is bad, the deduplicated leaf is sent first, so the client receives a payload for the defer at path a.b with incomplete data!!
        },
        "path": ["a", "b"],
      },
      {
        "data": {
          "a": {
            "b": {
              "e": {
                "f": "f",
                "slow": "slow",
              },
            },
          },
          "g": {
            "h": "h",
          },
        },
        "path": [],
      },
    ],
    "hasNext": false,
  },
]

My leaf deduplication algorithm is deterministic prior to field execution, only executing leaf fields once. I could switch which defer branch executes the leaf (the more deeply nested or the more shallow one, for example), but this wouldn't help, as what we need to do is only execute the field within the payload that will entirely complete first, and that is not yet known.

Another option is to send the defer payloads in some arbitrary fixed order (perhaps in terms of how deeply nested). This impinges on the utility of defer, as fields will no longer be sent when ready depending on what other field combinations have been requested, in the name of data savings not necessarily requested by the client.

@robrichard suggested the option to simply not deduplicate execution for leaf fields. Fields sent in the initial response can always still be deduplicated.

Servers still also have the option of deduplicating leaf results just prior to sending the data to the client, i.e. deduplicating the results within the response rather than deduplicating the execution itself. The data savings may not be the worth the performance penalty, but to aid exploration, I have implemented this within the PR at graphql/graphql-js#3820. The result for the above query will therefore be:

[
  {
    "data": {
      "a": {
        "b": {
          "c": {
            "d": "d",
          },
        },
      },
    },
    "hasNext": true,
  },
  {
    "incremental": [
      {
        "data": {
          "e": {
            "f": "f",  ### here is the first f, never deduplicated when first appears
          },
        },
        "path": ["a", "b"],
      },
      {
        "data": {
          "a": {
            "b": {
              "e": { ### f was executed again, but is hidden from the output!
                "slow": "slow",
              },
            },
          },
          "g": {
            "h": "h",
          },
        },
        "path": [],
      },
    ],
    "hasNext": false,
  },
]

[yaacovCR]

@robrichard wrote way above:

• It's no longer possible to have separate deferred fragments to arrive at different times. A future GraphQL spec with fragment aliases would allow that. Aliasing the parent field of the deferred fragments is a workaround that could be used to enable this behavior, (but that's not possible if the deferred fragments are directly under the query type).

Now that we have some logic for field deduplication worked out, I think it is worthwhile to revisit the above. Although the workaround mentioned above would allow defers at the same level to be sent separately, that would come at the cost of no longer being able to deduplicate fields sent in the initial response. When/if fragment aliases land, they may include their own form of data deduplication, so this might be mitigated later.

Using the current workaround, for the below example, we can no longer deduplicate much of the below deferred payloads:

query {
  a {
    b {
      c {
        d
      }
    }
    component1: b {
      ... @defer {
        c {
          d
        }
        e {
          f
          slow
        }
      }
    }
    component2: b {
      ... @defer {
        c {
          d
        }
        e {
          f
        }
      }
    }
  }
}

[yaacovCR]

Just an update on graphql/graphql-js#3820, which has been reworked a little further:

Progress

on graphql-spec:

robrichard/graphql-spec#10 -- spec PR for merging defers, adding a note that deduplication is allowed, but not including it within the spec algorithms

on graphql-js:

graphql/graphql-js#3820 -- ref impl changes corresponding to above spec PR #10

graphql/graphql-js#3828 -- deduplication type 1 = no re-execution of leaf fields in initial payload
graphql/graphql-js#3825 -- deduplication type 2 = track leaf fields and no re-sending of leaf fields by any payload if sent in any other payload

graphql/graphql-js#3826 -- an experiment built on #3820 showing hoisting of the field depth out of the TaggedFieldNode and into the FieldGroup type

Weakly-Held Opinions

On hoisting

I don't think #3826 needs to go in. In the long run, collectSubFields may end up as a method on a FieldGroup object, At that point, the depth should probably be hoisted!

On deduplication

I don't have an informed opinion as to whether deduplication should go in graphql-js right now -- i guess it depends on the real world performance of my implementations...

If my implementations carry with them a performance hit, I am still hopeful that algorithms can be found with satisfactory performance profiles => and if such algorithms can be found, I have a strong opinion that these optimizations should go into graphql-js even if not in spec, as besides being the reference implementation, graphql-js also functions as either the most popular or a very popular ecmascript execution engine.

But considering I don't know the performance profile of the above initial work, I can't really say we are there yet. Happy to put under a feature flag or just delay those portions.

[robrichard]

Discussion around this issue is being continued in these three related discussions:

• Remove labels and merge deferred fragments under the same object #64
• Deduplication of deferred fields #65
• Include an indication of upcoming payloads in response #66