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 support for Automatic Persisted Queries #401

Closed
icco opened this issue Oct 28, 2018 · 12 comments
Closed

Add support for Automatic Persisted Queries #401

icco opened this issue Oct 28, 2018 · 12 comments
Labels
enhancement New feature or request

Comments

@icco
Copy link
Contributor

icco commented Oct 28, 2018

This is more of a feature request than a bug, and I'd be willing to do the work if it's something people are interested in.

Apollo added this neat feature which allows for dynamic query caching.

This would require an addition to the parsing logic so that a request without a query would still work. For example:

{
  operationName: 'MyQuery',
  variables: null,
  extensions: {
    persistedQuery: {
      version: 1,
      sha256Hash: hashOfQuery
    }
  }
}

currently fails at

sendErrorf(w, http.StatusUnprocessableEntity, "operation %s not found", reqParams.OperationName)
,

Instead this would change to not send a 422, but rather a 200 with { message: 'PersistedQueryNotFound' }

@vektah
Copy link
Collaborator

vektah commented Oct 28, 2018

It's something we've talked a bit about internally and a few others have expressed interest. It needs a little thought and care around the interfaces for saving and fetching queries, as these will need to actually be persisted.

@icco
Copy link
Contributor Author

icco commented Oct 29, 2018

I was imagining a first pass would just be to augment the LRU being used to also store a hash. Not great for storing lots of queries, but would probably work well without drastically changing the performance profile of the app.

@mathewbyrne mathewbyrne added the enhancement New feature or request label Nov 2, 2018
@vektah
Copy link
Collaborator

vektah commented Nov 2, 2018

My current understanding of APQ is roughly like this:

  1. In CI you enumerate all queries, and register the query with a central database, which returns the hash
  2. Store the hash against the query locally somewhere, committed in your client repo
  3. at query send the hash instead of the query
  4. all running servers need to be able to lookup the registered query from a central data source

Without some kind of persistent storage, shared by all the servers in the pool, this wont work.

@icco
Copy link
Contributor Author

icco commented Nov 3, 2018

That's how relay works, APQ though is different.

apq

Basically Apollo first hashes the query, and sends the operation name and a hash. If the server knows that hash + operation name combo, it returns the results, otherwise it returns a unique error that says either:

  1. I don't support persisted queries
  2. I haven't stored that hash yet

if #1, the client stops sending hashes. if #2, the client sends the full query, with the hash it expects it to be stored at. There isn't any shared storage between the client and the server.

@vektah
Copy link
Collaborator

vektah commented Nov 3, 2018

Nice pic 👍

Ah, thats interesting. It means more another round trip on the first page load, but if the server has warmed up it should be better. I still think the backend probably needs to be plugable here so if the server can have some persistence so the clients don't get hit with the extra round trip every time.

@ghost
Copy link

ghost commented Nov 23, 2018

Persisted queries are great. Hope gqlgen gets them.

It makes the client code and refactoring in general much smoother.

I was even thinking of checking them at client compile time or at least an integration test at test time.

@dehypnosis
Copy link

this will reduce client's payload amazingly.
i wish someone to contribute for it. I will try when available

@DBL-Lee
Copy link
Contributor

DBL-Lee commented Feb 28, 2019

I think this would be extremely useful. I wonder is this on the roadmap somewhere?

@mtbohm
Copy link

mtbohm commented May 30, 2019

I can confirm this would be great to have. The major benefit for us would be that it allows us to transform complex POST requests into GET requests, which can then be cached by a CDN.

@DBL-Lee DBL-Lee mentioned this issue Jun 1, 2019
2 tasks
@BitPhinix
Copy link

It seems like this feature is now implemented, so should this issue be closed?

@MrSaints
Copy link

As an extension to APQ, it'd be great if APQ can be "enforced". That is, query whitelisting based on hashes. Unless I'm mistaken, the current behaviour would fallback to executing the query if it is not in the cache.

@icco
Copy link
Contributor Author

icco commented Jul 21, 2019

Implemented on my installation, and works great! @MrSaints you should file another bug for a seperate feature request.

@icco icco closed this as completed Jul 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

8 participants