Skip to content
Commit Detail

Commit 5ab21e0

Author
Milan Miladinovic <milan@cloudflare.com> 2025-11-25 18:52:51 -0500
Parents
49ec0b1
Tree
f6a9224
Don't schedule a move-later alarm if an interleaving commit setAlarm()

Prior to yielding to the event loop when we try to persist to SQLite, we
save the current alarm time in `alarmStateForCommit`. When we come back
after the operation completes, this value remains unchanged, but
`alarmScheduledNoLaterThan` may have changed while we were async.

Prior to this commit, that meant we might schedule a move-later alarm
synchronization with the alarm manager if `alarmScheduledNoLaterThan`
was now earlier than `alarmStateForCommit`, even if
`alarmStateForCommit` was set to the currently durable (and synced!)
alarm time.

Consider the following:
1. SQLite and the alarm manager have an alarm time of 2 years from now.
2. The application does a large SQL insert, starting commit 1. We yield
   to the event loop in `commitImpl` when we try to persist the write to
   SQLite, but save `alarmStateForCommit` as the current alarm time
   (2 years from now)
3. The application does a setAlarm() to run the alarm in 5 minutes. This
   is a "move-earlier" operation, so we sync to the alarm manager
   eagerly.
4. The alarm manager now has an alarm time 5 minutes in the future, and
   we start trying to persist the setAlarm in 5 minutes to SQLite.
5. The first commit from (2) continues after persisting its large SQL
   insert. It sees `alarmStateForCommit` is 2 years from now, and
   `alarmScheduledNoLaterThan` is 5 minutes from now. We decide to do a
   background "move-later" sync with the alarm manager, then finish the
   commit.
6. The second commit from (3) finishes persisting the 5 minute alarm to
   SQLite and finishes its commit.
7. The background "move-later" alarm from (5) modifies the alarm
   manager's state to have an alarm time of 2 years in the future.

The end state is we essentially miss the "move-earlier" update in the
alarm manager, breaking our model of "move-early eagerly and
move-later lazily".

**The fix**

We track `alarmVersion`, which is incremented every time the application
calls setAlarm(). We capture this value before starting to persist to
SQLite and compare it against the current `alarmVersion` after continuing
commitImpl. If our captured `alarmVersion` differs from the current
version, then the application called setAlarm() while our SQLite commit
was in-flight, and so we can safely skip attempting a move-later sync
with the alarm manager, since a subsequent commit will modify the alarm
anyways.

Files changed

3 files changed~3 modified