Skip to content

perf(webapp,run-engine,database): resolve the newest worker and deployment by createdAt - #4452

Merged
ericallam merged 2 commits into
mainfrom
feature/tri-12808-order-backgroundworker-most-recent-worker-lookup-by
Aug 1, 2026
Merged

perf(webapp,run-engine,database): resolve the newest worker and deployment by createdAt#4452
ericallam merged 2 commits into
mainfrom
feature/tri-12808-order-backgroundworker-most-recent-worker-lookup-by

Conversation

@ericallam

@ericallam ericallam commented Aug 1, 2026

Copy link
Copy Markdown
Member

Summary

Resolving "the most recent background worker for this environment" was ordered by id, which Postgres serves by scanning the primary key backwards and betting it finds a row for the target environment early. For an environment with many worker versions whose rows are all old, that bet loses and the scan crosses a large part of the index, so a lookup that normally takes about a millisecond takes hundreds. The same shape appeared on the deployments table.

These lookups now order by createdAt then id. On background workers that lands on the existing (runtimeEnvironmentId, createdAt) index with no schema change. Deployments had no matching index, so this adds one:

CREATE INDEX CONCURRENTLY IF NOT EXISTS "WorkerDeployment_environmentId_createdAt_idx"
ON "WorkerDeployment" ("environmentId", "createdAt");

The id tiebreak costs an incremental sort over a single createdAt group rather than a sort of the whole match set, so the ordering stays index-served.

Also drops three orderBy clauses that were dead: two findFirst calls keyed by primary key, and one hydration read whose caller re-sorts the rows itself.

Behaviour change

This changes which row counts as newest wherever id order and createdAt order disagree: those lookups now answer with the genuinely newest row. Verified end to end against a live environment by forcing that disagreement and confirming a real run resolves to, and successfully executes, the createdAt-newest worker. Tests covering both resolvers were red before the change and green after.

Rollout

The index build is the long pole and wants to go in on its own, so the migration and the code deploy should not be assumed simultaneous. Rollback is a revert plus DROP INDEX CONCURRENTLY "WorkerDeployment_environmentId_createdAt_idx". No data migration and no coexistence concern, since nothing persists the ordering and old and new code can run side by side.

Known limitation

The new index is keyed (environmentId, createdAt), so the deployment fallback's type = 'MANAGED' predicate is applied as a post-index filter rather than an index condition. For an environment whose newest MANAGED deployment sits behind many non-MANAGED ones, that scan still walks the intervening index entries. Left as-is on purpose: the high-volume query this index targets has no type predicate, and the fallback only runs when the promoted deployment is not MANAGED. If that fallback ever becomes hot, a partial index on MANAGED is the better shape than widening the composite.

Second change in here

findCurrentWorkerDeployment resolved the caller-supplied prisma client and used it for the promotion read, but issued the latest-of-type fallback read on the module-level client. Callers passing a replica were silently reading the primary for that one query. The fallback now uses the caller's client too.

This is a behaviour change, not just a tidy-up: the fallback read can now be replica-lagged for callers that pass a replica. That is the intended trade, because the previous mix could return a promotion read from a replica alongside a fallback deployment from the primary that did not correspond to it, and the promotion read (which serves the normal case) was already on the caller's client.

…yment by createdAt

Ordering these lookups by id makes Postgres scan the primary key backwards
betting on an early match for the target environment. When that bet loses the
scan crosses a large part of the index. Ordering by createdAt then id keeps the
lookup on a composite index instead.

Adds (environmentId, createdAt) on WorkerDeployment, which had no matching
index. BackgroundWorker already had one.
@changeset-bot

changeset-bot Bot commented Aug 1, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: e93a16d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Worker and deployment resolution now orders records by descending createdAt, with descending id as a tie-breaker. The WorkerDeployment model and migration add an environmentId and createdAt index. Worker and deployment tests now cover cases where identifier order differs from creation-time order. Run hydration no longer applies a separate descending ID order. A changelog entry documents the lookup update.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly and concisely describes the main change: selecting the newest worker and deployment by createdAt.
Description check ✅ Passed The description clearly covers the change, testing, rollout, rollback, behavior impact, and known limitation, but omits the template checklist and screenshots.
✨ Finishing Touches 💡 1
🛠️ Fix failing CI checks 💡
  • Create stacked PR
  • Commit on current branch
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feature/tri-12808-order-backgroundworker-most-recent-worker-lookup-by

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@ericallam
ericallam marked this pull request as ready for review August 1, 2026 08:08
devin-ai-integration[bot]

This comment was marked as resolved.

…t fallback

findCurrentWorkerDeployment resolved the caller-supplied client for the
promotion read but used the module-level client for the fallback read, so
callers passing a replica silently read the primary for that one query.
@ericallam
ericallam marked this pull request as draft August 1, 2026 08:17

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Devin Review found 2 new potential issues.

Open in Devin Review

Comment thread apps/webapp/app/v3/models/workerDeployment.server.ts
Comment thread internal-packages/run-engine/src/engine/controlPlaneResolver.ts
@ericallam
ericallam marked this pull request as ready for review August 1, 2026 08:19
@ericallam
ericallam merged commit f9c8d51 into main Aug 1, 2026
57 of 62 checks passed
@ericallam
ericallam deleted the feature/tri-12808-order-backgroundworker-most-recent-worker-lookup-by branch August 1, 2026 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants