perf(run-engine,run-store): one execution snapshot per triggered run - #4419
Conversation
|
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughRun triggering now generates and persists an initial snapshot ID with 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 2📝 Generate docstrings 💡
🛠️ Fix failing CI checks 💡
🧪 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.
a515874 to
f2eed68
Compare
@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: |
f2eed68 to
a9d0749
Compare
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
A non-delayed run was getting RUN_CREATED nested in the run-create transaction followed by a separate QUEUED write. It now gets a single QUEUED snapshot inside the create, with the trigger path only publishing to the queue.
…d trigger write The store-routing counters now also count createRun, since the trigger path's QUEUED snapshot is written nested in the run create. The replica-lag test starts the run attempt so it still has a middle snapshot to exercise rather than degenerating to an empty comparison.
… primary startRunAttempt resolves the background worker task through readOnlyPrisma, and this test configures a deliberately empty schema-only replica, so using it to add a snapshot failed on the replica read instead of exercising the lag scenario. Write the snapshot with the file's own primary-side helper.
The run's createdAt is caller-supplied and the scheduler sets it to the exact schedule time, so using it for the emitted event backdated the queued entry on every scheduled run's timeline. Use the write moment, as the other nested-create paths do.
The trigger path writes this snapshot nested in the run create and emits the timeline event itself, while every re-enqueue writes it through enqueueRun. Keep the status and the default description in one place so the persisted row and the emitted event cannot drift apart.
The store-routing counter separated nested creates from standalone ones, so it can assert the trigger path makes no standalone snapshot write at all rather than just more writes than before. A new case drives a run through suspend and resume to prove a re-enqueue still writes its own QUEUED snapshot.
5859358 to
ed4836e
Compare
2 participants
A non-delayed run used to get two execution snapshots the moment it was triggered:
RUN_CREATEDnested in the run-create transaction, immediately followed byQUEUEDfrom its ownBEGIN/INSERT/COMMIT. It now gets a singleQUEUEDsnapshot written inside the create, and the trigger path only publishes to the queue. One fewer row per run onTaskRunExecutionSnapshot, and one fewer round trip on the trigger hot path.EnqueueSystemgains apublishRunseam that enqueues without writing a snapshot. Every re-enqueue path (waitpoint resume, checkpoint restore, delayed enqueue, pending version, retry requeue) still callsenqueueRunand writes its ownQUEUED, so only the first enqueue changes. TheQUEUEDsnapshot still commits before the queue message, so a dequeue sees a dequeueable status exactly as before.Two things for reviewers. Nesting the write skips
createExecutionSnapshot, which is what emitsexecutionSnapshotCreatedand therefore the run timeline's[engine] QUEUEDentry, so the trigger path now emits it directly, the same way the dequeue and attempt-start paths already do for their nested creates. AndRUN_CREATEDis still written when a dequeued run has no background worker yet, so the status and bothstatuses.tshelpers stay live and existing rows keep reading correctly.Delayed runs are untouched:
DELAYEDthenQUEUEDare two genuinely different moments and stay two snapshots.Rollback is a revert. Create-and-enqueue happen in one request in one process, so no in-flight run needs both code paths to agree during a rollout.
One note for whoever debugs this path later. The
QUEUEDsnapshot now commits before the queue publish, so a failed publish leaves the run recorded asQUEUEDwith no queue message. That state was already reachable, since the publish was never part of the snapshot transaction, but it used to be recorded asRUN_CREATED, which was distinctive because it never otherwise persisted.QUEUEDwith no message is indistinguishable from a run waiting on a concurrency slot, so trigger-time publish failure is now one more cause of an apparently stuck queued run.