GraphQL returns inconsistent results for relationships

Link to reproduction

https://github.com/wkentdag/payload-gql-concurrency-repro

Describe the Bug

When querying a valid relationship that should return data, GraphQL will often return null, [], or Error 500: Cannot return null for non-nullable field X instead. This issue is very similar to #6221, except I'm seeing it across all relationship types, not just uploads.

The response is indeterminate, changing from one request to the next. Initially I thought it might be related to concurrency, but I've been able to reproduce it on individual queries sent to a freshly rebooted server as well. The behavior seems to occur when you request the same collection more than once in a single query:

query {
  articlesA: Articles {
    docs {
      id
      title
      _status
      contributors { # this will return `[]` sometimes
        name
      }
    }
  }

  articlesB: Articles {
    docs {
      id
      title
      _status
      contributors { # this will return `[]` sometimes
        name
      }
    }
  }
}

In practice, the error rate seems to increase along with the query complexity (my actual query errors ~90% of the time). But it happens frequently enough with this simple example that you should be able to reproduce it in the graphql playground with a few clicks.

Lastly, my queries that fit this criteria are performing horribly, often taking up to a minute to return or hanging until they time out. The vast majority of the execution time is spent at the application/resolver level, after postgres has mostly finished. Is it possible that there's a race condition or some kind of cyclic query dependency in the relationship resolver?

To Reproduce

Clone my reproduction repo
Add a couple Articles w/ referenced Contributors
Copy either of the demo queries into the GraphQL playground, run them 10-20 times, note the differing responses
Run cd ./test && chmod +x ./load-test.sh
Copy your API key into the script
Run the script to launch concurrent requests, note the discrepancy in response size:

~/Documents/code/larb/payload-gql-concurrency-repro/test  ~ ./load-test.sh 10
-rw-r--r--   1 wkd  staff  1925 Jun 17 02:35 out1.json
-rw-r--r--   1 wkd  staff  1925 Jun 17 02:35 out10.json
-rw-r--r--   1 wkd  staff  1925 Jun 17 02:35 out2.json
-rw-r--r--   1 wkd  staff  1925 Jun 17 02:35 out3.json
-rw-r--r--   1 wkd  staff  1925 Jun 17 02:35 out4.json
-rw-r--r--   1 wkd  staff  1925 Jun 17 02:35 out5.json
-rw-r--r--   1 wkd  staff  1574 Jun 17 02:35 out6.json 
                           ^^^^ missing data
-rw-r--r--   1 wkd  staff  1925 Jun 17 02:35 out7.json
-rw-r--r--   1 wkd  staff  1925 Jun 17 02:35 out8.json
-rw-r--r--   1 wkd  staff  1925 Jun 17 02:35 out9.json

I've also included two similar queries that don't reproduce the bug. I'm still trying to understand what's different about them, but thought that would be helpful context to add here.

graphql-missing-720.mov

Payload Version

2.20.0

Adapters and Plugins

@payloadcms/db-postgres": "^0.8.4"

I think there's a few associated problems here:

When both collection resolves execute concurrently, they each call the findResolver which internally uses isolateObjectProperty to proxy the request object and ensure a unique transaction. However, there's a bug in initTransaction: because transactionIDPromise is not proxied, other concurrent requests will wait on the promise, but will not have a transactionID on their request object after the await point (because they are a different proxy). This seems buggy.
There's a race in batchAndLoadDocs that may be related to the above. The transactionID on the computed batch key is assigned to the request object in order to reuse and existing transactions. However, it's possible for the parent transaction to have been closed before the batch load returns, which will cause the computed docsIndex key lookup to fail, as it uses a now null transaction id. This is the underlying problem in this issue which is why some relationships are returning null. When the invalid batch key is computed, these docs don't get populated and hence the problem.

It's possible to fix the above issue by NOT using req.transactionID in the computed batch key, but presumably the intention here was to re-use open transactions. The behavior we're seeing here is that sometimes our relationship batch fetch will happen in its own transaction and sometimes it won't if we don't reuse the transaction id from the batch key.

Okay, digging a little deeper, I was able to understand a bit better. The issue here isn't that the top level collection revolvers are executing concurrently, but that the relationship batching happens concurrently and the data loader find operation is not isolated. I have a patch that should work, I believe.

