feat: route new task runs to a parallel task_run_v2 table#4000
Draft
d-cs wants to merge 72 commits into
Draft
Conversation
Replaces the seven throwing stubs on PostgresRunStore with verbatim relocations of the Prisma statements from runAttemptSystem: startAttempt, completeAttemptSuccess, recordRetryOutcome, requeueRun, recordBulkActionMembership, cancelRun, and failRunPermanently. Each method splices the caller-supplied select/include into the Prisma call. Tests use real Postgres containers and cover each method including edge cases (append semantics, conditional fields in cancelRun).
…y-clear, and array-append methods
Replaces the seven throwing stubs in PostgresRunStore with verbatim-relocated
Prisma statements sourced from delayedRunSystem, debounceSystem, updateMetadata,
idempotencyKeys, resetIdempotencyKey, batchTriggerV3, and the realtime-stream
route handlers.
- rescheduleRun: writes delayUntil always; queueTimestamp when provided; nested
DELAYED executionSnapshot when snapshot arg provided
- enqueueDelayedRun: sets status PENDING + queuedAt
- rewriteDebouncedRun: pass-through update with associatedWaitpoint include
- updateMetadata: optimistic-lock path (updateMany with version predicate) or
direct path (update without predicate); both return { count }
- clearIdempotencyKey: three discriminated-union branches — byId clears both
columns, byPredicate clears both, byFriendlyIds clears only idempotencyKey
- pushTags: push-append to runTags array; returns { updatedAt }
- pushRealtimeStream: push-append to realtimeStreams array; returns void
…bapp BaseService Add RunStore field to SystemResources, instantiate PostgresRunStore in RunEngine constructor (after prisma/readOnlyPrisma are set), and expose it on the resources object passed to all systems. Create a webapp singleton (runStore.server.ts) and thread it as a default parameter into BaseService so subclasses can access it without changes.
…eave-unchanged semantics)
…s through RunStore
… debounce writes through RunStore
… writes through RunStore
The service statically imported the db.server-backed runStore singleton, which dragged the Prisma client into otherwise-light test module graphs and opened an eager connection to DATABASE_URL on import. The metadata service test then threw an unhandled connection error whenever no database was reachable at the configured address. Make runStore a required constructor option, pass the singleton at the production construction site, and inject a testcontainer-backed store in the tests.
Add findRun, findRunOrThrow and findRuns to RunStore, mirroring the existing write methods. They pass where/select/include through the same Prisma generics and default to the read replica, while letting the caller pass the writer or a transaction client when needed. This lets Postgres reads of TaskRun be routed through the store the same way writes already are. Additive only; no call sites change yet.
Add a no-args overload to findRun, findRunOrThrow and findRuns that returns the whole TaskRun row, for callers that read a run without a select or include.
Relocate the direct TaskRun reads in the engine and its systems to the RunStore read methods, preserving the exact client (writer, replica, or transaction) at each site. Behavior-preserving; the engine test suite is unchanged.
…tore Relocate the direct TaskRun reads in webapp services, run-engine concerns, realtime, mollifier and metadata to the RunStore read methods, preserving the exact client (writer, replica, or transaction) at each site. The run hydrator now receives the store by injection. Behavior-preserving.
Relocate the dashboard presenter TaskRun reads to the RunStore read methods, preserving the exact client per site. Behavior-preserving.
…store Relocate the route and loader TaskRun reads to the RunStore read methods, preserving the exact client per site, including the replica-resolve then writer-recheck realtime paths. Behavior-preserving.
…store Decompose the three reads that pulled TaskRun in through a parent model's relation include (alert, batch results, attempt dependencies): query the parent without the include, hydrate the run(s) via RunStore in a single batched read, and stitch them back. Preserves field selection, ordering, null handling and the query client. Adds container-backed tests for the batch-results and cancel-dependencies paths.
…tover The recovery script joins TaskRunExecutionSnapshot to TaskRun in raw SQL, so it is the one TaskRun read not routed through the run store. Add a note to revisit it at table cutover.
@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: |
![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.
Poll for the ClickHouse row with a bounded deadline instead of a fixed sleep, which is flaky under replication lag variance, and stop the replication service in a finally block so a failing assertion cannot leak it into later tests.
…on id-list reads findRuns now throws when given skip: offset pagination cannot span the two run tables, where each would independently skip N rows from its own result rather than N from the merged result. For an id-list predicate (id in [...]), it now queries only the table whose id format can contain those ids, avoiding a wasted query against an empty task_run_v2 while it is unpopulated during rollout.
…e merge collation A single-format id-list narrows findRuns to one physical table, but the ordered+limited path still built the cross-table comparator and threw the time-key guard; it now delegates natively to the one table (Postgres orders within a single table fine). Separately, the in-memory merge comparator ordered strings by code unit while the Postgres keyset continuation orders by the database collation (en_US); switching the comparator to localeCompare makes them agree, so a tied-createdAt boundary spanning both tables no longer skips or duplicates a row.
The pre-gate idempotency claim was eligible only when the org was on the mollifier. Concurrent same-key triggers that straddle a runTableV2 flip can mint into different physical tables, whose per-table unique constraints can't see each other, so two runs could share one key. The claim is now also eligible when the org is cut over to the v2 run table, serialising those triggers through Redis.
…warn when missing A v2 run DELETE needs the full old row so its ClickHouse soft-delete tombstone carries organization and environment ids; under the default replica identity those are dropped and the tombstone is lost. A migration sets REPLICA IDENTITY FULL on task_run_v2 rather than relying on an out-of-band step, and the replication client now warns when any co-published table that publishes UPDATE/DELETE lacks FULL. Adds a replication test for the v2 DELETE tombstone.
A v2 run can reference a legacy parent/root, or have legacy children, when a hierarchy straddles a runTableV2 flip. Prisma relation selects are bound to one table, so the run, span, and API-retrieve presenters returned null parent/root and dropped cross-table children. They now resolve parent/root by id (RunStore routes by id format) and children by a both-table predicate, via a shared hydrateParentAndRoot/hydrateChildRuns helper.
When a non-id predicate matches a row in both physical tables, findFirstAcrossTables now returns the v2 copy instead of legacy. Under this PR a run is in exactly one table (createRun routes by id format), so this is a no-op today; it forward-aligns with the later slow legacy to v2 migration, which copies a run into task_run_v2 (the canonical, operated-on copy) before operating. A comment in findRuns marks the matching dedup-by-id work for that migration PR.
![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.
TaskRunV2 declared implicit many-to-many relations (tags, connectedWaitpoints) whose join tables were never created by any migration and are absent from the database. Nothing reads them (v2 run tags use the scalar runTags array), so they were pure schema-vs-migration drift. Removing them makes the schema match the database with no migration.
findRuns rejects a Prisma cursor or a negative take on a both-tables read (neither can span two tables) instead of silently returning a wrong or empty result, and tablesForWhere now routes a plain id or friendlyId equality to the single matching table by id format, not just id:{in} lists. Also documents that the cross-table merge comparator assumes the en_US database collation and the COLLATE C fix needed for other collations.
… off Concurrent same-key triggers that straddle a runTableV2 flag flip can mint into different physical tables (cuid to TaskRun, ksuid to task_run_v2), whose per-table unique constraints cannot see each other, so neither insert conflicts and two runs share one key. The pre-gate claim now resolves its backend through a claim-only Redis buffer when the mollifier buffer is absent, so it serialises these triggers instead of falling open. v2-cutover orgs are claim-eligible for every idempotency-keyed trigger, including triggerAndWait, debounce, and one-time-use tokens, and the claim-resolved path blocks the parent on the winner's waitpoint.
A run routed to task_run_v2 was invisible to the Electric realtime feed, whose shapes were bound to the TaskRun table, so subscribeToRun, useRealtimeRun, and run polling returned nothing for those runs. Single-run subscriptions now route the shape to the correct table by id format, and the tag and batch feeds run two upstream shapes (TaskRun and task_run_v2) merged under one composite cursor the client round-trips opaquely, so no SDK change is needed.
runTableV2 is resolved per organization only, so a global toggle on the admin flags page did nothing. Mark it read-only there to remove the misleading control; per-org control stays on the org dialog.
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
…ment The parent/root/child hydration that resolves a run's hierarchy across both run tables looked runs up by id alone. Those pointers are now plain scalars with no foreign-key enforcement, so a stale or malformed pointer could resolve to a run in another environment and leak its metadata through the run and span presenters. Scope every lookup to the run's runtimeEnvironmentId, restoring the same-environment guarantee the table-bound relation select used to provide.
When the two-table realtime shape merge returns as soon as one upstream shape yields, it aborts the other fetch and returns immediately. That promise was left without a rejection handler, so the abort could surface as an unhandled rejection on the server. Attach a no-op catch to the aborted fetch.
The two-table shape merge could leave one upstream fetch pending without a rejection handler when it aborts the race loser or rethrows from the catch block. Attach a detached no-op catch to both fetches up front so an abandoned fetch can never surface as an unhandled rejection on any path. Also document that a tag/batch subscription opens two upstream Electric connections while an org spans both run tables.
…ectric shapes Electric realtime shapes are bound to a single table, so a task_run_v2 run was invisible to realtime subscriptions. The previous approach merged two Electric shapes per tag/batch feed under a composite cursor, which doubled Electric long-poll connections for those feeds. Electric is being retired in favor of the native realtime backend, which is table-agnostic and already observes both run tables, so that merge is throwaway. Drop the Electric dual-shape merge (revert realtimeClient to its single-table form, remove the merge module) and instead gate runTableV2 on the native backend: a run only routes to task_run_v2 when the deployment has native realtime enabled and the org's realtimeBackend flag is native. This keeps v2 runs realtime-observable without touching Electric, and the gate auto-satisfies once Electric is removed and native is the default. The idempotency pre-gate claim inherits the same gate.
Completes the Electric-merge removal: a run only routes to task_run_v2 when the deployment has native realtime enabled and the org's realtimeBackend flag is native. Electric shapes are single-table and can't observe a v2 run, so without this gate a v2 run would be realtime-invisible. shouldUseV2RunTable takes the native-realtime master switch as a parameter (kept env-free for unit tests); the trigger mint site and the idempotency pre-gate claim both pass it.
Restore the both-table Electric shape merge so tag-list and batch realtime feeds observe runs in TaskRun and task_run_v2 together, and gate the v2 run table on the runTableV2 flag alone (drop the native-realtime coupling). New runs route to task_run_v2 whenever an org has the flag on and stay visible in realtime on the existing Electric backend. Single-run feeds route to one table by id format; only tag and batch feeds fan out to both shapes under one composite continuation.
…ed window Routes that walk the run hierarchy through a Prisma relation only see one physical table, so during a runTableV2 flag flip (a parent and child on opposite tables) they silently miss the cross-table run. This closes the reachable cases: - cancelRun resolves child runs across both tables, so cancelling a parent cascades to a child in the other table instead of leaving it executing and holding concurrency. - updateMetadata routes metadata.parent/root operations to the scalar parent/root id, so they reach a parent in the other table instead of falling back to the child run. - a one-time-use token with no idempotency key now takes a cross-table claim for v2 orgs, so two presentations straddling a flip cannot each mint a run in a different table. - the Electric shape merge reports up-to-date only when both tables are caught up, so a multi-chunk initial snapshot no longer drops the rows that arrive after the first chunk.
… mixed window A cuid parent (TaskRun) with a ksuid child (task_run_v2): cancelling the parent must cascade to the child in the other table. Fails against the old table-bound childRuns relation, passes with the cross-table findRuns lookup.
…tables An unordered take capped each run table independently and concatenated the two results, so a both-table read could silently drop one table rows once the other filled the cap. Reject it like the existing skip and cursor guards; callers that need a bounded cross-table read pass an orderBy for the keyset merge.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
New task runs can be routed to a parallel
task_run_v2Postgres table instead of the mainTaskRuntable, decided per-org by a feature flag and keyed purely by the run id's format. Existing runs stay inTaskRun, with no backfill. The flag ships off, so behavior is unchanged until an org is opted in.This builds on the RunStore adapter that already funnels all Postgres
TaskRunaccess through one place (writes in #3981, reads in #3990). RunStore now routes each run to its physical table by id format: a KSUID id meanstask_run_v2, anything else (including legacy cuids) meansTaskRun.Design
runTableV2flag on; everyone else keeps minting legacy ids. The flag is read in memory at the single mint site in the trigger path, so the hot path adds no query. RunStore never sees the flag: it routes purely byisKsuidId(id), and a malformed id falls back to legacy.findRunsdoes a bounded two-way merged keyset cursor (ordered reads standardize on a(createdAt, id)keyset, since cuid and KSUID do not share a sort range), and a non-idfindRun(idempotency-key dedup, or "are there any runs in this environment") queries both tables. Both apply identical scoping to each table, so a merge cannot leak a run across an auth boundary.TaskRun,task_run_v2, and the mollifier buffer, so a reused key is always found and never produces a duplicate run.task_run_v2from the start (empty until orgs cut over), streaming its WAL rows through the same transform into the same ClickHouse table.task_run_v2carries the same relations asTaskRun, and the incoming foreign keys pointing atTaskRunare dropped so the two tables are not coupled by constraints.Stacked on #3990 (its base), so this PR shows only the routing commits on top of the read adapter.
Before enabling the flag for any org,
task_run_v2needsREPLICA IDENTITY FULLapplied the same out-of-band way asTaskRun, so its update and delete events stream to ClickHouse with the old row.