perf(webapp): batch declarative schedule cleanup queries - #4522
Conversation
|
WalkthroughDeclarative schedule cleanup now classifies missing schedules before database operations. Schedules owned only by the current environment are deleted in bulk. Shared schedules retain their records while the current environment’s instances are detached in bulk. Integration tests cover unowned, shared, and exclusively owned schedules, including deletion counts and preservation of other environments. A server change record documents the batching update. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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.
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.
b1640ff to
1952652
Compare
|
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. |
2 participants
Summary
syncDeclarativeSchedulesruns on every background-worker creation (every deploy, and every file save duringtrigger 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 byprojectId). A schedule leaves that set only when a declared task matches it bytaskIdentifierand 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
TaskSchedulerow 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:
taskIdentifierbut no dev instance, so it stays in the set and getsdeleteMany(taskScheduleId = prodRow, environmentId = dev), which matches zero rowsSo 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:
taskSchedule.deleteManytaskScheduleInstance.deleteMany, and only when such an instance actually existsBehavior 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.