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

GraphQL parse/validation caching #3077

Closed
dotansimha opened this issue Dec 16, 2021 · 3 comments
Closed

GraphQL parse/validation caching #3077

dotansimha opened this issue Dec 16, 2021 · 3 comments

Comments

@dotansimha
Copy link
Contributor

Since GraphQL parse and validate phase are static (does not depend on the user input like variables), we can implement caching for both phases (for parser: String => query::Document, for validation: String | query::Document => Vec<ValidationError>).

This might be more relevant once #3057 lands, because if we'll add more validation rules it might add some (probably minimal) overhead.

Usually it's implemented with LRU.

@lutter
Copy link
Collaborator

lutter commented Dec 22, 2021

That's a good idea! For our purposes, I think it would be enough to keep a HashSet of the shape hashes of the queries that were successfully validated. We already calculate that hash (a u64) for all queries anyway. And since it's so little data, I don't think we have to worry about cache eviction and just keep those hashes for the lifetime of the service.

@dotansimha dotansimha moved this from Todo to In Progress in GraphQL API Jan 13, 2022
@dotansimha
Copy link
Contributor Author

#3188 (comment)

Repository owner moved this from In Progress to Done in GraphQL API Jan 25, 2022
@dotansimha dotansimha reopened this Jul 20, 2022
@dotansimha
Copy link
Contributor Author

Reopened, due to the high CPU issue we recently had with GraphQL validations.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

3 participants