Wire Valkey cache scaffolding#35
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9d85414204
ℹ️ 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".
| response, err := okfcache.GetOrCompute(r.Context(), s.cache, key, 2*time.Minute, func(ctx context.Context) (galleryCatalogResponse, error) { | ||
| return s.galleryCatalogResponse(ctx, limit, offset, filters) | ||
| }) |
There was a problem hiding this comment.
Invalidate catalog cache after scheduled extraction
In deployments with Scheduler.Enabled, the scheduler writes new DownloadedPhoto rows in internal/scheduler/scheduler.go:62-99 but this new catalog cache is only epoch-bumped from the API extraction and favorite handlers. If a catalog page has been cached here before a scheduled run downloads photos, /api/v1/gallery/catalog continues serving the old photo list/facets for the cache TTL instead of reflecting the completed import; wire the same BumpEpoch invalidation into scheduled extraction or the shared write path.
Useful? React with 👍 / 👎.
| type Client struct { | ||
| r redis.Cmdable | ||
| logger zerolog.Logger | ||
| passthrough bool | ||
| } |
There was a problem hiding this comment.
Guard the passthrough flag across requests
During a Valkey error while the server is handling concurrent requests, this bool is written in error paths such as GetOrCompute, BumpEpoch, Delete, and Epoch while other goroutines read it via Passthrough(). Because Server.cache is shared by all HTTP handlers, those unsynchronized reads/writes create a Go data race and can leave cache behavior undefined; use an atomic or mutex-protected state instead.
Useful? React with 👍 / 👎.
6d8ef4b to
22f53b1
Compare
Refs #20
What changed
valkey:6379) andCACHE_*env overrides for deployment secret-store/Infisical values.internal/cachewith go-redis RESP2 pool setup, startup ping passthrough mode, cache-asideGetOrCompute, per-keySET NX PXlocks, TTL jitter, stale envelopes, epoch bumping, targeted delete, OK Folio key helpers, and stable filter hashing.okf:epochafter API extractions, scheduled extractions, favorite edits, and retry status updates; favorite/retry edits also delete the per-photo cache key.atomic.Boolso concurrent handlers do not race when Valkey errors flip the client into passthrough.Review fixes
Verification
Deferred: deployed Valkey integration verification is intentionally left for the later stack verification issue; this PR is unit-tested with miniredis only.
Maestro-Backend: codex (0-13m); codex (13m-end, in_place_respawn)
Greptile Summary
This PR adds optional Valkey caching for gallery catalog reads. The main changes are:
Confidence Score: 5/5
The cache scaffolding is optional and keeps existing behavior available when caching is disabled.
The changed cache, configuration, API invalidation, and key-helper paths are covered by focused unit tests and the repository verification reported successful Go and frontend builds.
What T-Rex did
Reviews (2): Last reviewed commit: "Fix cache invalidation review findings" | Re-trigger Greptile