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

Apollo caching everything when defaultMaxAge: 99, even fields without @cacheControl #3955

Closed
leonardoanalista opened this issue Apr 7, 2020 · 5 comments

Comments

@leonardoanalista
Copy link

leonardoanalista commented Apr 7, 2020

https://www.apollographql.com/docs/apollo-server/performance/caching/#the-overall-cache-policy

I am using apollo-server-cache-redis and apollo-server-plugin-response-cache to save full responses to Redis. I am not sure if we can run Redis in codesandbox.io but it is pretty easy to understand the problem.

Here is a sample types to illustrate the problem:

cacheControl: {
    defaultMaxAge: 60
},

type User @cacheControl(maxAge: 100) {
    name: String! 
    distance: Float!
}

type Product {
    name: String!
    description: String!
}

Problem 1: Cache does not work if defaultMaxAge is not specified.
All @cacheControl specified do absolutely nothing - nada in Redis cache.

Problem 2: Apollo not respecting default maxAge for nonScalar types.
Redis is caching Product (basically everything), even when there is no cacheControl on Product type. It is using defaultMaxAge.
According to the docs, all non scalar fields have maxAge: 0 and overall cache is the lowest value.
Since Procuct maxAge is 0, and it is lower than defaultMaxAge, the overall cache should be 0 for Product type.

Consequence: Apollo is caching everything, even private data or any type in any server response. I think the cache should be an opt-in.
Cache in type User is never respected as defaultMaxAge is always lower than 100.

The docs are probably correct regarding to problem 1 but we really should think how people will use this cache.

I am happy yo create a codesandbox if required.

This issue is too messy at this stage: #3559

@glasser
Copy link
Member

glasser commented Apr 14, 2020

According to the docs, all non scalar fields have maxAge: 0

Nope — according to the docs you linked (though it could be more clear), non scalar fields have maxAge 0 unless defaultMaxAge is specified. Specifically, root and non-scalar fields have maxAge=defaultMaxAge unless they specify their own maxAge, and the default value of defaultMaxAge is 0.

If you do not want to cache everything, don't set defaultMaxAge, which is documented as "when getting started with the cache control API".

Your problem 1 seems a lot like #3559, where I have been continually asking for somebody to show me a fully executable reproduction and nobody has been able to. Please do, if you can! Setting up redis may be hard, but you should be able to use InMemoryLRUCache instead?

So this seems like a duplicate of #3559.

@glasser glasser closed this as completed Apr 14, 2020
@leonardoanalista
Copy link
Author

It looks like default in memory cache is working as expected.
I was looking at the Redis cache and saw things getting cached for queries that didn't have any cache control configured, event without defaultMaxAge configured.

Here is the example:
https://codesandbox.io/s/apollo-server-example-8d4bp?file=/index.js

Is there a way to see what's inside the memory cache?

Thanks @glasser cache works as expected in Memory. I'll check it again in my local Redis but I believe it will work as expected.

@glasser
Copy link
Member

glasser commented Apr 15, 2020

Hi, can you explain exactly what I'm supposed to do to interact with your example? what to click, what queries to run, what to expect to see, what I actually see?

I also don't see where it uses the Redis cache at all?

Or are you saying there is no problem in that example?

@leonardoanalista
Copy link
Author

This example is to prove there is nothing wrong with the Apollo cache (at least in memory).

My next steps (in a few days due to personal commitments):

  • I am going to try in my local env with Redis, cache full response

  • Scenario 1 (problem 1): no defaultMaxAge and @cacheControl in one type:

    • expected: only the type with @cacheControl should be in Redis, all other types or queries with no @CacheControl should NOT be in Redis
  • Problem 2 - INVALID, misinterpretation of the docs.

I'll open a new issue case I am able to reproduce. If so, I'll spin up a public Redis so we can see the cached results.

@glasser
Copy link
Member

glasser commented Apr 15, 2020

Thanks. Please @-mention me when you have more information as I don't read all apollo-server issues.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 20, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants