Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add resolver timeout #19

Open
wants to merge 6 commits into
base: joysteam
Choose a base branch
from

Conversation

thesan
Copy link
Member

@thesan thesan commented Mar 12, 2024

Address the part 2 of Joystream/joystream#5088

It's a bit hacky but the only efficient way I found to really stop the resolvers (and not having queries still happening in the background) is through the DataLoader middleware. The main reason is that this middleware is responsible for the n+1 problem so other middlewares wouldn't be able to abort the queries the DataLoader initiate.

Also it has to split the relation queries into chunks instead of running everything in parallel (WARTHOG_RELATION_CONCURRENCY set the size of the chunk). It might affect the performance of slow queries but it could also free some db connection for other clients so overall I think it's not too bad.

I updated typescript in order to use AbortSignal.timeout(reqTimeout) because I couldn't figure out a way to clear timeout from the more traditional setTimeout, because next() resolves before the relations queries (which is weird because when running a middleware after this one it doesn't resolves until all fields have resolves 🤷)

FYI I tested it with slow "horizontal" queries like distributionBuckets(where: { id_eq: "0:1" }) { bags { objects { id } } but also huge queries with a lot of depth. It does work in both case because even with the deeper queries the first resolvers dataloaders get's called for all deeper entities. However these deep queries will return some of the data it got before the timeout and and errors for every entities which timed out.

@thesan
Copy link
Member Author

thesan commented Mar 19, 2024

I've been trying to test the QN build for a few hours, but it turns out to be a lot more difficult than I expected.

@joystream/warthog has not been touched in 2 years. The package builds fine but even with the last published version of the code (not including this PR changes) the Graphql server build fails.

Here's the way I went at it:

  • In the warthog repos I just install the packages and run the build: yarn --immutable && yarn run build
  • In the monorepo:
    • I added this package resolution: "@joystream/warthog": "file:/path/to/warthog" to both ./package.json and ./query-node/codegen/package.json.
    • I also added rm -rf ../node_modules/@joystream/warthog/src ; rm -rf codegen/node_modules/@joystream/warthog/src to ./query-node/build.sh (after the yarn line) because the warthog cli behaves differently if the package has an src folder.
    • Then I run yarn workspace query-node-root build && docker compose up graphql-server (with an already populated db running).

The builds kind of succeeds but the warthog codegen script shows:

CannotDetermineGraphQLTypeError: Cannot determine GraphQL input type for 'first' of 'ConnectionPageInputOptions' class. Is the value, that is used as its TS type or explicit type, decorated with a proper decorator or is it a proper input value?

Finally the Graphql server crashes with:

CannotDetermineGraphQLTypeError: Cannot determine GraphQL input type for 'offset' of 'PaginationArgs' class. Is the value, that is used as its TS type or explicit type, decorated with a proper decorator or is it a proper input value?

@thesan thesan marked this pull request as draft March 19, 2024 16:44
@thesan
Copy link
Member Author

thesan commented Apr 10, 2024

So there was 2 issues:

  1. the published version of the package is not on this repository it's actually zeeshanakram3:joysteam. So I merged it to this branch. As a result this PR includes unrelated changes, but I think these should be merged too.
  2. yarn link doesn't work for the query-node-codegen package, I think some link breaks in the query-node generated workspace node_modules (not sure why exactly). So I tested with resolutions to my local version (in package.json and query-node/codegen/package.json) and it finally built!

I ran the network tests with the build and everything passed.

@zeeshanakram3 please have a look when you get the time.

@thesan thesan marked this pull request as ready for review April 10, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants