Don't sync move-later alarm if alarm moved earlier when async

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 alarmScheduledNoLaterThan before starting to persist to SQLite
and compare it against alarmScheduledNoLaterThan after continuing
commitImpl. If it moved earlier while our write was in-flight, we don't
attempt a move-later alarm sync.

Author: MellowYarker

src/workerd/io/actor-sqlite.c++

@@ -433,6 +433,11 @@ kj::Promise<void> ActorSqlite::commitImpl(ActorSqlite::PrecommitAlarmState preco
   // pending commit so that the next commitImpl() invocation starts its own set of precommit alarm
   // updates and db commit.
   auto alarmStateForCommit = metadata.getAlarm();
+
+  // We need to know if a move-earlier setAlarm happens while our SQLite write is in-flight, if one
+  // occurs, we should not do a move-later when we continue execution, even if one was requested.
+  auto earliestBeforeAsync = alarmScheduledNoLaterThan;
+
   auto commitCallbackPromise = commitCallback();
   pendingCommit = kj::none;
 
@@ -449,27 +454,33 @@ kj::Promise<void> ActorSqlite::commitImpl(ActorSqlite::PrecommitAlarmState preco
   // Notify any merged commitImpl() requests that the db persistence completed.
   fulfiller->fulfill();
 
-  // If the db state is now later than the in-flight scheduled alarms, issue a request to update
-  // it to match the db state.
-  if (willFireEarlier(alarmScheduledNoLaterThan, alarmStateForCommit)) {
-    if (debugAlarmSync) {
-      KJ_LOG(WARNING, "NOSENTRY DEBUG_ALARM: Moving alarm later", "sqlite_has",
-          logDate(alarmStateForCommit), "alarmScheduledNoLaterThan",
-          logDate(alarmScheduledNoLaterThan));
+  // If another commit moved our alarmScheduledNoLaterThan earlier while we were in-flight, we must
+  // not attempt to sync the alarm manager with a move-later alarm. If however, there was no such
+  // move-earlier attempt (i.e. we had no other setAlarm, or another commit will attempt its own
+  // move-later), then we might want to try and move the alarm later.
+  if (!willFireEarlier(alarmScheduledNoLaterThan, earliestBeforeAsync)) {
+    // If the db state is now later than the in-flight scheduled alarms, issue a request to update
+    // it to match the db state.
+    if (willFireEarlier(alarmScheduledNoLaterThan, alarmStateForCommit)) {
+      if (debugAlarmSync) {
+        KJ_LOG(WARNING, "NOSENTRY DEBUG_ALARM: Moving alarm later", "sqlite_has",
+            logDate(alarmStateForCommit), "alarmScheduledNoLaterThan",
+            logDate(alarmScheduledNoLaterThan));
+      }
+      // We need to extend our alarmLaterChain now that we're adding a new "move-later" alarm task.
+      //
+      // Technically, we don't need serialize our "move-later" alarms since SQLite has the later
+      // time committed locally. We could just set the `alarmLaterChain` and pass a `kj::READY_NOW`
+      // to requestScheduledAlarm, and so if we have a partial failure we would just recover when
+      // the alarm runs early. That said, it doesn't hurt to serialize on the client-side.
+      alarmLaterChain = requestScheduledAlarm(alarmStateForCommit, alarmLaterChain.addBranch())
+                            .catch_([](kj::Exception&& e) {
+        // If an exception occurs when scheduling the alarm later, it's OK -- the alarm will
+        // eventually fire at the earlier time, and the rescheduling will be retried.
+        // We catch here to prevent the chain from breaking on errors.
+        LOG_WARNING_PERIODICALLY("NOSENTRY SQLite reschedule later alarm failed", e);
+      }).fork();
     }
-    // We need to extend our alarmLaterChain now that we're adding a new "move-later" alarm task.
-    //
-    // Technically, we don't need serialize our "move-later" alarms since SQLite has the later
-    // time committed locally. We could just set the `alarmLaterChain` and pass a `kj::READY_NOW`
-    // to requestScheduledAlarm, and so if we have a partial failure we would just recover when
-    // the alarm runs early. That said, it doesn't hurt to serialize on the client-side.
-    alarmLaterChain = requestScheduledAlarm(alarmStateForCommit, alarmLaterChain.addBranch())
-                          .catch_([](kj::Exception&& e) {
-      // If an exception occurs when scheduling the alarm later, it's OK -- the alarm will
-      // eventually fire at the earlier time, and the rescheduling will be retried.
-      // We catch here to prevent the chain from breaking on errors.
-      LOG_WARNING_PERIODICALLY("NOSENTRY SQLite reschedule later alarm failed", e);
-    }).fork();
   }
 }