Skip to content

Revert "Delete all alarms on abortAllDurableObjects()"#6355

Merged
kentonv merged 1 commit intomainfrom
revert-1918-bcoll/abort-all-durable-object-alarms
Mar 18, 2026
Merged

Revert "Delete all alarms on abortAllDurableObjects()"#6355
kentonv merged 1 commit intomainfrom
revert-1918-bcoll/abort-all-durable-object-alarms

Conversation

@kentonv
Copy link
Member

@kentonv kentonv commented Mar 18, 2026

Reverts #1918

Ironically, a week after this PR was merged, vitest-pool-workers actually removed its usage of this API. After two years of waiting...

And after some discussion, we feel that it doesn't make sense for abortAllDurableObjects() to cancel alarms. Cancelling alarms is essentially deleting data from the DOs, but since it doesn't delete all data, it leaves them in an inconsistent state.

It could make sense to add a new deleteAllDurableObjects() API which aborts DOs, cancels alarms, and deletes underlying data, but that should be a separate API.

@kentonv kentonv requested review from a team as code owners March 18, 2026 22:02
@ask-bonk
Copy link
Contributor

ask-bonk bot commented Mar 18, 2026

The revert is a clean, exact inverse of PR #1918. Every addition in #1918 is removed in #6355, and every removal in #1918 is restored. The changes are:

  1. alarm-scheduler.h: Removes deleteAllAlarms() declaration and stmtDeleteAllAlarms prepared statement.
  2. alarm-scheduler.c++: Removes deleteAllAlarms() implementation.
  3. server.c++: Removes the alarmScheduler->deleteAllAlarms() call from abortAllActors().
  4. Test files: Removes the AlarmObject class, test_abort_all_durable_objects_alarms test, related DO namespace/binding config, and reverts storage back to inMemory + drops rpc compat flag (both only needed for the alarm test).

No residual references to the removed functionality, no behavioral changes beyond the revert, no backward compatibility concerns (the reverted feature was internal/unsafe-module only). The PR description's rationale is sound — cancelling alarms without deleting all data leaves DOs in an inconsistent state.

LGTM

github run

Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

I think there is still a need for deleting alarms in tests but something that can be worked on separately.

@kentonv kentonv enabled auto-merge March 18, 2026 22:13
@kentonv kentonv disabled auto-merge March 18, 2026 23:13
@kentonv kentonv merged commit 1ba05cb into main Mar 18, 2026
26 of 28 checks passed
@kentonv kentonv deleted the revert-1918-bcoll/abort-all-durable-object-alarms branch March 18, 2026 23:13
@kentonv
Copy link
Member Author

kentonv commented Mar 18, 2026

Internal build flaked twice on a test that literally does not compile in any of the code modified in this PR, so I bypassed.

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.

3 participants