Add valkey cache handler to next app#1210
Conversation
| pingInterval: 10000, | ||
| }); | ||
| redisClient.on("error", (e) => { | ||
| if (typeof process.env.NEXT_PRIVATE_DEBUG_CACHE !== "undefined") { |
There was a problem hiding this comment.
maybe set this to be if (process.env.CACHE_LOG) { ? cuts down on one new env var
There was a problem hiding this comment.
nvm, this seems to be used internally in @neshca/cache-handler as well, let's leave it like this
tefkah
left a comment
tefkah
left a comment
There was a problem hiding this comment.
after testing it a bunch i can confirm it works swimmingly! for some reason it does work in dev for me, but this has always been a bit fickle before this change, sometimes it does use the cache, sometimes it doesn't. shouldnt be relied on probably.
the only thing id ask: could you add a small entry to docs describing that we use valkey for the cache, and how to debug it? (ie by setting NEXT_PRIVATE_CACHE_DEBUG=1)
otherwise great work! it's just as fast if not slightly faster than the file system cache handler we were using, which probably means it's a lot faster on prod.
Ok that makes me feel better about setting it up in dev. I was sure I've experienced caching behavior with the dev server but it prints a pretty definitive message saying the cache won't be used.
Absolutely!
That's great! If sentry doesn't actually give us good performance data in prod, I will manually instrument the cache handler so we can be sure. And even more importantly, this means all of our instances will use the same cache, so I'll bump our instance count up a bit after this is merged. |
3 participants
Issue(s) Resolved
#1131
High-level Explanation of PR
This PR changes core to use valkey as its cache, as opposed to the filesystem. Theoretically it should be automatically instrumented by sentry since we're using a supported redis client.
I haven't yet set a max size, because I'm really not sure what it should be. IMO we shouldn't really worry about that yet, since the large values are a problem for the node server which can't gracefully run out of memory (which we should fix), whereas valkey will simply evict keys. Once we have some data about how the cache is performing, we can tune that configuration more.
Test Plan
I've tested it locally and confirmed that it's writing and reading from the cache by running
redis-cli monitor(you need to use the compiled app, not the dev server, since the dev server doesn't use the cache apparently). I'm not sure how else to test this before prod, but I feel good about it!