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

🗃️ initial pass at a CacheControl document #4009

Closed
wants to merge 1 commit into from

Conversation

martinbonnin
Copy link
Contributor

@martinbonnin martinbonnin commented Apr 8, 2022

See #3566

@martinbonnin martinbonnin requested review from sav007 and BoD as code owners April 8, 2022 11:33
@netlify
Copy link

netlify bot commented Apr 8, 2022

Deploy Preview for apollo-android-docs canceled.

Name Link
🔨 Latest commit 4b553e1
🔍 Latest deploy log https://app.netlify.com/sites/apollo-android-docs/deploys/62501d6f2ec96c00087401fc

extend type Speaker @cacheControl(maxAge: 3600)
```

Or if introspection ever supports directive usages, it could be retrieved (and cached) at runtime.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL that it's not supported 😢


To keep the database size manageable, the Records can be serialized in binary format, skipping the Json overhead and compressing timestamps when the same timestamp is used for multiple fields.

To avoid storing another Long in the DB, it is proposed to define cache control rules in the client. Ideally using directives:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I'm not sure I understand this sentence: we still need to store the dates in the db right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, date but not date + maxAge (so it saves 64bits). I'll update that part.

@martinbonnin martinbonnin deleted the cache-control branch June 22, 2022 11:57

Date sounds required in all cases. MaxAge is required if we want to store that information when coming from the server but could be spared if the rules are defined on the client side.

Because any operation can query any field, this information needs to be stored per-field. This is the hard part. Adding one (or several) 64 bit timestamp for each field might double (or worse) the size of the cache. On the other hand a lot of fields will share the same timestamp, opening opportunities for compression (see #possible-optimisations).
Copy link

@ynkm169 ynkm169 Apr 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this information needs to be stored per-field

Question, does this mean the following code need to store expiry for all 3 fields? (name, bio, twitter) Wouldn't an expiry only on Speaker itself suffice?

type Speaker @cacheControl(maxAge: 3600) {
  name: String!
  bio: String!
  twitter: String
}


- OS I/O cache
- SQLite page cache
- Apollo Memory cache
Copy link

@ynkm169 ynkm169 Apr 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clarification question:
If we don't use SQL cache at all, will we be able to use expiry with just the memory cache?

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.

3 participants