From f2986b45c3539b47a449740c2568eabfd9248b56 Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Wed, 30 Apr 2025 11:28:15 -0500 Subject: [PATCH 01/15] Reapply "Add valkey cache handler to next app (#1210)" (#1215) This reverts commit 0951e72a46cf3de7d957006a1c1a408ce13fe942. --- core/.env.development | 2 +- core/cache-handler.mjs | 95 ++++++++++++++++++++--- core/instrumentation.node.mts | 5 ++ core/package.json | 3 + docker-compose.dev.yml | 2 + package.json | 2 + pnpm-lock.yaml | 139 ++++++++++++++++++++++++++++++++-- 7 files changed, 233 insertions(+), 15 deletions(-) diff --git a/core/.env.development b/core/.env.development index fc87fb9514..8fb25f5443 100644 --- a/core/.env.development +++ b/core/.env.development @@ -25,4 +25,4 @@ DATACITE_PASSWORD="" DATACITE_API_URL="https://api.test.datacite.org" GCLOUD_KEY_FILE='xxx' -VALKEY_URL='redis://cache:6379' \ No newline at end of file +VALKEY_URL='redis://localhost:6379' \ No newline at end of file diff --git a/core/cache-handler.mjs b/core/cache-handler.mjs index ea52f41d35..0d1c477c4f 100644 --- a/core/cache-handler.mjs +++ b/core/cache-handler.mjs @@ -1,11 +1,88 @@ -/** - * Solution taken from here: https://github.com/vercel/next.js/discussions/48324#discussioncomment-10542097 - * - * We are reexporting `next.js`'s default cache handler to get around - * the fixed 2mb limit for cached fetches. - * We run into this limit somewhat quickly if we fetch more than 100 pubs. - */ +// Based on https://github.com/fortedigital/nextjs-cache-handler#full-example -import FileSystemCache from "next/dist/server/lib/incremental-cache/file-system-cache.js"; +import { PHASE_PRODUCTION_BUILD } from "next/constants.js"; +import createBufferStringHandler from "@fortedigital/nextjs-cache-handler/buffer-string-decorator"; +import { Next15CacheHandler } from "@fortedigital/nextjs-cache-handler/next-15-cache-handler"; +import createRedisHandler from "@fortedigital/nextjs-cache-handler/redis-strings"; +import { CacheHandler } from "@neshca/cache-handler"; +import createLruHandler from "@neshca/cache-handler/local-lru"; +import { createClient } from "redis"; -export default FileSystemCache; +// Usual onCreation from @neshca/cache-handler +CacheHandler.onCreation(() => { + // Important - It's recommended to use global scope to ensure only one Redis connection is made + // This ensures only one instance get created + if (global.cacheHandlerConfig) { + return global.cacheHandlerConfig; + } + + // Important - It's recommended to use global scope to ensure only one Redis connection is made + // This ensures new instances are not created in a race condition + if (global.cacheHandlerConfigPromise) { + return global.cacheHandlerConfigPromise; + } + + // Main promise initializing the handler + global.cacheHandlerConfigPromise = (async () => { + /** @type {import("redis").RedisClientType | null} */ + let redisClient = null; + if (PHASE_PRODUCTION_BUILD !== process.env.NEXT_PHASE) { + try { + redisClient = createClient({ + url: process.env.VALKEY_URL, + pingInterval: 10000, + }); + redisClient.on("error", (e) => { + if (typeof process.env.NEXT_PRIVATE_DEBUG_CACHE !== "undefined") { + console.warn("Redis error", e); + } + global.cacheHandlerConfig = null; + global.cacheHandlerConfigPromise = null; + }); + } catch (error) { + console.warn("Failed to create Redis client:", error); + } + } + + if (redisClient) { + try { + console.info("Connecting Redis client..."); + await redisClient.connect(); + console.info("Redis client connected."); + } catch (error) { + console.warn("Failed to connect Redis client:", error); + await redisClient + .disconnect() + .catch(() => + console.warn("Failed to quit the Redis client after failing to connect.") + ); + } + } + const lruCache = createLruHandler(); + + if (!redisClient?.isReady) { + console.error("Failed to initialize caching layer."); + global.cacheHandlerConfigPromise = null; + global.cacheHandlerConfig = { handlers: [lruCache] }; + return global.cacheHandlerConfig; + } + + const redisCacheHandler = createRedisHandler({ + client: redisClient, + keyPrefix: "nextjs:", + keyExpirationStrategy: "EXAT", + }); + + global.cacheHandlerConfigPromise = null; + + global.cacheHandlerConfig = { + handlers: [createBufferStringHandler(redisCacheHandler)], + }; + + return global.cacheHandlerConfig; + })(); + + return global.cacheHandlerConfigPromise; +}); + +export default new Next15CacheHandler(); diff --git a/core/instrumentation.node.mts b/core/instrumentation.node.mts index 56f33a1524..3efb12f679 100644 --- a/core/instrumentation.node.mts +++ b/core/instrumentation.node.mts @@ -19,6 +19,11 @@ if (env.NODE_ENV === "production") { // Setting this option to true will print useful information to the console while you're setting up Sentry. debug: false, + integrations: [ + Sentry.redisIntegration({ + cachePrefixes: ["nextjs:"], + }), + ], }); logger.info("✅ Successfully instrumented Sentry"); } diff --git a/core/package.json b/core/package.json index 2382dfa78a..a27db56d05 100644 --- a/core/package.json +++ b/core/package.json @@ -62,11 +62,13 @@ "@dnd-kit/sortable": "^8.0.0", "@dnd-kit/utilities": "^3.2.2", "@faker-js/faker": "^9.0.0", + "@fortedigital/nextjs-cache-handler": "^1.2.0", "@googleapis/drive": "^8.16.0", "@handlewithcare/react-prosemirror": "catalog:", "@honeycombio/opentelemetry-node": "catalog:", "@hookform/resolvers": "catalog:", "@icons-pack/react-simple-icons": "^10.2.0", + "@neshca/cache-handler": "^1.9.0", "@nimpl/getters": "^2.0.0", "@node-rs/argon2": "^1.8.3", "@opentelemetry/auto-instrumentations-node": "catalog:", @@ -125,6 +127,7 @@ "react-hook-form": "catalog:", "react-markdown": "^9.0.1", "reactflow": "^11.10.4", + "redis": "^4.7.0", "rehype": "^13.0.2", "rehype-format": "^5.0.0", "rehype-parse": "^9.0.1", diff --git a/docker-compose.dev.yml b/docker-compose.dev.yml index 8d9843816f..ed6bc530b6 100644 --- a/docker-compose.dev.yml +++ b/docker-compose.dev.yml @@ -45,6 +45,8 @@ services: service: cache networks: - app-network + ports: + - 6379:6379 minio-init: env_file: diff --git a/package.json b/package.json index 3f8466deee..7edaf9ab9c 100644 --- a/package.json +++ b/package.json @@ -27,6 +27,8 @@ "dev:inbucket:stop": "docker compose -f docker-compose.dev.yml down inbucket", "dev:minio:start": "docker compose -f docker-compose.dev.yml up minio -d && docker compose -f docker-compose.dev.yml run minio-init", "dev:minio:stop": "docker compose -f docker-compose.dev.yml down minio", + "dev:cache:start": "docker compose -f docker-compose.dev.yml up cache -d", + "dev:cache:stop": "docker compose -f docker-compose.dev.yml down cache", "dev:setup": "pnpm install && docker compose -f docker-compose.dev.yml up -d && pnpm p:dev && pnpm --filter core migrate-dev && pnpm --filter core reset", "dev:teardown": "docker compose -f docker-compose.dev.yml down -v", "integration:setup": "docker compose -f docker-compose.test.yml --profile integration up -d", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 8f71c72c1e..4bb8733df9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -272,6 +272,9 @@ importers: '@faker-js/faker': specifier: ^9.0.0 version: 9.0.0 + '@fortedigital/nextjs-cache-handler': + specifier: ^1.2.0 + version: 1.2.0(next@15.2.3(@babel/core@7.25.2)(@opentelemetry/api@1.9.0)(@playwright/test@1.51.0)(react-dom@19.0.0(react@19.0.0))(react@19.0.0))(redis@4.7.0) '@googleapis/drive': specifier: ^8.16.0 version: 8.16.0 @@ -287,6 +290,9 @@ importers: '@icons-pack/react-simple-icons': specifier: ^10.2.0 version: 10.2.0(react@19.0.0) + '@neshca/cache-handler': + specifier: ^1.9.0 + version: 1.9.0(next@15.2.3(@babel/core@7.25.2)(@opentelemetry/api@1.9.0)(@playwright/test@1.51.0)(react-dom@19.0.0(react@19.0.0))(react@19.0.0))(redis@4.7.0) '@nimpl/getters': specifier: ^2.0.0 version: 2.0.0(next@15.2.3(@babel/core@7.25.2)(@opentelemetry/api@1.9.0)(@playwright/test@1.51.0)(react-dom@19.0.0(react@19.0.0))(react@19.0.0))(react-dom@19.0.0(react@19.0.0))(react@19.0.0) @@ -461,6 +467,9 @@ importers: reactflow: specifier: ^11.10.4 version: 11.11.4(@types/react@19.0.6)(react-dom@19.0.0(react@19.0.0))(react@19.0.0) + redis: + specifier: ^4.7.0 + version: 4.7.0 rehype: specifier: ^13.0.2 version: 13.0.2 @@ -3105,6 +3114,12 @@ packages: '@formatjs/intl-localematcher@0.6.0': resolution: {integrity: sha512-4rB4g+3hESy1bHSBG3tDFaMY2CH67iT7yne1e+0CLTsGLDcmoEWWpJjjpWVaYgYfYuohIRuo0E+N536gd2ZHZA==} + '@fortedigital/nextjs-cache-handler@1.2.0': + resolution: {integrity: sha512-dHu7+D6yVHI5ii1/DgNSZM9wVPk8uKAB0zrRoNNbZq6hggpRRwAExV4J6bSGOd26RN6ZnfYaGLBmdb0gLpeBQg==} + peerDependencies: + next: '>=13.5.1' + redis: '>=4.6' + '@googleapis/drive@8.16.0': resolution: {integrity: sha512-Xi2mMrUTQ+gsfyouRGd0pfnL+jjg4n4sjKsJruM1y4DknuRfdSBTk5E//WrL0YJ/CqpcBgyd7L8DvaPRtxZD3Q==} engines: {node: '>=12.0.0'} @@ -3563,6 +3578,12 @@ packages: '@napi-rs/wasm-runtime@0.2.4': resolution: {integrity: sha512-9zESzOO5aDByvhIAsOy9TbpZ0Ur2AJbUI7UT73kcUTS2mxAMHOBaa1st/jAymNoCtvrit99kkzT1FZuXVcgfIQ==} + '@neshca/cache-handler@1.9.0': + resolution: {integrity: sha512-dh0x4pdjDKvPRfZF5DZb8TtOUkbBfeTodOUdQsHDuv0oiuqQ3p7GLx38f6bPn8Sa4he8HsWo+rM4S20ZRqr7pA==} + peerDependencies: + next: '>= 13.5.1 < 15' + redis: '>= 4.6' + '@next/env@15.0.4': resolution: {integrity: sha512-WNRvtgnRVDD4oM8gbUcRc27IAhaL4eXQ/2ovGbgLnPGUvdyDr8UdXP4Q/IBDdAdojnD2eScryIDirv0YUCjUVw==} @@ -5650,6 +5671,35 @@ packages: react: '>=17' react-dom: '>=17' + '@redis/bloom@1.2.0': + resolution: {integrity: sha512-HG2DFjYKbpNmVXsa0keLHp/3leGJz1mjh09f2RLGGLQZzSHpkmZWuwJbAvo3QcRY8p80m5+ZdXZdYOSBLlp7Cg==} + peerDependencies: + '@redis/client': ^1.0.0 + + '@redis/client@1.6.0': + resolution: {integrity: sha512-aR0uffYI700OEEH4gYnitAnv3vzVGXCFvYfdpu/CJKvk4pHfLPEy/JSZyrpQ+15WhXe1yJRXLtfQ84s4mEXnPg==} + engines: {node: '>=14'} + + '@redis/graph@1.1.1': + resolution: {integrity: sha512-FEMTcTHZozZciLRl6GiiIB4zGm5z5F3F6a6FZCyrfxdKOhFlGkiAqlexWMBzCi4DcRoyiOsuLfW+cjlGWyExOw==} + peerDependencies: + '@redis/client': ^1.0.0 + + '@redis/json@1.0.7': + resolution: {integrity: sha512-6UyXfjVaTBTJtKNG4/9Z8PSpKE6XgSyEb8iwaqDcy+uKrd/DGYHTWkUdnQDyzm727V7p21WUMhsqz5oy65kPcQ==} + peerDependencies: + '@redis/client': ^1.0.0 + + '@redis/search@1.2.0': + resolution: {integrity: sha512-tYoDBbtqOVigEDMAcTGsRlMycIIjwMCgD8eR2t0NANeQmgK/lvxNAvYyb6bZDD4frHRhIHkJu2TBRvB0ERkOmw==} + peerDependencies: + '@redis/client': ^1.0.0 + + '@redis/time-series@1.1.0': + resolution: {integrity: sha512-c1Q99M5ljsIuc4YdaCwfUEXsofakb9c8+Zse2qxTadu8TalLXuAESzLvFAvNVbkmSlvlzIQOLpBCmWI9wTOt+g==} + peerDependencies: + '@redis/client': ^1.0.0 + '@remirror/core-constants@3.0.0': resolution: {integrity: sha512-42aWfPrimMfDKDi4YegyS7x+/0tlzaqwPQCULLanv3DMIlu96KTJR0fM5isWX2UViOqlGnX6YFgqWepcX+XMNg==} @@ -5846,6 +5896,11 @@ packages: cpu: [x64] os: [linux] + '@rollup/rollup-linux-x64-gnu@4.40.1': + resolution: {integrity: sha512-XiK5z70PEFEFqcNj3/zRSz/qX4bp4QIraTy9QjwJAb/Z8GM7kVUsD0Uk8maIPeTyPCP03ChdI+VVmJriKYbRHQ==} + cpu: [x64] + os: [linux] + '@rollup/rollup-linux-x64-musl@4.21.2': resolution: {integrity: sha512-7twFizNXudESmC9oneLGIUmoHiiLppz/Xs5uJQ4ShvE6234K0VB1/aJYU3f/4g7PhssLGKBVCC37uRkkOi8wjg==} cpu: [x64] @@ -7931,6 +7986,10 @@ packages: resolution: {integrity: sha512-eYm0QWBtUrBWZWG0d386OGAw16Z995PiOVo2B7bjWSbHedGl5e0ZWaq65kOGgUSNesEIDkB9ISbTg/JK9dhCZA==} engines: {node: '>=6'} + cluster-key-slot@1.1.2: + resolution: {integrity: sha512-RMr0FhtfXemyinomL4hrWcYJxmX6deFdCxpJzhDttxgO1+bcCnkk+9drydLVDmAMG7NE6aN/fl4F7ucU/90gAA==} + engines: {node: '>=0.10.0'} + cmdk@1.0.4: resolution: {integrity: sha512-AnsjfHyHpQ/EFeAnG216WY7A5LiYCoZzCSygiLvfXC3H3LFGCprErteUcszaVluGOhuOTbJS3jWHrSDYPBBygg==} peerDependencies: @@ -9073,6 +9132,10 @@ packages: resolution: {integrity: sha512-Jh/AIwwgaxan+7ZUUmRLCjtchyDiqh4KjBJ5tW3plBZb5iL/BPcso8A5DlzeD9qlw0duCamnNdpFjxwaT0KyKg==} engines: {node: '>=14'} + generic-pool@3.9.0: + resolution: {integrity: sha512-hymDOu5B53XvN4QT9dBmZxPX4CWhBPPLguTZ9MMFeFa/Kg0xWVfylOVNlJji/E7yTZWFd/q9GO5TxDLq156D7g==} + engines: {node: '>= 4'} + gensync@1.0.0-beta.2: resolution: {integrity: sha512-3hN7NaskYvMDLQY55gnW3NQ+mesEAepTqlg+VEbj7zzqEMBVNhzcGYYeqFo/TlYz6eQiFcp1HcsCZO+nGgS8zg==} engines: {node: '>=6.9.0'} @@ -9924,7 +9987,6 @@ packages: resolution: {integrity: sha512-t0etAxTUk1w5MYdNOkZBZ8rvYYN5iL+2dHCCx/DpkFm/bW28M6y5nUS83D4XdZiHy35Fpaw6LBb+F88fHZnVCw==} engines: {node: '>=8.17.0'} hasBin: true - bundledDependencies: [] jsonfile@6.1.0: resolution: {integrity: sha512-5dgndWOriYSm5cnYaJNhalLNDKOqFwyDB/rr1E9ZsGciGvKPs8R2xYGCacuf3z6K1YKDz182fd+fY3cn3pMqXQ==} @@ -10276,8 +10338,8 @@ packages: lru-cache@10.4.3: resolution: {integrity: sha512-JNAzZcXrCt42VGLuYz0zfAzDfAvJWW6AfYlDBQyDV5DClI2m5sAmK+OIO7s59XfsRsWHp02jAJrRadPRGTt6SQ==} - lru-cache@11.0.1: - resolution: {integrity: sha512-CgeuL5uom6j/ZVrg7G/+1IXqRY8JXX4Hghfy5YE0EhoYQWvndP1kufu58cmZLNIDKnRhZrXfdS9urVWx98AipQ==} + lru-cache@11.1.0: + resolution: {integrity: sha512-QIXZUBJUx+2zHUdQujWejBkcD9+cs94tLn0+YL8UrCh+D5sCXZ4c7LaEH48pNwRY3MLDgqUFyhlCyjJPf1WP0A==} engines: {node: 20 || >=22} lru-cache@5.1.1: @@ -11825,6 +11887,9 @@ packages: resolution: {integrity: sha512-6tDA8g98We0zd0GvVeMT9arEOnTw9qM03L9cJXaCjrip1OO764RDBLBfrB4cwzNGDj5OA5ioymC9GkizgWJDUg==} engines: {node: '>=8'} + redis@4.7.0: + resolution: {integrity: sha512-zvmkHEAdGMn+hMRXuMBtu4Vo5P6rHQjLoHftu+lBqq8ZTA3RCVC/WzD790bkKKiNFp7d5/9PcSD19fJyyRvOdQ==} + reflect.getprototypeof@1.0.10: resolution: {integrity: sha512-00o4I+DVrefhv+nX0ulyi3biSHCPDe+yLv5o/p6d/UVlirijB8E16FtfwSAi4g3tcqrQ4lRAqQSoFEZJehYEcw==} engines: {node: '>= 0.4'} @@ -13350,6 +13415,9 @@ packages: yallist@3.1.1: resolution: {integrity: sha512-a4UGQaWPH59mOXUYnAG2ewncQS4i4F43Tv3JoAM+s2VDAmS9NsK8GpDMLrCHPksFT7h3K6TOoUNn2pb7RoXx4g==} + yallist@4.0.0: + resolution: {integrity: sha512-3wdGidZyq5PB084XLES5TpOSRA3wjXAlIWMhum2kRcv/41Sn2emQ0dycQW4uZXLejwKvg6EsvbdlVL+FYEct7A==} + yaml@1.10.2: resolution: {integrity: sha512-r3vXyErRCYJ7wg28yvBY5VSoAF8ZvlcW9/BwUzEtUsjvX/DKs24dIkuwjtuprwJJHsbyUbLApepYTR1BN4uHrg==} engines: {node: '>= 6'} @@ -15487,6 +15555,16 @@ snapshots: dependencies: tslib: 2.8.1 + '@fortedigital/nextjs-cache-handler@1.2.0(next@15.2.3(@babel/core@7.25.2)(@opentelemetry/api@1.9.0)(@playwright/test@1.51.0)(react-dom@19.0.0(react@19.0.0))(react@19.0.0))(redis@4.7.0)': + dependencies: + '@neshca/cache-handler': 1.9.0(next@15.2.3(@babel/core@7.25.2)(@opentelemetry/api@1.9.0)(@playwright/test@1.51.0)(react-dom@19.0.0(react@19.0.0))(react@19.0.0))(redis@4.7.0) + cluster-key-slot: 1.1.2 + lru-cache: 11.1.0 + next: 15.2.3(@babel/core@7.25.2)(@opentelemetry/api@1.9.0)(@playwright/test@1.51.0)(react-dom@19.0.0(react@19.0.0))(react@19.0.0) + redis: 4.7.0 + optionalDependencies: + '@rollup/rollup-linux-x64-gnu': 4.40.1 + '@googleapis/drive@8.16.0': dependencies: googleapis-common: 7.2.0 @@ -16076,6 +16154,13 @@ snapshots: '@tybys/wasm-util': 0.9.0 optional: true + '@neshca/cache-handler@1.9.0(next@15.2.3(@babel/core@7.25.2)(@opentelemetry/api@1.9.0)(@playwright/test@1.51.0)(react-dom@19.0.0(react@19.0.0))(react@19.0.0))(redis@4.7.0)': + dependencies: + cluster-key-slot: 1.1.2 + lru-cache: 10.4.3 + next: 15.2.3(@babel/core@7.25.2)(@opentelemetry/api@1.9.0)(@playwright/test@1.51.0)(react-dom@19.0.0(react@19.0.0))(react@19.0.0) + redis: 4.7.0 + '@next/env@15.0.4': {} '@next/env@15.2.3': {} @@ -18552,6 +18637,32 @@ snapshots: - '@types/react' - immer + '@redis/bloom@1.2.0(@redis/client@1.6.0)': + dependencies: + '@redis/client': 1.6.0 + + '@redis/client@1.6.0': + dependencies: + cluster-key-slot: 1.1.2 + generic-pool: 3.9.0 + yallist: 4.0.0 + + '@redis/graph@1.1.1(@redis/client@1.6.0)': + dependencies: + '@redis/client': 1.6.0 + + '@redis/json@1.0.7(@redis/client@1.6.0)': + dependencies: + '@redis/client': 1.6.0 + + '@redis/search@1.2.0(@redis/client@1.6.0)': + dependencies: + '@redis/client': 1.6.0 + + '@redis/time-series@1.1.0(@redis/client@1.6.0)': + dependencies: + '@redis/client': 1.6.0 + '@remirror/core-constants@3.0.0': {} '@remirror/core-helpers@4.0.0': @@ -18720,6 +18831,9 @@ snapshots: '@rollup/rollup-linux-x64-gnu@4.35.0': optional: true + '@rollup/rollup-linux-x64-gnu@4.40.1': + optional: true + '@rollup/rollup-linux-x64-musl@4.21.2': optional: true @@ -21319,6 +21433,8 @@ snapshots: clsx@2.1.1: {} + cluster-key-slot@1.1.2: {} + cmdk@1.0.4(@types/react-dom@19.0.3(@types/react@19.0.6))(@types/react@19.0.6)(react-dom@19.0.0(react@19.0.0))(react@19.0.0): dependencies: '@radix-ui/react-dialog': 1.1.4(@types/react-dom@19.0.3(@types/react@19.0.6))(@types/react@19.0.6)(react-dom@19.0.0(react@19.0.0))(react@19.0.0) @@ -22709,6 +22825,8 @@ snapshots: - encoding - supports-color + generic-pool@3.9.0: {} + gensync@1.0.0-beta.2: {} get-caller-file@2.0.5: {} @@ -24075,7 +24193,7 @@ snapshots: lru-cache@10.4.3: {} - lru-cache@11.0.1: {} + lru-cache@11.1.0: {} lru-cache@5.1.1: dependencies: @@ -25342,7 +25460,7 @@ snapshots: path-scurry@2.0.0: dependencies: - lru-cache: 11.0.1 + lru-cache: 11.1.0 minipass: 7.1.2 path-to-regexp@3.3.0: {} @@ -26163,6 +26281,15 @@ snapshots: indent-string: 4.0.0 strip-indent: 3.0.0 + redis@4.7.0: + dependencies: + '@redis/bloom': 1.2.0(@redis/client@1.6.0) + '@redis/client': 1.6.0 + '@redis/graph': 1.1.1(@redis/client@1.6.0) + '@redis/json': 1.0.7(@redis/client@1.6.0) + '@redis/search': 1.2.0(@redis/client@1.6.0) + '@redis/time-series': 1.1.0(@redis/client@1.6.0) + reflect.getprototypeof@1.0.10: dependencies: call-bind: 1.0.8 @@ -28093,6 +28220,8 @@ snapshots: yallist@3.1.1: {} + yallist@4.0.0: {} + yaml@1.10.2: {} yaml@2.3.1: {} From e169a24d7355af2cde07447cb13ecc262f1521f6 Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Wed, 30 Apr 2025 16:15:22 -0500 Subject: [PATCH 02/15] Disable cache when unable to connect to redis --- core/cache-handler.mjs | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/core/cache-handler.mjs b/core/cache-handler.mjs index 0d1c477c4f..2f5993806a 100644 --- a/core/cache-handler.mjs +++ b/core/cache-handler.mjs @@ -5,9 +5,17 @@ import createBufferStringHandler from "@fortedigital/nextjs-cache-handler/buffer import { Next15CacheHandler } from "@fortedigital/nextjs-cache-handler/next-15-cache-handler"; import createRedisHandler from "@fortedigital/nextjs-cache-handler/redis-strings"; import { CacheHandler } from "@neshca/cache-handler"; -import createLruHandler from "@neshca/cache-handler/local-lru"; import { createClient } from "redis"; +// A cache that always misses - intended to let us disable caching when redis is unavailable. Should +// be replaced if there's a better way to do that. +const dummyHandler = { + name: "no-cache", + get: () => undefined, + set: () => undefined, + revalidateTag: () => undefined, +}; + // Usual onCreation from @neshca/cache-handler CacheHandler.onCreation(() => { // Important - It's recommended to use global scope to ensure only one Redis connection is made @@ -24,6 +32,7 @@ CacheHandler.onCreation(() => { // Main promise initializing the handler global.cacheHandlerConfigPromise = (async () => { + console.info("Getting cache handler"); /** @type {import("redis").RedisClientType | null} */ let redisClient = null; if (PHASE_PRODUCTION_BUILD !== process.env.NEXT_PHASE) { @@ -33,14 +42,14 @@ CacheHandler.onCreation(() => { pingInterval: 10000, }); redisClient.on("error", (e) => { - if (typeof process.env.NEXT_PRIVATE_DEBUG_CACHE !== "undefined") { - console.warn("Redis error", e); - } - global.cacheHandlerConfig = null; + console.warn("Redis error", e); + console.warn("Disabling caching"); + global.cacheHandlerConfig = { handlers: [dummyHandler] }; global.cacheHandlerConfigPromise = null; + throw e; }); } catch (error) { - console.warn("Failed to create Redis client:", error); + console.error("Failed to create Redis client:", error); } } @@ -58,12 +67,11 @@ CacheHandler.onCreation(() => { ); } } - const lruCache = createLruHandler(); if (!redisClient?.isReady) { console.error("Failed to initialize caching layer."); global.cacheHandlerConfigPromise = null; - global.cacheHandlerConfig = { handlers: [lruCache] }; + global.cacheHandlerConfig = { handlers: [dummyHandler] }; return global.cacheHandlerConfig; } From 5dc59460268caafcd2f66d035fc167ad55e8ae9d Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Mon, 5 May 2025 13:24:53 -0500 Subject: [PATCH 03/15] Cleanup dockerfile --- Dockerfile | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/Dockerfile b/Dockerfile index f23946bb53..ffa93e6465 100644 --- a/Dockerfile +++ b/Dockerfile @@ -15,7 +15,7 @@ ARG PNPM_VERSION=9.10.0 ################################################################################ # Use node image for base image for all stages. -FROM node:${NODE_VERSION}-alpine${ALPINE_VERSION} as base +FROM node:${NODE_VERSION}-alpine${ALPINE_VERSION} AS base # these are necessary to be able to use them inside of `base` ARG BASE_IMAGE @@ -39,22 +39,22 @@ RUN --mount=type=cache,target=/root/.npm \ npm install -g pnpm@${PNPM_VERSION} -FROM base as fetch-deps +FROM base AS fetch-deps # Copy pnpm-lock.yaml so that we can use pnpm to install dependencies COPY pnpm-lock.yaml ./ -# Could possibly be sped up using `turbo prune` +# Could possibly be sped up using `turbo prune` # https://turbo.build/repo/docs/guides/tools/docker RUN pnpm fetch # Install dependencies we only need to run pnpm install -RUN apk add g++ make py3-pip +RUN apk add g++ make py3-pip ################################################################################ # Create a stage for building the application. -FROM fetch-deps as monorepo +FROM fetch-deps AS monorepo # Copy over the rest of the files ADD . ./ @@ -76,19 +76,19 @@ RUN test -n "$PACKAGE" || (echo "PACKAGE not set, required for this target" && ENV DOCKERBUILD=1 ARG CI -ENV CI $CI +ENV CI=$CI RUN --mount=type=secret,id=SENTRY_AUTH_TOKEN,env=SENTRY_AUTH_TOKEN \ - pnpm --filter $PACKAGE build + pnpm --filter $PACKAGE build -FROM withpackage as prepare-jobs +FROM withpackage AS prepare-jobs ARG PACKAGE RUN pnpm --filter $PACKAGE --prod deploy /tmp/app -FROM base as jobs +FROM base AS jobs WORKDIR /usr/src/app @@ -96,19 +96,19 @@ COPY --from=prepare-jobs --chown=node:node /tmp/app . USER node -CMD pnpm start +CMD ["pnpm", "start"] ################################################################################ # Create a new stage to run the application with minimal runtime dependencies # where the necessary files are copied from the build stage. # this is separated by package to make it slightly more clear what happens # and because you cannot conditionally copy from a different folder -# based on the argument -FROM base as prod-setup +# based on the argument +FROM base AS prod-setup ARG PORT # Use production node environment by default. -ENV NODE_ENV production +ENV NODE_ENV=production # Run the application as a non-root user. USER node @@ -117,10 +117,10 @@ USER node EXPOSE $PORT # Use production node environment by default. -ENV NODE_ENV production +ENV NODE_ENV=production # otherwise it will use the strange default docker hostname -ENV HOSTNAME "0.0.0.0" +ENV HOSTNAME="0.0.0.0" ### Core @@ -132,4 +132,4 @@ COPY --from=withpackage --chown=node:node /usr/src/app/core/public ./core/public # needed to set the database url correctly based on PGHOST variables COPY --from=withpackage --chown=node:node /usr/src/app/core/.env.docker ./core/.env -CMD node core/server.js +CMD ["node", "core/server.js"] From 536080f151eab306c4cf43b50335de9fa0e18024 Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Mon, 5 May 2025 13:26:53 -0500 Subject: [PATCH 04/15] Capture sentry exception when redis errors --- core/cache-handler.mjs | 37 ++++++++++++------------------------- core/lib/redis.ts | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 25 deletions(-) create mode 100644 core/lib/redis.ts diff --git a/core/cache-handler.mjs b/core/cache-handler.mjs index 2f5993806a..f076a7abf0 100644 --- a/core/cache-handler.mjs +++ b/core/cache-handler.mjs @@ -5,7 +5,9 @@ import createBufferStringHandler from "@fortedigital/nextjs-cache-handler/buffer import { Next15CacheHandler } from "@fortedigital/nextjs-cache-handler/next-15-cache-handler"; import createRedisHandler from "@fortedigital/nextjs-cache-handler/redis-strings"; import { CacheHandler } from "@neshca/cache-handler"; -import { createClient } from "redis"; +import { captureException } from "@sentry/nextjs"; + +import { getRedisClient } from "lib/server/redis"; // A cache that always misses - intended to let us disable caching when redis is unavailable. Should // be replaced if there's a better way to do that. @@ -37,34 +39,19 @@ CacheHandler.onCreation(() => { let redisClient = null; if (PHASE_PRODUCTION_BUILD !== process.env.NEXT_PHASE) { try { - redisClient = createClient({ - url: process.env.VALKEY_URL, - pingInterval: 10000, - }); - redisClient.on("error", (e) => { - console.warn("Redis error", e); - console.warn("Disabling caching"); + redisClient = await getRedisClient(); + redisClient.on("error", (err) => { + logger.error({ + msg: "Disabling caching because of redis connection error", + err, + }); + captureException(err); global.cacheHandlerConfig = { handlers: [dummyHandler] }; global.cacheHandlerConfigPromise = null; throw e; }); - } catch (error) { - console.error("Failed to create Redis client:", error); - } - } - - if (redisClient) { - try { - console.info("Connecting Redis client..."); - await redisClient.connect(); - console.info("Redis client connected."); - } catch (error) { - console.warn("Failed to connect Redis client:", error); - await redisClient - .disconnect() - .catch(() => - console.warn("Failed to quit the Redis client after failing to connect.") - ); + } catch (err) { + logger.error({ msg: "Failed to create Redis client:", err }); } } diff --git a/core/lib/redis.ts b/core/lib/redis.ts new file mode 100644 index 0000000000..39ee38beea --- /dev/null +++ b/core/lib/redis.ts @@ -0,0 +1,35 @@ +import type { RedisClientType } from "redis"; + +import { captureException } from "@sentry/nextjs"; +import { createClient } from "redis"; + +import { logger } from "logger"; + +import { env } from "./env/env"; + +let redisClient: RedisClientType; + +export const getRedisClient = async () => { + if (!redisClient) { + logger.info({ msg: "Creating redis client" }); + redisClient = createClient({ + url: env.VALKEY_URL, + pingInterval: 10000, + disableOfflineQueue: true, + }); + } + + if (!redisClient.isReady) { + logger.info({ msg: "Connecting redis client" }); + try { + await redisClient.connect(); + logger.info({ msg: "Redis client connected" }); + } catch (err) { + logger.error("Failed to connect Redis client", err); + captureException(err); + await redisClient.disconnect(); + } + } + + return redisClient; +}; From cad3af5845548e4bfdda6f9c8609531738820967 Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Mon, 5 May 2025 13:27:15 -0500 Subject: [PATCH 05/15] Add redis query to healthcheck endpoint --- core/app/api/health/route.ts | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/core/app/api/health/route.ts b/core/app/api/health/route.ts index 1be19c1a65..ac0c7a2e31 100644 --- a/core/app/api/health/route.ts +++ b/core/app/api/health/route.ts @@ -2,25 +2,25 @@ import type { NextRequest } from "next/server"; import { NextResponse } from "next/server"; +import { logger } from "logger"; + import { db } from "~/kysely/database"; +import { getRedisClient } from "~/lib/redis"; import { handleErrors } from "~/lib/server"; export async function GET(req: NextRequest) { return await handleErrors(async () => { - const errors: string[] = []; try { - const dbQuery = await db + const dbQuery = db .selectFrom("communities") - .selectAll() + .select("id") + .limit(1) .executeTakeFirstOrThrow(); + const cacheQuery = (await getRedisClient()).ping(); + await Promise.all([dbQuery, cacheQuery]); } catch (err) { - if (err instanceof Error) { - errors.push(err.message); - } - } - - if (errors.length > 0) { - return NextResponse.json({ errors }, { status: 500 }); + logger.error({ msg: "error in health check", err }); + return NextResponse.json({}, { status: 500 }); } return NextResponse.json({}); From 8304abc3e7f8e0f0f88788c15fadc986b08c9691 Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Mon, 5 May 2025 17:59:01 -0500 Subject: [PATCH 06/15] Inline forte-digital cache handler --- core/cache-handler.mjs | 309 ++++++++++++++++++++++++++++++++++------- 1 file changed, 262 insertions(+), 47 deletions(-) diff --git a/core/cache-handler.mjs b/core/cache-handler.mjs index f076a7abf0..cc3ed86f26 100644 --- a/core/cache-handler.mjs +++ b/core/cache-handler.mjs @@ -3,11 +3,9 @@ import { PHASE_PRODUCTION_BUILD } from "next/constants.js"; import createBufferStringHandler from "@fortedigital/nextjs-cache-handler/buffer-string-decorator"; import { Next15CacheHandler } from "@fortedigital/nextjs-cache-handler/next-15-cache-handler"; -import createRedisHandler from "@fortedigital/nextjs-cache-handler/redis-strings"; import { CacheHandler } from "@neshca/cache-handler"; -import { captureException } from "@sentry/nextjs"; - -import { getRedisClient } from "lib/server/redis"; +// src/handlers/redis-strings.ts +import { getTimeoutRedisCommandOptions, isImplicitTag } from "@neshca/cache-handler/helpers"; // A cache that always misses - intended to let us disable caching when redis is unavailable. Should // be replaced if there's a better way to do that. @@ -18,6 +16,265 @@ const dummyHandler = { revalidateTag: () => undefined, }; +// src/constants.ts +var REVALIDATED_TAGS_KEY = "__revalidated_tags__"; + +// src/handlers/redis-strings.ts +function createHandler({ + client, + keyPrefix = "", + sharedTagsKey = "__sharedTags__", + sharedTagsTtlKey = "__sharedTagsTtl__", + timeoutMs = 5e3, + keyExpirationStrategy = "EXPIREAT", + revalidateTagQuerySize = 1e4, +}) { + function assertClientIsReady() { + if (!client.isReady) { + throw new Error("Redis client is not ready yet or connection is lost. Keep trying..."); + } + } + async function revalidateTags(tag) { + const tagsMap = /* @__PURE__ */ new Map(); + let cursor = 0; + const hScanOptions = { COUNT: revalidateTagQuerySize }; + do { + const remoteTagsPortion = await client.hScan( + getTimeoutRedisCommandOptions(timeoutMs), + keyPrefix + sharedTagsKey, + cursor, + hScanOptions + ); + for (const { field, value } of remoteTagsPortion.tuples) { + tagsMap.set(field, JSON.parse(value)); + } + cursor = remoteTagsPortion.cursor; + } while (cursor !== 0); + const keysToDelete = []; + const tagsToDelete = []; + for (const [key, tags] of tagsMap) { + if (tags.includes(tag)) { + keysToDelete.push(keyPrefix + key); + tagsToDelete.push(key); + } + } + if (keysToDelete.length === 0) { + return; + } + await client.unlink(getTimeoutRedisCommandOptions(timeoutMs), keysToDelete); + const updateTagsOperation = client.hDel( + { isolated: true, ...getTimeoutRedisCommandOptions(timeoutMs) }, + keyPrefix + sharedTagsKey, + tagsToDelete + ); + const updateTtlOperation = client.hDel( + { isolated: true, ...getTimeoutRedisCommandOptions(timeoutMs) }, + keyPrefix + sharedTagsTtlKey, + tagsToDelete + ); + await Promise.all([updateTtlOperation, updateTagsOperation]); + } + async function revalidateSharedKeys() { + const ttlMap = /* @__PURE__ */ new Map(); + let cursor = 0; + const hScanOptions = { COUNT: revalidateTagQuerySize }; + do { + const remoteTagsPortion = await client.hScan( + getTimeoutRedisCommandOptions(timeoutMs), + keyPrefix + sharedTagsTtlKey, + cursor, + hScanOptions + ); + for (const { field, value } of remoteTagsPortion.tuples) { + ttlMap.set(field, Number(value)); + } + cursor = remoteTagsPortion.cursor; + } while (cursor !== 0); + const tagsAndTtlToDelete = []; + const keysToDelete = []; + for (const [key, ttlInSeconds] of ttlMap) { + if (/* @__PURE__ */ new Date().getTime() > ttlInSeconds * 1e3) { + tagsAndTtlToDelete.push(key); + keysToDelete.push(keyPrefix + key); + } + } + if (tagsAndTtlToDelete.length === 0) { + return; + } + await client.unlink(getTimeoutRedisCommandOptions(timeoutMs), keysToDelete); + const updateTtlOperation = client.hDel( + { + isolated: true, + ...getTimeoutRedisCommandOptions(timeoutMs), + }, + keyPrefix + sharedTagsTtlKey, + tagsAndTtlToDelete + ); + const updateTagsOperation = client.hDel( + { + isolated: true, + ...getTimeoutRedisCommandOptions(timeoutMs), + }, + keyPrefix + sharedTagsKey, + tagsAndTtlToDelete + ); + await Promise.all([updateTagsOperation, updateTtlOperation]); + } + const revalidatedTagsKey = keyPrefix + REVALIDATED_TAGS_KEY; + return { + name: "forte-digital-redis-strings", + async get(key, { implicitTags }) { + assertClientIsReady(); + const result = await client.get( + getTimeoutRedisCommandOptions(timeoutMs), + keyPrefix + key + ); + if (!result) { + return null; + } + const cacheValue = JSON.parse(result); + if (!cacheValue) { + return null; + } + const sharedTagKeyExists = await client.hExists( + getTimeoutRedisCommandOptions(timeoutMs), + keyPrefix + sharedTagsKey, + key + ); + if (!sharedTagKeyExists) { + await client.unlink(getTimeoutRedisCommandOptions(timeoutMs), keyPrefix + key); + return null; + } + const combinedTags = /* @__PURE__ */ new Set([...cacheValue.tags, ...implicitTags]); + if (combinedTags.size === 0) { + return cacheValue; + } + const revalidationTimes = await client.hmGet( + getTimeoutRedisCommandOptions(timeoutMs), + revalidatedTagsKey, + Array.from(combinedTags) + ); + for (const timeString of revalidationTimes) { + if (timeString && Number.parseInt(timeString, 10) > cacheValue.lastModified) { + await client.unlink(getTimeoutRedisCommandOptions(timeoutMs), keyPrefix + key); + return null; + } + } + return cacheValue; + }, + async set(key, cacheHandlerValue) { + assertClientIsReady(); + const options = getTimeoutRedisCommandOptions(timeoutMs); + let setOperation; + let expireOperation; + const lifespan = cacheHandlerValue.lifespan; + const setTagsOperation = + cacheHandlerValue.tags.length > 0 + ? client.hSet( + options, + keyPrefix + sharedTagsKey, + key, + JSON.stringify(cacheHandlerValue.tags) + ) + : void 0; + const setSharedTtlOperation = lifespan + ? client.hSet(options, keyPrefix + sharedTagsTtlKey, key, lifespan.expireAt) + : void 0; + await Promise.all([setTagsOperation, setSharedTtlOperation]); + switch (keyExpirationStrategy) { + case "EXAT": { + setOperation = client.set( + options, + keyPrefix + key, + JSON.stringify(cacheHandlerValue), + typeof lifespan?.expireAt === "number" + ? { + EXAT: lifespan.expireAt, + } + : void 0 + ); + break; + } + case "EXPIREAT": { + setOperation = client.set( + options, + keyPrefix + key, + JSON.stringify(cacheHandlerValue) + ); + expireOperation = lifespan + ? client.expireAt(options, keyPrefix + key, lifespan.expireAt) + : void 0; + break; + } + default: { + throw new Error(`Invalid keyExpirationStrategy: ${keyExpirationStrategy}`); + } + } + await Promise.all([setOperation, expireOperation]); + }, + async revalidateTag(tag) { + assertClientIsReady(); + if (isImplicitTag(tag)) { + await client.hSet( + getTimeoutRedisCommandOptions(timeoutMs), + revalidatedTagsKey, + tag, + Date.now() + ); + } + await Promise.all([revalidateTags(tag), revalidateSharedKeys()]); + }, + async delete(key) { + await client.unlink(getTimeoutRedisCommandOptions(timeoutMs), keyPrefix + key); + await Promise.all([ + client.hDel(keyPrefix + sharedTagsKey, key), + client.hDel(keyPrefix + sharedTagsTtlKey, key), + ]); + }, + }; +} + +async function getCacheHandlerPromise() { + let redisClient = null; + if (PHASE_PRODUCTION_BUILD !== process.env.NEXT_PHASE) { + try { + redisClient = createClient({ + url: process.env.VALKEY_URL, + pingInterval: 10000, + }); + + await redisClient.connect(); + console.log("Successfully connected to redis for caching"); + redisClient.on("error", (err) => { + console.error("Cache error:", err); + }); + } catch (err) { + console.error("Failed to create Redis client:", err); + } + } + if (redisClient?.status !== "ready") { + console.error("Failed to initialize caching layer."); + global.cacheHandlerConfigPromise = null; + global.cacheHandlerConfig = { handlers: [dummyHandler] }; + return global.cacheHandlerConfig; + } + + const redisCacheHandler = createBufferStringHandler( + createRedisHandler({ + client: redisClient, + keyPrefix: "nextjs:", + }) + ); + + global.cacheHandlerConfigPromise = null; + + global.cacheHandlerConfig = { + handlers: [redisCacheHandler], + }; + + return global.cacheHandlerConfig; +} + // Usual onCreation from @neshca/cache-handler CacheHandler.onCreation(() => { // Important - It's recommended to use global scope to ensure only one Redis connection is made @@ -33,49 +290,7 @@ CacheHandler.onCreation(() => { } // Main promise initializing the handler - global.cacheHandlerConfigPromise = (async () => { - console.info("Getting cache handler"); - /** @type {import("redis").RedisClientType | null} */ - let redisClient = null; - if (PHASE_PRODUCTION_BUILD !== process.env.NEXT_PHASE) { - try { - redisClient = await getRedisClient(); - redisClient.on("error", (err) => { - logger.error({ - msg: "Disabling caching because of redis connection error", - err, - }); - captureException(err); - global.cacheHandlerConfig = { handlers: [dummyHandler] }; - global.cacheHandlerConfigPromise = null; - throw e; - }); - } catch (err) { - logger.error({ msg: "Failed to create Redis client:", err }); - } - } - - if (!redisClient?.isReady) { - console.error("Failed to initialize caching layer."); - global.cacheHandlerConfigPromise = null; - global.cacheHandlerConfig = { handlers: [dummyHandler] }; - return global.cacheHandlerConfig; - } - - const redisCacheHandler = createRedisHandler({ - client: redisClient, - keyPrefix: "nextjs:", - keyExpirationStrategy: "EXAT", - }); - - global.cacheHandlerConfigPromise = null; - - global.cacheHandlerConfig = { - handlers: [createBufferStringHandler(redisCacheHandler)], - }; - - return global.cacheHandlerConfig; - })(); + global.cacheHandlerConfigPromise = getCacheHandlerPromise(); return global.cacheHandlerConfigPromise; }); From 43461952190f20c6074827a8fb21738a398f26f0 Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Mon, 5 May 2025 18:05:37 -0500 Subject: [PATCH 07/15] Switch to ioredis --- .env.docker-compose.dev | 2 +- .env.docker-compose.test | 2 +- core/.env.development | 2 +- core/.env.test | 2 +- core/cache-handler.mjs | 164 ++++++------------ core/lib/env/env.ts | 2 +- core/lib/redis.ts | 21 +-- core/package.json | 2 +- .../modules/core-services/outputs.tf | 2 +- .../terraform/modules/deployment/main.tf | 4 +- pnpm-lock.yaml | 67 +++++-- self-host/.env.example | 2 +- 12 files changed, 129 insertions(+), 143 deletions(-) diff --git a/.env.docker-compose.dev b/.env.docker-compose.dev index 09a6d46a4c..a9fc15eac3 100644 --- a/.env.docker-compose.dev +++ b/.env.docker-compose.dev @@ -12,4 +12,4 @@ POSTGRES_USER=postgres POSTGRES_PASSWORD=postgres POSTGRES_DB=postgres -VALKEY_URL='redis://cache:6379' \ No newline at end of file +VALKEY_HOST='cache' \ No newline at end of file diff --git a/.env.docker-compose.test b/.env.docker-compose.test index b2c1a5796b..0e8455c368 100644 --- a/.env.docker-compose.test +++ b/.env.docker-compose.test @@ -38,4 +38,4 @@ MAILGUN_SMTP_USERNAME=omitted OTEL_SERVICE_NAME=core.core PUBPUB_URL=http://localhost:3000 API_KEY=xxx -VALKEY_URL='redis://cache:6379' \ No newline at end of file +VALKEY_HOST='cache' \ No newline at end of file diff --git a/core/.env.development b/core/.env.development index 8fb25f5443..c30d247cc9 100644 --- a/core/.env.development +++ b/core/.env.development @@ -25,4 +25,4 @@ DATACITE_PASSWORD="" DATACITE_API_URL="https://api.test.datacite.org" GCLOUD_KEY_FILE='xxx' -VALKEY_URL='redis://localhost:6379' \ No newline at end of file +VALKEY_HOST='localhost' \ No newline at end of file diff --git a/core/.env.test b/core/.env.test index 47fdfe7d93..f90d463c1b 100644 --- a/core/.env.test +++ b/core/.env.test @@ -19,4 +19,4 @@ HONEYCOMB_API_KEY="xxx" # KYSELY_DEBUG="true" GCLOUD_KEY_FILE='xxx' -VALKEY_URL='redis://cache:6379' \ No newline at end of file +VALKEY_HOST='cache' \ No newline at end of file diff --git a/core/cache-handler.mjs b/core/cache-handler.mjs index cc3ed86f26..7b2f01efaf 100644 --- a/core/cache-handler.mjs +++ b/core/cache-handler.mjs @@ -4,8 +4,8 @@ import { PHASE_PRODUCTION_BUILD } from "next/constants.js"; import createBufferStringHandler from "@fortedigital/nextjs-cache-handler/buffer-string-decorator"; import { Next15CacheHandler } from "@fortedigital/nextjs-cache-handler/next-15-cache-handler"; import { CacheHandler } from "@neshca/cache-handler"; -// src/handlers/redis-strings.ts -import { getTimeoutRedisCommandOptions, isImplicitTag } from "@neshca/cache-handler/helpers"; +import { isImplicitTag } from "@neshca/cache-handler/helpers"; +import Redis from "ioredis"; // A cache that always misses - intended to let us disable caching when redis is unavailable. Should // be replaced if there's a better way to do that. @@ -16,34 +16,30 @@ const dummyHandler = { revalidateTag: () => undefined, }; -// src/constants.ts var REVALIDATED_TAGS_KEY = "__revalidated_tags__"; -// src/handlers/redis-strings.ts -function createHandler({ +/** + * Creates a redis handler based on fortedigital's redis-strings handler, but using the ioredis + * client + * + * @param {{ client: Redis}} props + */ +function createRedisHandler({ client, keyPrefix = "", sharedTagsKey = "__sharedTags__", sharedTagsTtlKey = "__sharedTagsTtl__", - timeoutMs = 5e3, - keyExpirationStrategy = "EXPIREAT", revalidateTagQuerySize = 1e4, }) { - function assertClientIsReady() { - if (!client.isReady) { - throw new Error("Redis client is not ready yet or connection is lost. Keep trying..."); - } - } async function revalidateTags(tag) { const tagsMap = /* @__PURE__ */ new Map(); let cursor = 0; - const hScanOptions = { COUNT: revalidateTagQuerySize }; do { - const remoteTagsPortion = await client.hScan( - getTimeoutRedisCommandOptions(timeoutMs), + const remoteTagsPortion = await client.hscan( keyPrefix + sharedTagsKey, cursor, - hScanOptions + "COUNT", + revalidateTagQuerySize ); for (const { field, value } of remoteTagsPortion.tuples) { tagsMap.set(field, JSON.parse(value)); @@ -61,29 +57,20 @@ function createHandler({ if (keysToDelete.length === 0) { return; } - await client.unlink(getTimeoutRedisCommandOptions(timeoutMs), keysToDelete); - const updateTagsOperation = client.hDel( - { isolated: true, ...getTimeoutRedisCommandOptions(timeoutMs) }, - keyPrefix + sharedTagsKey, - tagsToDelete - ); - const updateTtlOperation = client.hDel( - { isolated: true, ...getTimeoutRedisCommandOptions(timeoutMs) }, - keyPrefix + sharedTagsTtlKey, - tagsToDelete - ); + await client.unlink(keysToDelete); + const updateTagsOperation = client.hdel(keyPrefix + sharedTagsKey, tagsToDelete); + const updateTtlOperation = client.hdel(keyPrefix + sharedTagsTtlKey, tagsToDelete); await Promise.all([updateTtlOperation, updateTagsOperation]); } async function revalidateSharedKeys() { const ttlMap = /* @__PURE__ */ new Map(); let cursor = 0; - const hScanOptions = { COUNT: revalidateTagQuerySize }; do { - const remoteTagsPortion = await client.hScan( - getTimeoutRedisCommandOptions(timeoutMs), - keyPrefix + sharedTagsTtlKey, + const remoteTagsPortion = await client.hscan( + keyPrefix + sharedTagsKey, cursor, - hScanOptions + "COUNT", + revalidateTagQuerySize ); for (const { field, value } of remoteTagsPortion.tuples) { ttlMap.set(field, Number(value)); @@ -101,34 +88,16 @@ function createHandler({ if (tagsAndTtlToDelete.length === 0) { return; } - await client.unlink(getTimeoutRedisCommandOptions(timeoutMs), keysToDelete); - const updateTtlOperation = client.hDel( - { - isolated: true, - ...getTimeoutRedisCommandOptions(timeoutMs), - }, - keyPrefix + sharedTagsTtlKey, - tagsAndTtlToDelete - ); - const updateTagsOperation = client.hDel( - { - isolated: true, - ...getTimeoutRedisCommandOptions(timeoutMs), - }, - keyPrefix + sharedTagsKey, - tagsAndTtlToDelete - ); + await client.unlink(keysToDelete); + const updateTtlOperation = client.hdel(keyPrefix + sharedTagsTtlKey, tagsAndTtlToDelete); + const updateTagsOperation = client.hdel(keyPrefix + sharedTagsKey, tagsAndTtlToDelete); await Promise.all([updateTagsOperation, updateTtlOperation]); } const revalidatedTagsKey = keyPrefix + REVALIDATED_TAGS_KEY; return { - name: "forte-digital-redis-strings", + name: "pubpub-redis-strings", async get(key, { implicitTags }) { - assertClientIsReady(); - const result = await client.get( - getTimeoutRedisCommandOptions(timeoutMs), - keyPrefix + key - ); + const result = await client.get(keyPrefix + key); if (!result) { return null; } @@ -136,99 +105,63 @@ function createHandler({ if (!cacheValue) { return null; } - const sharedTagKeyExists = await client.hExists( - getTimeoutRedisCommandOptions(timeoutMs), - keyPrefix + sharedTagsKey, - key - ); + const sharedTagKeyExists = await client.hexists(keyPrefix + sharedTagsKey, key); if (!sharedTagKeyExists) { - await client.unlink(getTimeoutRedisCommandOptions(timeoutMs), keyPrefix + key); + await client.unlink(keyPrefix + key); return null; } const combinedTags = /* @__PURE__ */ new Set([...cacheValue.tags, ...implicitTags]); if (combinedTags.size === 0) { return cacheValue; } - const revalidationTimes = await client.hmGet( - getTimeoutRedisCommandOptions(timeoutMs), + const revalidationTimes = await client.hmget( revalidatedTagsKey, Array.from(combinedTags) ); for (const timeString of revalidationTimes) { if (timeString && Number.parseInt(timeString, 10) > cacheValue.lastModified) { - await client.unlink(getTimeoutRedisCommandOptions(timeoutMs), keyPrefix + key); + await client.unlink(keyPrefix + key); return null; } } return cacheValue; }, async set(key, cacheHandlerValue) { - assertClientIsReady(); - const options = getTimeoutRedisCommandOptions(timeoutMs); - let setOperation; - let expireOperation; const lifespan = cacheHandlerValue.lifespan; const setTagsOperation = cacheHandlerValue.tags.length > 0 - ? client.hSet( - options, + ? client.hset( keyPrefix + sharedTagsKey, key, JSON.stringify(cacheHandlerValue.tags) ) : void 0; const setSharedTtlOperation = lifespan - ? client.hSet(options, keyPrefix + sharedTagsTtlKey, key, lifespan.expireAt) + ? client.hset(keyPrefix + sharedTagsTtlKey, key, lifespan.expireAt) : void 0; await Promise.all([setTagsOperation, setSharedTtlOperation]); - switch (keyExpirationStrategy) { - case "EXAT": { - setOperation = client.set( - options, - keyPrefix + key, - JSON.stringify(cacheHandlerValue), - typeof lifespan?.expireAt === "number" - ? { - EXAT: lifespan.expireAt, - } - : void 0 - ); - break; - } - case "EXPIREAT": { - setOperation = client.set( - options, - keyPrefix + key, - JSON.stringify(cacheHandlerValue) - ); - expireOperation = lifespan - ? client.expireAt(options, keyPrefix + key, lifespan.expireAt) - : void 0; - break; - } - default: { - throw new Error(`Invalid keyExpirationStrategy: ${keyExpirationStrategy}`); - } + if (typeof lifespan?.expireAt === "number") { + await client.set( + keyPrefix + key, + JSON.stringify(cacheHandlerValue), + "EXAT", + lifespan.expireAt + ); + } else { + await client.set(keyPrefix + key, JSON.stringify(cacheHandlerValue)); } - await Promise.all([setOperation, expireOperation]); }, async revalidateTag(tag) { - assertClientIsReady(); if (isImplicitTag(tag)) { - await client.hSet( - getTimeoutRedisCommandOptions(timeoutMs), - revalidatedTagsKey, - tag, - Date.now() - ); + await client.hset(revalidatedTagsKey, tag, Date.now()); } await Promise.all([revalidateTags(tag), revalidateSharedKeys()]); }, async delete(key) { - await client.unlink(getTimeoutRedisCommandOptions(timeoutMs), keyPrefix + key); + await client.unlink(keyPrefix + key); await Promise.all([ - client.hDel(keyPrefix + sharedTagsKey, key), - client.hDel(keyPrefix + sharedTagsTtlKey, key), + client.hdel(keyPrefix + sharedTagsKey, key), + client.hdel(keyPrefix + sharedTagsTtlKey, key), ]); }, }; @@ -238,9 +171,14 @@ async function getCacheHandlerPromise() { let redisClient = null; if (PHASE_PRODUCTION_BUILD !== process.env.NEXT_PHASE) { try { - redisClient = createClient({ - url: process.env.VALKEY_URL, - pingInterval: 10000, + redisClient = new Redis({ + host: process.env.VALKEY_HOST, + lazyConnect: true, + commandTimeout: 1000, + retryStrategy: (times) => { + console.log("Retrying redis connection attempt:", times); + return (2 ^ times) + Math.random() * 1000; + }, }); await redisClient.connect(); diff --git a/core/lib/env/env.ts b/core/lib/env/env.ts index 89c15bb90e..7e707e5f5b 100644 --- a/core/lib/env/env.ts +++ b/core/lib/env/env.ts @@ -25,7 +25,7 @@ export const env = createEnv({ * Whether or not to verbosely log `memoize` cache hits and misses */ CACHE_LOG: z.string().optional(), - VALKEY_URL: z.string().url(), + VALKEY_HOST: z.string(), DATABASE_URL: z.string().url(), ENV_NAME: z.string().optional(), FLAGS: flagsSchema, diff --git a/core/lib/redis.ts b/core/lib/redis.ts index 39ee38beea..7a77c0293c 100644 --- a/core/lib/redis.ts +++ b/core/lib/redis.ts @@ -1,25 +1,26 @@ -import type { RedisClientType } from "redis"; - import { captureException } from "@sentry/nextjs"; -import { createClient } from "redis"; +import Redis from "ioredis"; import { logger } from "logger"; import { env } from "./env/env"; -let redisClient: RedisClientType; +let redisClient: Redis; export const getRedisClient = async () => { if (!redisClient) { logger.info({ msg: "Creating redis client" }); - redisClient = createClient({ - url: env.VALKEY_URL, - pingInterval: 10000, - disableOfflineQueue: true, + redisClient = new Redis({ + host: env.VALKEY_HOST, + lazyConnect: true, + commandTimeout: 1000, + retryStrategy: (times) => { + return (2 ^ times) + Math.random() * 1000; + }, }); } - if (!redisClient.isReady) { + if (redisClient.status !== "ready") { logger.info({ msg: "Connecting redis client" }); try { await redisClient.connect(); @@ -27,7 +28,7 @@ export const getRedisClient = async () => { } catch (err) { logger.error("Failed to connect Redis client", err); captureException(err); - await redisClient.disconnect(); + redisClient.disconnect(); } } diff --git a/core/package.json b/core/package.json index a27db56d05..abd06d27c2 100644 --- a/core/package.json +++ b/core/package.json @@ -101,6 +101,7 @@ "hast": "^1.0.0", "hastscript": "^9.0.0", "import-in-the-middle": "^1.13.1", + "ioredis": "^5.6.1", "jsonpath-plus": "^10.2.0", "jsonwebtoken": "^9.0.0", "katex": "catalog:", @@ -127,7 +128,6 @@ "react-hook-form": "catalog:", "react-markdown": "^9.0.1", "reactflow": "^11.10.4", - "redis": "^4.7.0", "rehype": "^13.0.2", "rehype-format": "^5.0.0", "rehype-parse": "^9.0.1", diff --git a/infrastructure/terraform/modules/core-services/outputs.tf b/infrastructure/terraform/modules/core-services/outputs.tf index 7e57f8801e..f3a61f31d5 100644 --- a/infrastructure/terraform/modules/core-services/outputs.tf +++ b/infrastructure/terraform/modules/core-services/outputs.tf @@ -35,6 +35,6 @@ output "rds_connection_components" { } } -output "valkey_url" { +output "valkey_host" { value = aws_elasticache_replication_group.core_valkey.primary_endpoint_address } diff --git a/infrastructure/terraform/modules/deployment/main.tf b/infrastructure/terraform/modules/deployment/main.tf index 10a9dd6ab9..acbab8754c 100644 --- a/infrastructure/terraform/modules/deployment/main.tf +++ b/infrastructure/terraform/modules/deployment/main.tf @@ -100,7 +100,7 @@ module "service_core" { { name = "SUPABASE_PUBLIC_KEY", value = var.NEXT_PUBLIC_SUPABASE_PUBLIC_KEY }, { name = "HOSTNAME", value = var.HOSTNAME }, { name = "DATACITE_API_URL", value = var.DATACITE_API_URL }, - { name = "VALKEY_URL", value = "redis://${module.core_dependency_services.valkey_url}" } + { name = "VALKEY_HOST", value = module.core_dependency_services.valkey_host } ] secrets = [ @@ -167,7 +167,7 @@ module "service_bastion" { { name = "SUPABASE_URL", value = var.NEXT_PUBLIC_SUPABASE_URL }, { name = "HOSTNAME", value = var.HOSTNAME }, { name = "PAGER", value = "less -S" }, - { name = "VALKEY_URL", value = "redis://${module.core_dependency_services.valkey_url}" } + { name = "VALKEY_HOST", value = module.core_dependency_services.valkey_host } ] secrets = [ diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4bb8733df9..b26ac04dd9 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -54,9 +54,6 @@ catalogs: date-fns: specifier: ^4.1.0 version: 4.1.0 - eslint: - specifier: ^9.9.0 - version: 9.10.0 katex: specifier: ^0.16.18 version: 0.16.21 @@ -69,9 +66,6 @@ catalogs: postcss: specifier: ^8.4.27 version: 8.4.49 - prettier: - specifier: ^3.4.2 - version: 3.4.2 react-hook-form: specifier: ^7.54.2 version: 7.54.2 @@ -389,6 +383,9 @@ importers: import-in-the-middle: specifier: ^1.13.1 version: 1.13.1 + ioredis: + specifier: ^5.6.1 + version: 5.6.1 jsonpath-plus: specifier: ^10.2.0 version: 10.2.0 @@ -467,9 +464,6 @@ importers: reactflow: specifier: ^11.10.4 version: 11.11.4(@types/react@19.0.6)(react-dom@19.0.0(react@19.0.0))(react@19.0.0) - redis: - specifier: ^4.7.0 - version: 4.7.0 rehype: specifier: ^13.0.2 version: 13.0.2 @@ -3301,6 +3295,9 @@ packages: cpu: [x64] os: [win32] + '@ioredis/commands@1.2.0': + resolution: {integrity: sha512-Sx1pU8EM64o2BrqNpEO1CNLtKQwyhuXuqyfH7oGKCk+1a33d2r5saW8zNwm3j6BTExtjrv2BxTgzzkMwts6vGg==} + '@isaacs/cliui@8.0.2': resolution: {integrity: sha512-O8jcjabXaleOG9DQ0+ARXWZBTfnP4WNAqzuiJK7ll44AmxGKv/J2M4TPjxjY3znBCfvBXFzucm1twdyFybFqEA==} engines: {node: '>=12'} @@ -8499,6 +8496,10 @@ packages: resolution: {integrity: sha512-ZySD7Nf91aLB0RxL4KGrKHBXl7Eds1DAmEdcoVawXnLD7SDhpNgtuII2aAkg7a7QS41jxPSZ17p4VdGnMHk3MQ==} engines: {node: '>=0.4.0'} + denque@2.1.0: + resolution: {integrity: sha512-HVQE3AAb/pxF8fQAoiqpvg9i3evqug3hoiwakOyZAwJm+6vZehbkYXZ0l4JxS+I3QxM97v5aaRNhj8v5oBhekw==} + engines: {node: '>=0.10'} + dequal@2.0.3: resolution: {integrity: sha512-0je+qPKHEMohvfRTCEo3CrPG6cAzAYgmzKyxRiYSSDkS6eGJdyVJm7WaYA5ECaAD9wLB2T4EEeymA5aFVcYXCA==} engines: {node: '>=6'} @@ -9578,6 +9579,10 @@ packages: resolution: {integrity: sha512-6xwYfHbajpoF0xLW+iwLkhwgvLoZDfjYfoFNu8ftMoXINzwuymNLd9u/KmwtdT2GbR+/Cz66otEGEVVUHX9QLQ==} engines: {node: '>=10.13.0'} + ioredis@5.6.1: + resolution: {integrity: sha512-UxC0Yv1Y4WRJiGQxQkP0hfdL0/5/6YvdfOOClRgJ0qppSarkhneSa6UvkMkms0AkdGimSH3Ikqm+6mkMmX7vGA==} + engines: {node: '>=12.22.0'} + ip-address@9.0.5: resolution: {integrity: sha512-zHtQzGojZXTwZTHQqra+ETKd4Sn3vgi7uBmlPoXVWZqYvuKmtI0l/VZTjqGmJY9x88GGOaZ9+G9ES8hC4T4X8g==} engines: {node: '>= 12'} @@ -10271,6 +10276,9 @@ packages: lodash.includes@4.3.0: resolution: {integrity: sha512-W3Bx6mdkRTGtlJISOvVD/lbqjTlPPUDTMnlXZFnVwi9NKJ6tiAk6LVdlhZMm17VZisqhKcgzpO5Wz91PCt5b0w==} + lodash.isarguments@3.1.0: + resolution: {integrity: sha512-chi4NHZlZqZD18a0imDHnZPrDeBbTtVN7GXMwuGdRH9qotxAjYs3aVLKc7zNOG9eddR5Ksd8rvFEBc9SsggPpg==} + lodash.isboolean@3.0.3: resolution: {integrity: sha512-Bz5mupy2SVbPHURB98VAcw+aHh4vRV5IPNhILUCsOzRmsTmSQ17jIuqopAentWoehktxGd9e/hbIXq980/1QJg==} @@ -11887,6 +11895,14 @@ packages: resolution: {integrity: sha512-6tDA8g98We0zd0GvVeMT9arEOnTw9qM03L9cJXaCjrip1OO764RDBLBfrB4cwzNGDj5OA5ioymC9GkizgWJDUg==} engines: {node: '>=8'} + redis-errors@1.2.0: + resolution: {integrity: sha512-1qny3OExCf0UvUV/5wpYKf2YwPcOqXzkwKKSmKHiE6ZMQs5heeE/c8eXK+PNllPvmjgAbfnsbpkGZWy8cBpn9w==} + engines: {node: '>=4'} + + redis-parser@3.0.0: + resolution: {integrity: sha512-DJnGAeenTdpMEH6uAJRK/uiyEIH9WVsUmoLwzudwGJUwZPp80PDBWPHXSAGNPwNvIXAbe7MSUB1zQFugFml66A==} + engines: {node: '>=4'} + redis@4.7.0: resolution: {integrity: sha512-zvmkHEAdGMn+hMRXuMBtu4Vo5P6rHQjLoHftu+lBqq8ZTA3RCVC/WzD790bkKKiNFp7d5/9PcSD19fJyyRvOdQ==} @@ -12389,6 +12405,9 @@ packages: resolution: {integrity: sha512-KJP1OCML99+8fhOHxwwzyWrlUuVX5GQ0ZpJTd1DFXhdkrvg1szxfHhawXUZ3g9TkXORQd4/WG68jMlQZ2p8wlg==} engines: {node: '>=6'} + standard-as-callback@2.1.0: + resolution: {integrity: sha512-qoRRSyROncaz1z0mvYqIE4lCd9p2R90i6GxW3uZv5ucSu8tU7B5HXUP1gG8pVZsYNVaXjk8ClXHPttLyxAL48A==} + static-eval@2.0.2: resolution: {integrity: sha512-N/D219Hcr2bPjLxPiV+TQE++Tsmrady7TqAJugLy7Xk1EumfDWS/f5dtBbkRCGE7wKKXuYockQoj8Rm2/pVKyg==} @@ -15753,6 +15772,8 @@ snapshots: '@img/sharp-win32-x64@0.33.5': optional: true + '@ioredis/commands@1.2.0': {} + '@isaacs/cliui@8.0.2': dependencies: string-width: 5.1.2 @@ -21950,6 +21971,8 @@ snapshots: delayed-stream@1.0.0: {} + denque@2.1.0: {} + dequal@2.0.3: {} detect-indent@6.1.0: {} @@ -23501,6 +23524,20 @@ snapshots: interpret@3.1.1: {} + ioredis@5.6.1: + dependencies: + '@ioredis/commands': 1.2.0 + cluster-key-slot: 1.1.2 + debug: 4.4.0 + denque: 2.1.0 + lodash.defaults: 4.2.0 + lodash.isarguments: 3.1.0 + redis-errors: 1.2.0 + redis-parser: 3.0.0 + standard-as-callback: 2.1.0 + transitivePeerDependencies: + - supports-color + ip-address@9.0.5: dependencies: jsbn: 1.1.0 @@ -23759,7 +23796,7 @@ snapshots: jest-worker@27.5.1: dependencies: - '@types/node': 20.17.12 + '@types/node': 22.14.0 merge-stream: 2.0.0 supports-color: 8.1.1 @@ -24136,6 +24173,8 @@ snapshots: lodash.includes@4.3.0: {} + lodash.isarguments@3.1.0: {} + lodash.isboolean@3.0.3: {} lodash.isequalwith@4.4.0: {} @@ -26281,6 +26320,12 @@ snapshots: indent-string: 4.0.0 strip-indent: 3.0.0 + redis-errors@1.2.0: {} + + redis-parser@3.0.0: + dependencies: + redis-errors: 1.2.0 + redis@4.7.0: dependencies: '@redis/bloom': 1.2.0(@redis/client@1.6.0) @@ -27031,6 +27076,8 @@ snapshots: dependencies: type-fest: 0.7.1 + standard-as-callback@2.1.0: {} + static-eval@2.0.2: dependencies: escodegen: 1.14.3 diff --git a/self-host/.env.example b/self-host/.env.example index 508222d393..3654f92d11 100644 --- a/self-host/.env.example +++ b/self-host/.env.example @@ -42,4 +42,4 @@ HONEYCOMB_API_KEY="xxx" GCLOUD_KEY_FILE='xxx' SELF_HOST="true" -VALKEY_URL='redis://cache:6379' \ No newline at end of file +VALKEY_HOST='cache' \ No newline at end of file From 84543b3ba57cb458dd22347171567f1639ceb102 Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Mon, 5 May 2025 22:36:31 -0500 Subject: [PATCH 08/15] Fix bugs caused by differences in hscan api between redis clients --- core/cache-handler.mjs | 22 ++++++++++++++-------- 1 file changed, 14 insertions(+), 8 deletions(-) diff --git a/core/cache-handler.mjs b/core/cache-handler.mjs index 7b2f01efaf..eaae539e21 100644 --- a/core/cache-handler.mjs +++ b/core/cache-handler.mjs @@ -34,18 +34,21 @@ function createRedisHandler({ async function revalidateTags(tag) { const tagsMap = /* @__PURE__ */ new Map(); let cursor = 0; + let remoteTagsPortion; do { - const remoteTagsPortion = await client.hscan( + [cursor, remoteTagsPortion] = await client.hscan( keyPrefix + sharedTagsKey, cursor, "COUNT", revalidateTagQuerySize ); - for (const { field, value } of remoteTagsPortion.tuples) { + + for (let i = 0; i < remoteTagsPortion.length; i += 2) { + const field = remoteTagsPortion[i]; + const value = remoteTagsPortion[i + 1]; tagsMap.set(field, JSON.parse(value)); } - cursor = remoteTagsPortion.cursor; - } while (cursor !== 0); + } while (cursor !== "0"); const keysToDelete = []; const tagsToDelete = []; for (const [key, tags] of tagsMap) { @@ -65,18 +68,21 @@ function createRedisHandler({ async function revalidateSharedKeys() { const ttlMap = /* @__PURE__ */ new Map(); let cursor = 0; + let remoteTagsPortion; do { - const remoteTagsPortion = await client.hscan( + [cursor, remoteTagsPortion] = await client.hscan( keyPrefix + sharedTagsKey, cursor, "COUNT", revalidateTagQuerySize ); - for (const { field, value } of remoteTagsPortion.tuples) { + + for (let i = 0; i < remoteTagsPortion.length; i += 2) { + const field = remoteTagsPortion[i]; + const value = remoteTagsPortion[i + 1]; ttlMap.set(field, Number(value)); } - cursor = remoteTagsPortion.cursor; - } while (cursor !== 0); + } while (cursor !== "0"); const tagsAndTtlToDelete = []; const keysToDelete = []; for (const [key, ttlInSeconds] of ttlMap) { From e234789f040155c4e0c108484a564983b42ad087 Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Mon, 5 May 2025 22:37:27 -0500 Subject: [PATCH 09/15] Fix retry strategy calculation --- core/cache-handler.mjs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/cache-handler.mjs b/core/cache-handler.mjs index eaae539e21..fb3508bf23 100644 --- a/core/cache-handler.mjs +++ b/core/cache-handler.mjs @@ -182,8 +182,11 @@ async function getCacheHandlerPromise() { lazyConnect: true, commandTimeout: 1000, retryStrategy: (times) => { + if (times >= 15) { + return; + } console.log("Retrying redis connection attempt:", times); - return (2 ^ times) + Math.random() * 1000; + return Math.pow(2, times) + Math.random() * 1000; }, }); From d289b0ab9725051b44a668c0202ba84591feae6e Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Mon, 5 May 2025 22:50:59 -0500 Subject: [PATCH 10/15] Add error logging to cache functions --- core/cache-handler.mjs | 111 +++++++++++++++++++++++------------------ 1 file changed, 63 insertions(+), 48 deletions(-) diff --git a/core/cache-handler.mjs b/core/cache-handler.mjs index fb3508bf23..23c89d6aa1 100644 --- a/core/cache-handler.mjs +++ b/core/cache-handler.mjs @@ -103,65 +103,80 @@ function createRedisHandler({ return { name: "pubpub-redis-strings", async get(key, { implicitTags }) { - const result = await client.get(keyPrefix + key); - if (!result) { - return null; - } - const cacheValue = JSON.parse(result); - if (!cacheValue) { - return null; - } - const sharedTagKeyExists = await client.hexists(keyPrefix + sharedTagsKey, key); - if (!sharedTagKeyExists) { - await client.unlink(keyPrefix + key); - return null; - } - const combinedTags = /* @__PURE__ */ new Set([...cacheValue.tags, ...implicitTags]); - if (combinedTags.size === 0) { - return cacheValue; - } - const revalidationTimes = await client.hmget( - revalidatedTagsKey, - Array.from(combinedTags) - ); - for (const timeString of revalidationTimes) { - if (timeString && Number.parseInt(timeString, 10) > cacheValue.lastModified) { + try { + const result = await client.get(keyPrefix + key); + if (!result) { + return null; + } + const cacheValue = JSON.parse(result); + if (!cacheValue) { + return null; + } + const sharedTagKeyExists = await client.hexists(keyPrefix + sharedTagsKey, key); + if (!sharedTagKeyExists) { await client.unlink(keyPrefix + key); return null; } + const combinedTags = /* @__PURE__ */ new Set([...cacheValue.tags, ...implicitTags]); + if (combinedTags.size === 0) { + return cacheValue; + } + const revalidationTimes = await client.hmget( + revalidatedTagsKey, + Array.from(combinedTags) + ); + for (const timeString of revalidationTimes) { + if (timeString && Number.parseInt(timeString, 10) > cacheValue.lastModified) { + await client.unlink(keyPrefix + key); + return null; + } + } + return cacheValue; + } catch (err) { + console.err("Cache get err", err); + throw err; } - return cacheValue; }, async set(key, cacheHandlerValue) { - const lifespan = cacheHandlerValue.lifespan; - const setTagsOperation = - cacheHandlerValue.tags.length > 0 - ? client.hset( - keyPrefix + sharedTagsKey, - key, - JSON.stringify(cacheHandlerValue.tags) - ) + try { + const lifespan = cacheHandlerValue.lifespan; + const setTagsOperation = + cacheHandlerValue.tags.length > 0 + ? client.hset( + keyPrefix + sharedTagsKey, + key, + JSON.stringify(cacheHandlerValue.tags) + ) + : void 0; + const setSharedTtlOperation = lifespan + ? client.hset(keyPrefix + sharedTagsTtlKey, key, lifespan.expireAt) : void 0; - const setSharedTtlOperation = lifespan - ? client.hset(keyPrefix + sharedTagsTtlKey, key, lifespan.expireAt) - : void 0; - await Promise.all([setTagsOperation, setSharedTtlOperation]); - if (typeof lifespan?.expireAt === "number") { - await client.set( - keyPrefix + key, - JSON.stringify(cacheHandlerValue), - "EXAT", - lifespan.expireAt - ); - } else { - await client.set(keyPrefix + key, JSON.stringify(cacheHandlerValue)); + await Promise.all([setTagsOperation, setSharedTtlOperation]); + if (typeof lifespan?.expireAt === "number") { + await client.set( + keyPrefix + key, + JSON.stringify(cacheHandlerValue), + "EXAT", + lifespan.expireAt + ); + } else { + await client.set(keyPrefix + key, JSON.stringify(cacheHandlerValue)); + } + } catch (err) { + console.error("Cache set error", err); + throw err; } }, async revalidateTag(tag) { - if (isImplicitTag(tag)) { - await client.hset(revalidatedTagsKey, tag, Date.now()); + try { + if (isImplicitTag(tag)) { + await client.hset(revalidatedTagsKey, tag, Date.now()); + } + await Promise.all([revalidateTags(tag), revalidateSharedKeys()]); + } catch (err) { + console.error("Cache revalidate error", err); + throw err; } - await Promise.all([revalidateTags(tag), revalidateSharedKeys()]); }, async delete(key) { await client.unlink(keyPrefix + key); From d6035482614c1de88d2131123f580c07a61f69d8 Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Mon, 5 May 2025 23:02:21 -0500 Subject: [PATCH 11/15] Make sure fallback behavior is instant when client is reconnecting --- core/cache-handler.mjs | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/core/cache-handler.mjs b/core/cache-handler.mjs index 23c89d6aa1..5e08069d9d 100644 --- a/core/cache-handler.mjs +++ b/core/cache-handler.mjs @@ -7,15 +7,6 @@ import { CacheHandler } from "@neshca/cache-handler"; import { isImplicitTag } from "@neshca/cache-handler/helpers"; import Redis from "ioredis"; -// A cache that always misses - intended to let us disable caching when redis is unavailable. Should -// be replaced if there's a better way to do that. -const dummyHandler = { - name: "no-cache", - get: () => undefined, - set: () => undefined, - revalidateTag: () => undefined, -}; - var REVALIDATED_TAGS_KEY = "__revalidated_tags__"; /** @@ -31,6 +22,13 @@ function createRedisHandler({ sharedTagsTtlKey = "__sharedTagsTtl__", revalidateTagQuerySize = 1e4, }) { + function assertClientIsReady() { + if (!client.status === "ready") { + // Throwing here ensures that we immediately fall back to uncached behavior, rather than + // waiting for the command timeout + throw new Error("Redis client is not ready yet or connection is lost."); + } + } async function revalidateTags(tag) { const tagsMap = /* @__PURE__ */ new Map(); let cursor = 0; @@ -104,6 +102,7 @@ function createRedisHandler({ name: "pubpub-redis-strings", async get(key, { implicitTags }) { try { + assertClientIsReady(); const result = await client.get(keyPrefix + key); if (!result) { return null; @@ -139,6 +138,7 @@ function createRedisHandler({ }, async set(key, cacheHandlerValue) { try { + assertClientIsReady(); const lifespan = cacheHandlerValue.lifespan; const setTagsOperation = cacheHandlerValue.tags.length > 0 @@ -169,6 +169,7 @@ function createRedisHandler({ }, async revalidateTag(tag) { try { + assertClientIsReady(); if (isImplicitTag(tag)) { await client.hset(revalidatedTagsKey, tag, Date.now()); } @@ -216,9 +217,6 @@ async function getCacheHandlerPromise() { } if (redisClient?.status !== "ready") { console.error("Failed to initialize caching layer."); - global.cacheHandlerConfigPromise = null; - global.cacheHandlerConfig = { handlers: [dummyHandler] }; - return global.cacheHandlerConfig; } const redisCacheHandler = createBufferStringHandler( From ec1fffc55fe9eaab46f366870e5a658e8a92a2f6 Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Mon, 5 May 2025 23:12:49 -0500 Subject: [PATCH 12/15] Also fix retry logic for lib/redis client --- core/lib/redis.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/core/lib/redis.ts b/core/lib/redis.ts index 7a77c0293c..c7b99bfada 100644 --- a/core/lib/redis.ts +++ b/core/lib/redis.ts @@ -15,7 +15,10 @@ export const getRedisClient = async () => { lazyConnect: true, commandTimeout: 1000, retryStrategy: (times) => { - return (2 ^ times) + Math.random() * 1000; + if (times >= 15) { + return; + } + return Math.pow(2, times) + Math.random() * 1000; }, }); } From 921b6f7e426ade3bbbe40edabdd2aeea754fcfdb Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Mon, 5 May 2025 23:36:32 -0500 Subject: [PATCH 13/15] Add timestamps to logs output in ci --- .github/workflows/e2e.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml index bf096df68b..9709483221 100644 --- a/.github/workflows/e2e.yml +++ b/.github/workflows/e2e.yml @@ -132,7 +132,7 @@ jobs: - name: Print container logs if: ${{failure() || cancelled()}} - run: docker compose -f docker-compose.test.yml --profile integration logs + run: docker compose -f docker-compose.test.yml --profile integration logs -t env: INTEGRATION_TESTS_IMAGE: ${{steps.label.outputs.core_label}} JOBS_IMAGE: ${{steps.label.outputs.jobs_label}} From 8577e1130b630ddf730fa0bf1ffa8f5434128f2e Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Mon, 5 May 2025 23:45:31 -0500 Subject: [PATCH 14/15] Use proper healthcheck and set restart policy --- .../terraform/modules/container-generic/main.tf | 11 ++++++++--- .../terraform/modules/container-generic/variables.tf | 6 ++++++ infrastructure/terraform/modules/deployment/main.tf | 5 +++-- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/infrastructure/terraform/modules/container-generic/main.tf b/infrastructure/terraform/modules/container-generic/main.tf index fc253030d2..07133615aa 100644 --- a/infrastructure/terraform/modules/container-generic/main.tf +++ b/infrastructure/terraform/modules/container-generic/main.tf @@ -49,6 +49,11 @@ module "ecs_service" { containerPort = var.listener.to_port }] : [] + restartPolicy = { + enabled = true + restartAttemptPeriod = 60 + } + # use concat() to add a computible variable environment = concat( var.configuration.environment, @@ -158,13 +163,13 @@ resource "aws_lb_target_group" "this" { # when the container does not provide a more meaningful # one. health_check { - path = "/legacy_healthcheck" - interval = "30" + path = coalesce(var.health_check_path, "/legacy_healthcheck") + interval = "5" protocol = "HTTP" matcher = "200" timeout = "5" unhealthy_threshold = "3" - healthy_threshold = "5" + healthy_threshold = "3" } } diff --git a/infrastructure/terraform/modules/container-generic/variables.tf b/infrastructure/terraform/modules/container-generic/variables.tf index 92de7ba07a..5f2ed802be 100644 --- a/infrastructure/terraform/modules/container-generic/variables.tf +++ b/infrastructure/terraform/modules/container-generic/variables.tf @@ -99,3 +99,9 @@ variable "command" { default = [] type = list(string) } + +variable "health_check_path" { + description = "A path to an endpoint on the container suitable for use as a health check" + type = string + default = null +} diff --git a/infrastructure/terraform/modules/deployment/main.tf b/infrastructure/terraform/modules/deployment/main.tf index acbab8754c..597fb9a0d0 100644 --- a/infrastructure/terraform/modules/deployment/main.tf +++ b/infrastructure/terraform/modules/deployment/main.tf @@ -52,8 +52,9 @@ module "service_core" { service_name = "core" cluster_info = module.cluster.cluster_info - repository_url = var.ecr_repository_urls.core - nginx_image = "${var.ecr_repository_urls.nginx}:latest" + repository_url = var.ecr_repository_urls.core + nginx_image = "${var.ecr_repository_urls.nginx}:latest" + health_check_path = "/api/health" listener = { service_name = "core" From ebb20d4295b8fde88170d804fd2ff93b0be45255 Mon Sep 17 00:00:00 2001 From: Kalil Smith-Nuevelle Date: Tue, 6 May 2025 08:15:42 -0500 Subject: [PATCH 15/15] Lower health check timeout --- infrastructure/terraform/modules/container-generic/main.tf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/infrastructure/terraform/modules/container-generic/main.tf b/infrastructure/terraform/modules/container-generic/main.tf index 07133615aa..0bfed825e9 100644 --- a/infrastructure/terraform/modules/container-generic/main.tf +++ b/infrastructure/terraform/modules/container-generic/main.tf @@ -167,7 +167,7 @@ resource "aws_lb_target_group" "this" { interval = "5" protocol = "HTTP" matcher = "200" - timeout = "5" + timeout = "2" unhealthy_threshold = "3" healthy_threshold = "3" }