fix(cache): hotpath db queries caching - #2662
Conversation
Two hot-path lookups ran on every gateway request instead of being served from the Drizzle/Redis cache, hammering Postgres under load: - getEffectiveRateLimit used the uncached `db` client, so the rate_limit query hit Postgres on every request (swrWrap only mirrors to Redis on DB error, not on the happy path). Switch to the cached `cdb` client; the WHERE clause is time-independent so the cache key is stable. - getEffectiveDiscount put `now` (new Date()) in the SQL WHERE clause. The cached client keys on hashQuery(sql, params), so a per-request millisecond timestamp made the key unique every call -> 0% cache hit. Keep the SQL time-independent and evaluate expiry in JS, mirroring the existing findEffectiveDiscount fix. Verified e2e against a local gateway: a warm request now does 0 Postgres reads (was ~14), and a 60-request concurrent burst hit Postgres only 3 times total (cold-cache races) instead of ~840. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Allow pointing the load test at a local gateway and varying the model without editing the script. Also move the API key to an env var and add categorized error reporting (per-category counts + sampled messages). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
WalkthroughDiscount expiry filtering was moved from SQL to JavaScript, rate-limit lookups now use the cached DB client with updated tests, a Node.js load-test CLI was added, and an e2e verifies chat completions metadata reads are served from cache. ChangesDatabase Query Optimization
Load Testing CLI
Chat completions cache e2e
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
changed the title
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3f8a9ffcbb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const rateLimits = await cdb | ||
| .select({ |
There was a problem hiding this comment.
When an admin creates or deletes a rate limit to stop traffic, this cached lookup can keep returning the previous result because the admin handlers mutate tables.rateLimit through the uncached db client (for example apps/api/src/routes/admin.ts uses db.insert/db.delete for these routes), so RedisCache.onMutate is not invoked for the Drizzle cache. A gateway that has already cached “no limit” for an org/provider/model will continue allowing requests until the cache entry expires, regressing the previous immediate enforcement behavior for emergency throttles; either invalidate this cache from the admin mutations or keep this lookup uncached for enforcement-sensitive changes.
Useful? React with 👍 / 👎.
Add an e2e regression test that warms the gateway caches, then spies on the shared pg pool during a fresh chat completion and asserts zero SELECTs hit the cached metadata tables (api_key, project, organization, provider_key, rate_limit, discount, routing metrics, ...). Both the cached (cdb) and uncached (db) clients share this pool, so the spy catches any query that actually reaches Postgres — protecting against both regression vectors: an unstable cache key (per-request Date param) and switching a hot lookup to the uncached client. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/gateway/src/chat-cache-no-db.e2e.ts`:
- Around line 81-89: cachedTableReads only matches the exact substring from
"<table>" and misses variants like schema-qualified ("public"."api_key"), joins,
aliases, or unquoted identifiers; update cachedTableReads to build a pattern
from CACHED_READ_TABLES that tests for table occurrences following SQL keywords
(e.g., FROM, JOIN, INTO) with optional schema qualification and optional quoting
or unquoted identifiers and word boundaries, and use that pattern against the
lowered SQL to return true for any of those variants; reference the
cachedTableReads function and the CACHED_READ_TABLES constant when making this
change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 375e4074-0cac-4cc3-965b-80aef278a4e0
📒 Files selected for processing (1)
apps/gateway/src/chat-cache-no-db.e2e.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7e8bd82c27
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // (hot, per-provider-candidate) lookup would query Postgres on every | ||
| // request. Keeping the SQL time-independent lets the cache key stay stable | ||
| // while expiry is still evaluated fresh on each call. | ||
| const rows = await cdb |
There was a problem hiding this comment.
With this lookup now using a stable cdb cache key, admin-created or deleted discounts can be ignored until the 60s Drizzle cache TTL expires, because the admin discount routes mutate tables.discount through the uncached db client (apps/api/src/routes/admin.ts inserts/deletes at lines 3165/3183 and 3273/3291), so RedisCache.onMutate is not invoked. In production this means a newly added discount may not be applied, or a removed discount may keep affecting gateway pricing/cost accounting, for requests whose org/provider/model tuple was already warmed in cache; invalidate the discount cache on these mutations or keep this enforcement/pricing lookup uncached.
Useful? React with 👍 / 👎.
1 participant
Two hot-path lookups ran on every gateway request instead of being served from the Drizzle/Redis cache, hammering Postgres under load:
getEffectiveRateLimitused the uncacheddbclient, so therate_limitquery hit Postgres on every request (swrWraponly mirrors to Redis on DB error, not on the happy path). Switch to the cachedcdbclient; the WHERE clause is time-independent so the cache key is stable.getEffectiveDiscountputnow(new Date()) in the SQL WHERE clause. The cached client keys onhashQuery(sql, params), so a per-request millisecond timestamp made the key unique every call → 0% cache hit. Keep the SQL time-independent and evaluate expiry in JS, mirroring the existingfindEffectiveDiscountfix.Verification
Measured e2e against a local gateway (Postgres statement logging, repeated requests with unique prompts so the response cache can't mask the path):
Regression guard
Adds
apps/gateway/src/chat-cache-no-db.e2e.ts: it warms the gateway caches, then spies on the shared pgpoolduring a fresh chat completion and asserts zero SELECTs reach the cached metadata tables (api_key,project,organization,provider_key,rate_limit,discount, routing metrics, …). Both the cached (cdb) and uncached (db) clients share this pool, so the spy catches any query that actually reaches Postgres — protecting against both regression vectors: an unstable cache key (per-requestDateparam) and switching a hot lookup back to the uncached client. Confirmed the test fails when either bug is reintroduced.Also adds
API_URL/MODELenv overrides toscripts/loadtest.mjsso the load test can target a local gateway.Summary by CodeRabbit
Improvements
Tests
Chores