feat(run-store): Redis-backed store for the run execution-state log - #4754
Conversation
Thread runId into #observeSizes so all three high-water logger.warn payloads name the run, per spec. Adds a capturing-logger test proving the warning fires with the run id above the mark, and stays silent under a high threshold.
Record actual byte values instead of booleans and partition per append so a mis-wired metric can't hide behind a flat toContain. Cover the succeeding direction of expectedCur: "" against a genuinely unset cur, assert recordCycleMismatch fires, pin the CRC16 helper against a known vector plus a negative control, bound the prefixed cycle-key TTL, prove cur is untouched by a stale CAS, and add a matching-environment getSince with a non-empty window.
A retried append whose write already succeeded advanced cur to its own id, so the CAS above the duplicate guard saw its own id as a stale expectedCur and reported forked instead of duplicate. Snapshot ids are unique per append, so checking duplicate first is always correct.
Adds the reachable-in-tests, unreachable-in-prod case where the Lua- chosen head is dropped by the TS env filter. It surfaced a real bug: headOrder stayed attached to whatever row ended up last after filtering, donating the dropped head's waitpoints to it. Track whether the actual head row survives and only then attach its order.
… order Implements the spec's read-side check that was previously unwritten: a sentinel problem (an empty order string meant both "read as empty" and "not read for this row" in getSince's tail rows) blocked it. #decode now takes an explicit orderKnown flag, runs the count-vs-length check only when the order was actually read, and never sets completedWaitpointIds on a row whose order wasn't read -- which also removes the need to delete it again afterward.
|
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughAdded a Redis-backed 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…is-snapshot-store
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
2 participants
Adds
RedisSnapshotStoreto@internal/run-store: a Redis-backed, append-only store for a run's execution-state log, as an alternative to keeping that log in Postgres.Nothing constructs it. No existing code path can reach it, so merging this changes no behaviour. The store, the wiring that would use it, and the switch that would enable it are deliberately separate changes.
Design
Four keys per run, plus one key per wait cycle, all sharing a
{runId}hash tag. Every mutation for a run therefore lands in one cluster slot, and each operation is a single Lua script.No script mints a key name. Dynamic keys are derived from
KEYS[1]by string surgery, because ioredis applieskeyPrefixonly to the KEYS array: a key built inside Lua would be unprefixed while the client wrote a prefixed one.Retention is keyed to run completion. A non-terminal run's keys carry no expiry at all, since a suspended run can wait indefinitely with nothing left to refresh a TTL. The terminal transition sets the completion expiry once, and a write arriving after completion re-applies that same expiry rather than a live one, so a stale client cannot resurrect a key.
Entry JSON round-trips byte for byte. No script calls
cjson, and the values the store assigns itself live in their own hash fields instead of being patched into the caller's document.Sizes are observed, never enforced. Entry and cycle-key bytes are recorded, with a warning above a configurable mark. Nothing rejects, truncates, or spills.
appendtakes an optional expected-current-snapshot argument. Left out, it advances the pointer unconditionally, matching the Postgres behaviour it replaces. Supplied, it advances only on a match and otherwise reports the conflict without writing.Covered by 48 tests against a real Redis container, including the retention transitions, the single-slot guarantee under a key prefix, and tenant-scoped reads.