next.js
579ff601 - example(cache-handler-redis): fix connection leak, hangs on a down Redis, and cross-page tag invalidation (#98716)

Commit
32 days ago
example(cache-handler-redis): fix connection leak, hangs on a down Redis, and cross-page tag invalidation (#98716) Three runtime problems in `examples/cache-handler-redis`. ### 1. One new Redis connection per request, never closed Next.js constructs the singular `cacheHandler` class once per request (`new CurCacheHandler(...)` in `IncrementalCache`, which `route-module.ts` creates per request). The example's constructor calls `createClient()` + `connect()`, so every request opens a connection that is never closed. ``` $ redis-cli client list | wc -l # before 102 $ for i in $(seq 1 10); do curl -s -o /dev/null localhost:3000/cet; done $ redis-cli client list | wc -l 132 # +3 per request ``` With Redis' default `maxclients 10000`, a single instance runs out after a few thousand requests. ### 2. Requests hang while Redis is unavailable The README says the handlers "degrade gracefully when Redis is unavailable, so the app still builds and runs, just without a shared cache". At runtime they don't: with Redis stopped, every request on every instance blocks until Redis comes back. ``` $ docker stop cache-handler-redis $ curl -s -o /dev/null -m 60 -w "%{http_code} %{time_total}s\n" localhost:3000/cet 000 60.007305s # (uncapped: 188s, returned the moment Redis was started again) ``` Same when the app is started while Redis is down. Cause: node-redis keeps retrying in the background and `client.connect()` does not settle until a connection succeeds. Isolated: ```js const c = createClient({ url: "redis://localhost:6379" }); c.on("error", () => {}); await Promise.race([c.connect(), new Promise(r => setTimeout(() => r("pending"), 15000))]); // -> "pending" after 15s, isOpen=true isReady=false (redis@6.2.1) ``` Because each request built a new handler, each request awaited a fresh, never-settling `connect()` in `getClient()`. `remote-cache-handler.js` has one module-level client, but its `getClient()` awaits the same promise, so it hangs the same way if the app starts while Redis is down. ### 3. `updateTag` never reaches other pages' remote entries The remote handler's `get` never checks the entry's own tags. Next.js only passes soft tags to `getExpiration`, and the `"use cache"` wrapper only knows about tags revalidated in the current request, so `updateTag("time-data")` from `/cet` left the `/gmt` entry stale on every instance, including the one that ran it, until it expired (`cacheLife` `expire: 3600`). The [`cacheHandlers` docs](https://nextjs.org/docs/app/api-reference/config/next-config-js/cacheHandlers#get) say `get` should report an entry whose tag was invalidated as missing or stale. ### Fix - Hoist the client in `cache-handler.js` to module scope (the pattern `remote-cache-handler.js` already uses). - `getClient()` awaits the connect promise raced against a 1s `unref()`'d timer (suggested in review), then returns the client when `isReady` and `null` otherwise. Requests during startup still wait for the connection, and a down Redis costs one bounded wait instead of blocking every request. The client keeps retrying and `isReady` flips back on its own. - `disableOfflineQueue: true`, so a command issued while the connection is down rejects immediately (`ClientOfflineError`) instead of being queued for the 5s command timeout. - Entry `get` / `set` catch the Redis call only and degrade to a miss. - Remote `get` compares the entry's tags against their revalidation timestamps (one `MGET`) and misses when any is newer, the same comparison as the built-in handler. `getExpiration` returns `Date.now()` when Redis can't answer, so the entry is discarded rather than served. - `revalidateTag` / `updateTags` throw when Redis isn't ready, so an invalidation that never reached Redis surfaces as an error instead of a success whose entries come back once Redis does. ### After Two `next start` instances on one Redis, same script on `next@16.3.5` and `16.4.0-canary.35` (identical results), canary's handlers vs this PR: | scenario | before | after | | --- | --- | --- | | `updateTag` from `/cet`, read `/gmt` on both instances | stale | refreshed | | `revalidateTag("time-data", "max")` | neither page refreshed | both refreshed | | tag lookup fails | 500 | 200, regenerated | | request while Redis is stopped | no response | 200, uncached | | `updateTag` while Redis is stopped | no response | 500, error logged | | app started without Redis | no response | first ISR read waits ≤1.1s once | | Redis connections, 200 requests | 173 → 773 | 7 → 7 | | `/cet` under load (autocannon, 10 connections × 10s, `16.3.5`) | 332 req/s, hits Redis `maxclients` | 1,106 req/s median, p99 20ms, 3 connections | | 10 readers during 20 `updateTag`s: reads served a value from before a completed invalidation | 3,089 of 3,089 | 0 of 33,899 | | Redis down ~4s under load | timeouts, caching doesn't come back | 0 errors, caching resumes | The tag check costs one Redis round trip per remote hit (−9 to −13% on a route that only reads one remote entry). Handler-level cases, full tables and the benchmark breakdown are in [this comment](https://github.com/vercel/next.js/pull/98716#issuecomment-5724918096).
Author
Parents
Loading