apollo-server: apollo-server-plugin-response-cache: cache hint not working
My apollo graphql server setup uses the following:
"apollo-datasource-rest": "^0.6.9",
"apollo-server-express": "^2.9.12",
"apollo-server-plugin-response-cache": "^0.3.8",
My schema:
type Query {
types: [Type!]! @cacheControl(maxAge: 3600)
}
type Type {
id: ID!
label: String!
active: Boolean!
}
My responseCachePlugin config:
responseCachePlugin({
sessionId: (requestContext) => (requestContext.request.http.headers.get('sessionId') || null),
})
Neither the static cache hint like the above nor the dynamic cache hint in the resolver work. i.e., they don’t cache the response from the datasource (REST API).
A bunch of other devs reported this issue on Spectrum. One of the Spectrum posts points to the problem:
The property requestContext.overallCachePolicy doesn't get updated by the cache hint, nither the static one in schema or the dynamic one ...
The value is always the cacheControl you configured when creating ApolloServer
github link to the line in the code where there is a potential problem: https://github.com/apollographql/apollo-server/blob/1acf62d903253f5669c0c4f7255c8a87b95b5e6c/packages/apollo-server-plugin-response-cache/src/ApolloServerPluginResponseCache.ts#L243
Spectrum post that details the above issue: https://spectrum.chat/apollo/apollo-server/apollo-server-plugin-response-cache-is-not-working-properly~e3390cd4-9e3f-4919-88b4-92508e235e38
Other Spectrum posts that talk about this problem:
- https://spectrum.chat/apollo/apollo-server/cachecontrol-does-not-work-unless-defaultmaxage-is-set~0e4dc3c5-7cdc-40be-b833-0b2d436d1288
- https://spectrum.chat/apollo/apollo-server/does-rediscache-work-in-dev-mode~18358268-8d11-4b13-a0c9-abddfb6623f3
cc to: @trevor-scheer
About this issue
- Original URL
- State: closed
- Created 5 years ago
- Reactions: 37
- Comments: 64 (17 by maintainers)
Commits related to this issue
- Implement `@cacheControl(noDefaultMaxAge: true)` Previously, the cache control logic treats root fields and fields that return object or interface types which don't declare `maxAge` specially: they a... — committed to apollographql/apollo-server by glasser 3 years ago
- Implement `@cacheControl(noDefaultMaxAge: true)` Previously, the cache control logic treats root fields and fields that return object or interface types which don't declare `maxAge` specially: they a... — committed to apollographql/apollo-server by glasser 3 years ago
- Implement `@cacheControl(noDefaultMaxAge: true)` Previously, the cache control logic treats root fields and fields that return object or interface types which don't declare `maxAge` specially: they a... — committed to apollographql/apollo-server by glasser 3 years ago
- Implement `@cacheControl(noDefaultMaxAge: true)` Previously, the cache control logic treats root fields and fields that return object or interface types which don't declare `maxAge` specially: they a... — committed to apollographql/apollo-server by glasser 3 years ago
- Implement `@cacheControl(noDefaultMaxAge: true)` Previously, the cache control logic treats root fields and fields that return object or interface types which don't declare `maxAge` specially: they a... — committed to apollographql/apollo-server by glasser 3 years ago
- Implement `@cacheControl(noDefaultMaxAge: true)` Previously, the cache control logic treats root fields and fields that return object or interface types which don't declare `maxAge` specially: they a... — committed to apollographql/apollo-server by glasser 3 years ago
- Implement `@cacheControl(inheritMaxAge: true)` (#5247) Previously, the cache control logic treats root fields and fields that return object or interface types which don't declare `maxAge` specially:... — committed to apollographql/apollo-server by glasser 3 years ago
It’s disappointing the cache only works if you set a global caching settings:
I am using
merge-graphql-schemas
with Redis and I am unable to get anything cached without the global settings.I think it is a great idea to
@cacheControl
in the schema but it would be even better it it works first.It is documented correctly but the result is not something usable. If you can’t set a overall cache, the cache doesn’t work at all. This is basically the issue. I wonder if someone from Apollo actively looks at the issues?
Applying this diff to your repo and running your curl command, the header is set to 100.
The point is that passing
true
for cacheControl enables a backwards-compatibility mode that you don’t want, but just leaving it out (or specifying an object with other options in it) gives the result you want.Definitely a lesson here about the naming of boolean options: we should have been more thoughtful and named it
includeCacheControlExtension
orcacheControl: {includeExtension}
.This is a bug. The overall cache policy is documented correctly, yes, however the documentation is very specific that the default is that things are uncacheable:
The current behavior is that
@cache-control
doesn’t work, period, unless you also set adefaultMaxAge
. That means you have to explicitly annotate every type you have with@cache-control
and then setdefaultMaxAge
to something enormous, because of the overall cache policy logic.I will say that the overall design approach here is fantastic! This seems like it should be an easy thing to fix frankly.
I tried without defaultMaxAge and caching is working for me. I noticed that if we are not using cache control hints properly then it looks like it is not working. Make sure that cache hint is applied to a field and its nested non scalar fields as well then only caching will work. Otherwise when it finds nested non scalar field is missing cache control so it tries to use defaultMaxAge and which is 0 by default. Hence, it looks like caching is not working.
Some points to be noted
Facing similar issue, when I use caching at the Apollo server level then everything is cached I can see no DB queries in log after the first request. Caching seems to be working
But when I try the fine grain caching at resolvers I can see the maxAge set in HTTP response but the response doesn’t seem to be cached as I can see DB queries on each request
Am I doing anything wrong here?
It is how it should work but now
someField
query is cached also for 5 sec - not for 50 sec. Just tested it.Btw it would be also great to have ability to set default scope on all queries to
PRIVATE
. In my app they all should be ‘PRIVATE’ so I need to mark each individually to be ‘PRIVATE’ instead of setting it once eg like this:@glasser Thanks for the reply. I wasn’t testing field level caching, so I’m not sure on that. I will try to test it later in my schema and let you know the result. My object example would be like this:
In the above case, with defaultMaxAge=5, if you fetch
User
by id, it will be cached for 5 seconds, not 50.Really I just want it to not use defaultMaxAge, at all, if a cache hint has been set, either via
@cache-control
or dynamically. To me default means default, i.e. use this if there is no other value. Instead the way it works in my testing is, override cache hint value with defaultMaxAge if it is less than what is currently computed, i.e. defaultMaxAge is the “uber parent” of the object graph. I think that’s what is throwing people for a loop here.And sorry yes let me try to get you an example git repo later this week, I know this is difficult without seeing a concrete thing that doesn’t work as expected. 😃
In case you want to save the effort of setting up Redis, getting the config working etc, you may wanna have a look at https://graphcdn.io. I’m building that right now, especially to make caching easier, as I struggled with it myself before.
Absolutely, I couldn’t agree more. It’s designed perfectly and we hope it gets fixed. I’ll send this thread to the Apollo support team in several channels. Hopefully we get an answer soon.
I also noticed that only the first query has the right cacheHint:
first request responses with
cache-control: max-age=60, public
second request responses withcache-control: max-age=3600, public
it seems like apollo server caches the cache hints as well!
Edit:
its clear. Not setting defaultMaxAge --> caching DOES NOT WORK AT ALL you have to set defaultMaxAge to something huge, because it defines the maximum caching time, not the default caching time, when no other cache hint is available.
But setting cacheHints dynamically for every resolver also does not work properly. As mentioned above it only works shortly. After further investigation it seems that it works for the given cachehint time, after that it falls back to the global setting.
Something is totally wrong in the cache logic and its not only the flawed design of defaultMaxAge
Edit2: its really confusing to debug.
But it seems clear that resolver-level cache hints get lost when it tries to use the cache.
Edit3: it had something to do how we set cachehints dynamically, i think i solved it.
But the one flaw remains: defaultMaxAge is the maximum TTL that you can set. if you omit it, no resolver will get cached
After reading through parts of this thread I’m still pretty confused, and I can’t tell if Apollo Server is not working at it should; or else if I just don’t understand what it’s supposed to do.
To clarify, if I add this at the top of a resolver:
I would expect subsequent queries to that resolver to not execute the
console.log
for the next five minutes? If so then that’s not what I’m seeing currently.To follow up on my last post, I only just now finally understood that this is about setting cache hints so that Apollo can figure out whether the entire request is cacheable or not; and not telling Apollo to cache individual fields.
In other words, if I have 9 cached fields and 1 uncacheable fields in my request I falsely expected after reading the documentation that those 9 fields would be cached and not trigger database, API, etc. requests. Whereas the truth (as I now understand it, I might still be wrong?) is that none of my 10 fields will be cached because the entire request is not cacheable.
On the other hand, if I used Datasources for those 9 fields then those database, API, etc. requests would be cached independently of whether the full GraphQL can be cached or not.
Again sorry if I’m wrong and just making things more confusing, but I suspect other people might’ve misunderstood this as well.
I’m seeing exactly this problem also. I was working on adding response caching today and couldn’t figure out why I wasn’t seeing any caching happening. I’m glad I’m not going mad but also disappointed that response caching is apparently broken 😦
Here’s an implementation of something that’s similar to the
inherit
concept I discussed above: https://github.com/apollographql/apollo-server/pull/5247 I’m interested in feedback from folks who were interested in this issue!I only want to be able to cache things that have explicit cache hints, without defining the defaultMaxAge. cache hints i define in resolvers don’t work without defaultMaxAge, and when I define it, apollo server automatically caches things I don’t want it to cache (even without hints) which results in unexpected behaviors on the front end.
How do I only cache things that have explicit hints, and disable automatic root object caching? It is not possible to do that?
@glasser I might be wrong but I get the impression this is maybe more an issue with unclear documentation than an actual bug? I haven’t had time to revisit this issue but now I just learned about datasources and I feel like that’s one more caching parameter I need to juggle with… I think a comprehensive blog post or example app that clearly outlines the different caching methods available with Apollo Server and the pros and cons of each one would be helpful here.
Anyone figure it out how to make cache control works? Im also with the same situation
Oh, does this mean that setting the metadata by itself should not be expected to actually cache the data? If so that would explain why I didn’t notice any difference in my implementation. If that’s the case maybe the docs could state this explicitly, it might be confusing for others as well?
I think that I have the same issue with the
cache-control
header not set whendefaultMaxAge
is not defined. @glasser here is a repo with a detailed reproduction recipe. Let me know if something is not clear.https://github.com/mthenw/apollo-cache-issue
@glasser I’m not sure exactly what you need for a self-contained reproduction, but I feel my initial comment on expected vs actual behaviour describes the issue most here are experiencing:
If I can offer anything further that might help, please let me know.
@Jylth agree that the logic is not what you’d expect, but is technically documented correctly (but could probably be improved):
My initial expectation when testing this kind of setup:
would be for Destination.distance to cache for 60 seconds, and Destination.name to cache for 10. But the result is that the entire Destination type caches for the minimum maxAge across all fields, in this case 10.
HUGE THANKS ankuradhey!!! Your comment lightened up my implementation! So… if we want defaultMaxAge: 0, than we need to explicitly define the ‘cacheControl(maxAge: XXX)’ directive to all non scalar fields of the respected query or mutation. Once I did that, the all the fields marked with the directive started to respect the the defined maxAge and all the other fields (non marked) remained with 0 caching time.
Hi @SachaG !
As mentioned above it is very challenging for me to provide support on this feature just based on single line snippets, rather than complete examples.
apollo-server-plugin-response-cache
is a full response cache that caches full GraphQL responses if every field queried in it is cacheable. So if a query asks for a cacheable field and an uncacheable field, it will re-run the resolver for the cacheable field too because the whole response is not cacheable.Much of the confusion in this PR is about what the definition of a field being marked as cacheable is, because the default behavior (while internally consistent and accurately documented, AFAIK) is surprising and perhaps not ideal.
@mthenw: According to the docs at https://www.apollographql.com/docs/apollo-server/api/apollo-server/
As documented,
cacheControl: true
literally just includes this somewhat useless metadata in the GraphQL response. This was something we implemented before we implemented writing caching HTTP response headers in apollo-server, so we wanted to keep its behavior consistent even as we changed the default for users who don’t specifycacheControl
at all. (Additionally, this value was intended for folks running behind the old binary engineproxy, which I believe calculated its own HTTP response headers based on this metadata, so having two separate sets of calculation seemed like a bad plan.)The logic in the code is:
Are there docs somewhere that encouraged you to set
cacheControl: true
that we should fix?My solution (for now) was to make custom @CacheControl decorator (with type-graphql):
import { UseMiddleware } from 'type-graphql';
@CacheControl({ maxAge: 60 })
Obviously
scope: private/public
is missing fromcontext.cacheOptions
but many times settingttl
is enough.Tested only with
type-graphql
-moduleThat is not my understanding of how defaultMaxAge works (though of course, there may be a bug, that’s why we’re here).
If you set defaultMaxAge to 5 but have a schema like
Then queries against someField will be cached for 50, queries against someOtherField will be cached for 5, and queries that hit both fields will also be cached for 5 because they contain some low-TTL data. Is that not what you’re experiencing?
The main feature that I think is missing here (other than features that would make this model more comprehensible/inspectable) is the ability to mark an object/interface field as “inherit parent maxAge” just like scalar fields do.
But I don’t understand how your example differs from your example without setting defaultMaxAge (unless this is what helps us narrow down what the bug is!). I am really looking forward to the clear example of this bug given by somebody on this thread!
@jbarefoot How is that better than setting defaultMaxAge to 60?
@glasser That’s much appreciated, I’ll try to find some time soon to create a basic repo that illustrates the issue. In the meantime, I implemented a workaround which I’ll post here for others. The workaround is to use a wrapper function around every resolver function, that checks for a cache hint and adds it if it doesn’t exist. As long as you remember to use the wrapper on every query resolver, it works fine. Then you set
defaultMaxAge
to something really high, like a day, so it will never be the minimum in the overall computed cache policy.