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

Entity cache: support queries with private scope #4855

Merged
merged 23 commits into from
Apr 23, 2024

Conversation

Geal
Copy link
Contributor

@Geal Geal commented Mar 26, 2024

This adds support for caching responses marked with the private scope. For now, this is meant to work only from what the subgraph is returning, without any schema level information.

this encodes the following behaviour:

  • if the subgraph does not return Cache-Control: private for a query, then it works as usual
  • if the subgraph would return Cache-Control: private for a query:
    • if we have seen this query before:
      • if we have configured a private_id option and the related data is found in context provided by the authentication plugin, then we look for the cache entry for this query, by adding a hash of the private_id to the cache key. If not present, we perform the query, and then store the data at this personalized cache key
      • if there is no private_id, the we do not cache the response because we have no way to distinguish users
    • if it is the first time we see this query:
      • first we look into the cache for the basic cache key (without the private_id hash) because we cannot know in advance if it requires private data
      • there should not be a cached entry since we have not seen this query before. Another router instance could have seen it?
      • we send the request to the subgraph and get back a response with Cache-Control: private
      • add the query to the list of known private queries
      • if we have a `private_id:
        • update the cache key to add the hash of the sub claim
        • store the response in cache
      • if there is no private_id: since we don't have a way to differentiate users, then we do not cache the response

Checklist

Complete the checklist (and note appropriate exceptions) before the PR is marked ready-for-review.

  • Changes are compatible1
  • Documentation2 completed
  • Performance impact assessed and acceptable
  • Tests added and passing3
    • Unit Tests
    • Integration Tests
    • Manual Tests

Exceptions

Note any exceptions here

Notes

Footnotes

  1. It may be appropriate to bring upcoming changes to the attention of other (impacted) groups. Please endeavour to do this before seeking PR approval. The mechanism for doing this will vary considerably, so use your judgement as to how and when to do this.

  2. Configuration is an important part of many changes. Where applicable please try to document configuration examples.

  3. Tick whichever testing boxes are applicable. If you are adding Manual Tests, please document the manual testing (extensively) in the Exceptions.

This comment has been minimized.

@router-perf
Copy link

router-perf bot commented Mar 26, 2024

CI performance tests

  • reload - Reload test over a long period of time at a constant rate of users
  • events_big_cap_high_rate_callback - Stress test for events with a lot of users, deduplication enabled and high rate event with a big queue capacity using callback mode
  • events_without_dedup_callback - Stress test for events with a lot of users and deduplication DISABLED using callback mode
  • large-request - Stress test with a 1 MB request payload
  • const - Basic stress test that runs with a constant number of users
  • no-graphos - Basic stress test, no GraphOS.
  • step-jemalloc-tuning - Clone of the basic stress test for jemalloc tuning
  • events - Stress test for events with a lot of users and deduplication ENABLED
  • events_callback - Stress test for events with a lot of users and deduplication ENABLED in callback mode
  • events_big_cap_high_rate - Stress test for events with a lot of users, deduplication enabled and high rate event with a big queue capacity
  • events_without_dedup - Stress test for events with a lot of users and deduplication DISABLED
  • xxlarge-request - Stress test with 100 MB request payload
  • xlarge-request - Stress test with 10 MB request payload
  • step - Basic stress test that steps up the number of users over time

@Geal Geal requested a review from a team April 17, 2024 07:20
@Geal Geal marked this pull request as ready for review April 17, 2024 07:23
different subgraphs might use different options, and the `sub` claim
from the JWT might not be used directly, so we make it configurable
we should be able to store the root response even if we have seen this
query before.
If the subgraph returned no_store, we should srill process all the
entities because we might have obtained part of the data from cache
before
@Geal Geal merged commit 7cf4f45 into dev Apr 23, 2024
14 checks passed
@Geal Geal deleted the geal/entity-cache-private-scope branch April 23, 2024 09:39
abernix added a commit that referenced this pull request May 6, 2024
#4855

Co-authored-by: Edward Huang <edward.huang@apollographql.com>
@BrynCooke BrynCooke mentioned this pull request May 7, 2024
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.

None yet

3 participants