Skip to content

perf(webapp): batch declarative schedule cleanup queries - #4522

Merged
ericallam merged 1 commit into
mainfrom
feature/tri-13016-syncdeclarativeschedules-issues-970-mostly-no-op-deletessec
Aug 7, 2026
Merged

perf(webapp): batch declarative schedule cleanup queries#4522
ericallam merged 1 commit into
mainfrom
feature/tri-13016-syncdeclarativeschedules-issues-970-mostly-no-op-deletessec

Conversation

@ericallam

@ericallam ericallam commented Aug 6, 2026

Copy link
Copy Markdown
Member

Summary

syncDeclarativeSchedules runs on every background-worker creation (every deploy, and every file save during trigger dev). It issued one instance-delete per declarative schedule the current worker no longer declares, in a loop, and the overwhelming majority of those deletes matched zero rows. This collapses the loop into at most two set-based statements and skips the instance delete entirely when the current environment owns no instance of the schedule.

Why so many, and mostly no-op

The loop runs once per entry in missingSchedules, which starts as every DECLARATIVE schedule for the whole project across all its environments (the query filters only by projectId). A schedule leaves that set only when a declared task matches it by taskIdentifier and the schedule already has an instance in the current environment.

That last clause is the amplifier. When a task's schedule has no instance in the current environment, the create branch inserts a brand-new TaskSchedule row with an instance for this environment rather than adding an instance to the existing row. So the same scheduled task, once it has run in dev and been deployed to prod, exists as two separate schedule rows: one carrying a dev instance, one carrying a prod instance.

On a dev worker sync of that project:

  • the dev-instance row matches the declared task and is removed from the set
  • the prod-instance row has the same taskIdentifier but no dev instance, so it stays in the set and gets deleteMany(taskScheduleId = prodRow, environmentId = dev), which matches zero rows

So every declarative task that has been synced in another environment contributes one guaranteed no-op delete per sync, and the count scales with (declarative tasks x environments), plus any leftover rows from renamed or removed tasks. A project does not need to have dropped a schedule to generate these; it just needs the same declarative tasks present in more than one environment, which is the normal develop-in-dev, deploy-to-prod case.

Fix

The candidate schedules are already loaded with their instances, so the branch is decided in memory:

  • schedules with no instances (or only current-environment instances) are removed in a single taskSchedule.deleteMany
  • schedules that still have another environment's instance have only the current environment's instance detached, in a single taskScheduleInstance.deleteMany, and only when such an instance actually exists

Behavior is unchanged (cascade delete still removes the instances of a deleted schedule); the difference is statement count. A zero-row delete writes no WAL and creates no dead tuples, so the removed work was pure query and commit overhead.

Verified with a testcontainer test (red before, green after) counting the emitted deletes across the no-op, batched-detach, and schedule-delete cases, and end to end through trigger dev: three declarative schedules created, surviving a re-sync, then two removed in a single batched delete with the third preserved.

@changeset-bot

changeset-bot Bot commented Aug 6, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 8030e5f

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 6, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

Walkthrough

Declarative schedule cleanup now classifies missing schedules before database operations. It batches current-environment instance detachment and deletion of schedules owned only by that environment. Integration tests cover unowned, shared, and exclusively owned schedules. A server change record documents the update.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Description check ⚠️ Warning The description explains the change and testing, but it omits the required issue reference, checklist, Changelog, and Screenshots sections. Add the required template sections, link the closing issue, complete the checklist, and document testing, changelog, and screenshots or mark them not applicable.
✅ Passed checks (4 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 identifies the performance change: batching declarative schedule cleanup queries.
✨ 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-13016-syncdeclarativeschedules-issues-970-mostly-no-op-deletessec

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.

coderabbitai[bot]

This comment was marked as resolved.

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

This comment was marked as resolved.

@ericallam
ericallam force-pushed the feature/tri-13016-syncdeclarativeschedules-issues-970-mostly-no-op-deletessec branch from b1640ff to 1952652 Compare August 6, 2026 15:02
@coderabbitai

coderabbitai Bot commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

syncDeclarativeSchedules issued one instance-delete per missing schedule on
every worker creation, and the overwhelming majority matched zero rows.
Collapse the loop into at most two set-based deletes and skip the instance
delete entirely when the environment owns no instance of the schedule.
@ericallam
ericallam force-pushed the feature/tri-13016-syncdeclarativeschedules-issues-970-mostly-no-op-deletessec branch from 1952652 to 8030e5f Compare August 7, 2026 08:46
@coderabbitai

coderabbitai Bot commented Aug 7, 2026

Copy link
Copy Markdown
Contributor

Note

GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer.

@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 1 new potential issue.

Open in Devin Review

Comment thread apps/webapp/app/v3/services/createBackgroundWorker.server.ts
@pkg-pr-new

pkg-pr-new Bot commented Aug 7, 2026

Copy link
Copy Markdown

Open in StackBlitz

@trigger.dev/build

npm i https://pkg.pr.new/@trigger.dev/build@8030e5f

trigger.dev

npm i https://pkg.pr.new/trigger.dev@8030e5f

@trigger.dev/core

npm i https://pkg.pr.new/@trigger.dev/core@8030e5f

@trigger.dev/python

npm i https://pkg.pr.new/@trigger.dev/python@8030e5f

@trigger.dev/react-hooks

npm i https://pkg.pr.new/@trigger.dev/react-hooks@8030e5f

@trigger.dev/redis-worker

npm i https://pkg.pr.new/@trigger.dev/redis-worker@8030e5f

@trigger.dev/rsc

npm i https://pkg.pr.new/@trigger.dev/rsc@8030e5f

@trigger.dev/schema-to-json

npm i https://pkg.pr.new/@trigger.dev/schema-to-json@8030e5f

@trigger.dev/sdk

npm i https://pkg.pr.new/@trigger.dev/sdk@8030e5f

commit: 8030e5f

@ericallam
ericallam merged commit 6c6e58e into main Aug 7, 2026
54 of 62 checks passed
@ericallam
ericallam deleted the feature/tri-13016-syncdeclarativeschedules-issues-970-mostly-no-op-deletessec branch August 7, 2026 09:27
@github-actions github-actions Bot mentioned this pull request Aug 7, 2026
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