Ensure concurrent relationship batches use unique transacations.

@tychedelia

confirmed that #6821 fixes both the test reproduction and my actual use case. thanks a million @tychedelia 💕

fix(payload): #6800, #5108 - graphql query concurrency issues

fix(payload): #6800, #5108 - graphql query concurrency issues (#6857)

fix: graphql query concurrency issues (#6925)

This seems to be a problem again after the beta 95 / 96 release

This seems to be a problem again after the beta 95 / 96 release

I encountered this too, and furthermore it happens that on the REST API the depth parameter is not applied to the field in question. It only returns the ID of the related document, no matter how high the depth.
Somehow it works when I make an authenticated request, even though the field doesn't have any custom access configuration.

@linobino1

Somehow it works when I make an authenticated request, even though the field doesn't have any custom access configuration.

If you do not configure access control for a collection or global, the default is that it is only able to read by authenticated users. If I understand what you're saying, this is the expected behavior. Any failing access control on a relationship field itself will prevent the response from having the property at all, instead of just the ID.

I hope that clears this up for you @linobino1.

@linobino1

Somehow it works when I make an authenticated request, even though the field doesn't have any custom access configuration.

If you do not configure access control for a collection or global, the default is that it is only able to read by authenticated users. If I understand what you're saying, this is the expected behavior. Any failing access control on a relationship field itself will prevent the response from having the property at all, instead of just the ID.

I hope that clears this up for you @linobino1.

Thanks, I was a bit confused. In my case, the related collection was lacking public access permissions, thus the REST API would return the IDs only, and the GraphQL was not allowed to return any of the fields I requested, so it returned an empty array. I would have expected a 403 in that case maybe. But I can see that this behaviour is useful in other cases.

@yobottehg are there any additional reproduction steps? I can't reproduce this issue with the ones outlined in the original post

@paulpopus

@paulpopus we provided our project to @denolfe and provided videos and screenshots of the problem in slack. With that it's easily reproducible. I guess it correlates to the changes in beta 95 / 96 with the translatable reference fields with deep nested arrays inside it.

fix: #6800, graphql parallel queries with different fallback locales

fix: #6800, graphql parallel queries with different fallback locales (#…

Hey @yobottehg this one is now resolved and will be released shortly!

@jmikrut

@jmikrut can you confirm that the regression patched in d44fb2d was introduced in 3.x (and therefore not an issue in 2.x)?

@wkentdag

Hey @wkentdag this was actually re-opened due to another issue (not the original one that you experienced). The PR above fixed a case where you could have a single GraphQL query that specified different fallbackLocales, which would prevent certain populations from succeeding. It was not a regression - more like a new fix. We've been working with yobottehg on his issue separately so if you have not run into any funny business with separate fallback locales, you should still be good to go on 2.x.

That said, the fix I just pushed is not present on 2.x (it's only landed on 3.x). Technically, the bug that has been resolved with this PR probably still exists on 2.x but it's pretty fringe. If you run into a case where you need the new fix, let us know - but you should be good!

🚀 This is included in version v3.0.0-beta.101

This issue has been automatically locked.
Please open a new issue if this issue persists with any additional detail.

Navigation Menu

Repository navigation

GraphQL returns inconsistent results for relationships #6800

Link to reproduction

Describe the Bug

To Reproduce

Payload Version

Adapters and Plugins

4 remaining items

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

GraphQL returns inconsistent results for relationships #6800

Description

Link to reproduction

Describe the Bug

To Reproduce

Payload Version

Adapters and Plugins

Activity

tychedelia commented on Jun 17, 2024

tychedelia commented on Jun 17, 2024

wkentdag commented on Jun 18, 2024

rainer-friederich commented on Aug 30, 2024

4 remaining items

linobino1 commented on Sep 4, 2024

DanRibbens commented on Sep 4, 2024

linobino1 commented on Sep 4, 2024

paulpopus commented on Sep 6, 2024

rainer-friederich commented on Sep 6, 2024

jmikrut commented on Sep 9, 2024

wkentdag commented on Sep 9, 2024

jmikrut commented on Sep 9, 2024

github-actions commented on Sep 9, 2024

github-actions commented on Sep 9, 2024

github-actions commented on Sep 11, 2024

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions