feat(api):refactor alias cache to redis - #1914
Conversation
df24895 to
75bb7ca
Compare
75bb7ca to
8b5eab4
Compare
| @@ -97,8 +103,8 @@ func (c *AliasCache) Resolve(ctx context.Context, identifier string, namespaceFa | |||
| return nil, err | |||
| } | |||
|
|
|||
| // lookup performs a single lookup (cache then DB) for namespace/alias. | |||
| // Caches both positive hits and negative hits to avoid repeated DB queries. | |||
| // lookup performs a single lookup using memory → Redis → DB for namespace/alias. | |||
There was a problem hiding this comment.
is it actually using anything in memory?
ba29230 to
c36f5c7
Compare
8b5eab4 to
f61a55e
Compare
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
|
|
||
| return info, nil | ||
| // Track alias key in reverse index for InvalidateByTemplateID | ||
| c.trackReverseKey(ctx, originalKey, idKey, info) |
There was a problem hiding this comment.
Race between cache fill and reverse index invalidation
Low Severity
cacheByTemplateID runs inside fetchFromDB's defer, which executes before GetOrSet writes the original alias key to Redis. This means trackReverseKey registers the originalKey in the reverse index before that key actually exists in Redis. If InvalidateByTemplateID runs in this narrow window, the DEL for the original key is a no-op, and then GetOrSet writes it afterward — leaving a stale cache entry that the reverse index no longer tracks. It self-heals via TTL expiry but could serve stale data for up to aliasCacheTTL.
Additional Locations (1)
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 68cbe79f82
ℹ️ 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".
| local members = redis.call("SMEMBERS", KEYS[1]) | ||
| redis.call("DEL", KEYS[1]) | ||
| return members |
There was a problem hiding this comment.
The Lua pop script deletes the reverse-index set before the subsequent DEL pipeline runs, so if the pipeline times out or fails (for example, templates with many cached aliases or transient Redis slowness under the shared 2s timeout in InvalidateByTemplateID), we permanently lose the member list needed for retries and stale alias/id cache entries can survive until TTL expiry. This turns a transient Redis failure into persistent stale routing for that template.
Useful? React with 👍 / 👎.
Note
Medium Risk
Touches core cache invalidation and alias resolution paths and introduces Redis scripts/pipelining, so bugs could cause stale or missing alias mappings across processes. Behavioral intent is preserved and covered by updated integration tests against Redis.
Overview
Refactors template alias resolution caching to use Redis instead of in-process memory, including Redis-backed negative caching tombstones and backfilling lookups by template ID.
Adds a Redis reverse-index per
templateID(set + Lua pop) soInvalidateByTemplateIDcan efficiently delete all alias/id cache keys across a cluster, updates invalidation call sites to passcontext.Context(usingcontext.WithoutCancelwhere needed), and standardizes Redis operations on a sharedRedisDefaultTimeout(removing per-cache timeouts and adjusting tests to assert Redis key presence/deletion).Written by Cursor Bugbot for commit 68cbe79. This will update automatically on new commits. Configure here.