Skip to content

Commit 350656a

Browse files
noreplyclaude
andcommitted
Auto-update: held work starts once, after the launcher commits
- Startup recovery only keeps work held for an update; startup starts it after launcher.prepareTrial succeeds, so a rolled-back trial starts nothing. - Held work is chosen by save order, not request time: everything unclaimed at the restart was saved after the hold ("send now", resume and retry keep their old request time). - The marker records the runs kept by the first startup and the runs started, so later startups (after a rollback, a crash or a part-way failure) start only what is left. A failure keeps the marker. - The updater waits for held work to settle before checking again, and keeps the hold past the 30-minute cap while the launcher's record is pending. - Tests: end-to-end rollback on a file database, startup order, each guard. Co-Authored-By: Claude Opus 5.5 <[email protected]>
1 parent 5164fea commit 350656a

10 files changed

Lines changed: 1080 additions & 237 deletions

‎apps/server/src/cloud/autoUpdate.test.ts‎

Lines changed: 44 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -326,8 +326,8 @@ const activeGate = Effect.fn("test.activeGate")(function* (
326326
) {
327327
const gate = yield* makeWorkAdmissionGate({ marker: marker ?? (yield* memoryUpdateHoldStore) });
328328
for (const consumer of consumers) yield* gate.register(consumer);
329-
// Startup recovery has run.
330-
yield* gate.heldForUpdateSince;
329+
// Startup has started whatever the last update held.
330+
yield* gate.finishHeldWork;
331331
return gate;
332332
});
333333

@@ -336,9 +336,7 @@ const makeWorld = Effect.fn("test.makeWorld")(function* (
336336
options: {
337337
readonly bootService?: boolean;
338338
readonly gate?: WorkAdmissionGate;
339-
readonly marker?: UpdateHoldStore & {
340-
readonly current: Effect.Effect<{ heldAt: string; targetVersion: string } | undefined>;
341-
};
339+
readonly marker?: Effect.Success<typeof memoryUpdateHoldStore>;
342340
/**
343341
* What the launcher does when asked: accept, refuse, or take the request without the answer
344342
* coming back (the 30 s reply timeout), with or without its record then showing it pending.
@@ -688,16 +686,39 @@ describe("auto-update loop", () => {
688686
}),
689687
);
690688

691-
it.effect("starts held work again after the maximum hold, whatever the launcher says", () =>
689+
it.effect("keeps holding past the maximum hold while the launcher has this update pending", () =>
692690
Effect.gen(function* () {
693691
const world = yield* makeWorld();
694692
yield* world.at(DUE);
695693
yield* world.at(DUE + 1);
696-
// A hung launcher: its record still says pending, or cannot be read at all.
694+
// A slow restart: the launcher's record still shows this update pending, so the restart is
695+
// still coming and work started now would be cut off by it.
696+
for (const extra of [0, 1, 10 * AUTO_UPDATE_QUIET_POLL_MS]) {
697+
yield* world.at(DUE + 1 + AUTO_UPDATE_MAX_HOLD_MS + extra);
698+
expect(yield* world.gate.isHeld).toBe(true);
699+
}
700+
expect((yield* world.state).phase).toBe("handed-off");
701+
expect(yield* world.resets).toBe(0);
702+
expect(yield* world.marker.current).toMatchObject({ targetVersion: TARGET });
703+
// Once the record says something else, the cap lets held work start.
704+
yield* Ref.set(world.record, { type: "update", update: outcome("rolled-back") });
705+
const due = DUE + 1 + AUTO_UPDATE_MAX_HOLD_MS + 11 * AUTO_UPDATE_QUIET_POLL_MS;
706+
yield* world.at(due);
707+
expect(yield* world.gate.isHeld).toBe(false);
708+
expect(yield* world.resets).toBe(1);
709+
}),
710+
);
711+
712+
it.effect("starts held work after the maximum hold when the launcher record is unreadable", () =>
713+
Effect.gen(function* () {
714+
const world = yield* makeWorld();
715+
yield* world.at(DUE);
716+
yield* world.at(DUE + 1);
717+
// A hung launcher whose record cannot be read: nothing shows a restart is still coming.
697718
const almost = DUE + 1 + AUTO_UPDATE_MAX_HOLD_MS - 1;
719+
yield* Ref.set(world.record, { type: "unreadable", reason: "bad file" });
698720
yield* world.at(almost);
699721
expect(yield* world.gate.isHeld).toBe(true);
700-
yield* Ref.set(world.record, { type: "unreadable", reason: "bad file" });
701722
const due = DUE + 1 + AUTO_UPDATE_MAX_HOLD_MS;
702723
yield* world.at(due);
703724
expect(yield* world.gate.isHeld).toBe(false);
@@ -763,24 +784,28 @@ describe("auto-update loop", () => {
763784
}),
764785
);
765786

766-
it.effect("clears a marker left by the last process once the launcher is settled", () =>
787+
it.effect("does not check for updates until work held by the last update has started", () =>
767788
Effect.gen(function* () {
768789
const marker = yield* memoryUpdateHoldStore;
769-
const left = { heldAt: DateTime.formatIso(DateTime.makeUnsafe(0)), targetVersion: CURRENT };
790+
const left = { heldAt: DateTime.formatIso(DateTime.makeUnsafe(DUE)), targetVersion: CURRENT };
770791
yield* marker.write(left);
771792
const gate = yield* makeWorkAdmissionGate({ marker });
772793
for (const consumer of WORK_ADMISSION_CONSUMERS) yield* gate.register(consumer);
773794
const world = yield* makeWorld({ gate, marker });
774-
// Startup recovery has not read it yet.
775-
yield* world.at(QUIET_TICK);
776-
expect(yield* marker.current).toEqual(left);
777-
yield* gate.heldForUpdateSince;
778-
// A trial is still pending: a rollback would need the marker again.
779-
yield* Ref.set(world.record, { type: "update", update: pending });
780-
yield* world.at(2 * QUIET_TICK);
795+
// Startup recovery kept the held work; the update is not committed yet, so none started.
796+
yield* TestClock.setTime(DUE);
797+
expect(yield* gate.heldForUpdate).toBeDefined();
798+
for (let minute = 0; minute < 5; minute++) {
799+
yield* world.at(DUE + minute * QUIET_TICK);
800+
}
801+
expect(yield* world.state).toEqual(initialAutoUpdateState(0));
802+
expect(yield* world.calls).toEqual([]);
803+
expect(yield* gate.isHeld).toBe(false);
781804
expect(yield* marker.current).toEqual(left);
782-
yield* Ref.set(world.record, { type: "update", update: outcome("committed") });
783-
yield* world.at(3 * QUIET_TICK);
805+
// Startup started it: updates go on as usual.
806+
yield* gate.finishHeldWork;
807+
yield* world.at(DUE + 5 * QUIET_TICK);
808+
expect((yield* world.state).phase).toBe("waiting");
784809
expect(yield* marker.current).toBeUndefined();
785810
}),
786811
);

‎apps/server/src/cloud/autoUpdate.ts‎

Lines changed: 41 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Automatic updates for boot-service installs, once threads are quiet and the optional hook agrees.
2-
// The handoff closes the WorkAdmission gate before the last read and writes a marker; work after
3-
// that stays unclaimed and startup recovery starts it after the restart. The gate stays closed
4-
// until exit, the launcher's record says no restart, or AUTO_UPDATE_MAX_HOLD_MS passes.
2+
// The handoff closes the WorkAdmission gate and writes a marker; held work starts once the
3+
// restarted server's update is committed. The gate stays closed until exit or the launcher's
4+
// record says no restart is coming (AUTO_UPDATE_MAX_HOLD_MS applies only while not pending).
55
import {
66
ServerSelfUpdateError,
77
type ServerSelfUpdateOutcome,
@@ -50,7 +50,8 @@ export const AUTO_UPDATE_QUIET_POLL_MS = MINUTE;
5050
export const AUTO_UPDATE_MAX_QUIET_WAIT_MS = 2 * 60 * MINUTE;
5151
/** After a failed attempt, so a release that cannot install is not fetched every hour. */
5252
export const AUTO_UPDATE_FAILURE_BACKOFF_MS = 6 * 60 * MINUTE;
53-
/** New work waits at most this long for the launcher to restart the server, then starts again. */
53+
/** New work waits at most this long for the launcher to restart the server, then starts again,
54+
* unless the launcher's record still shows this update pending. */
5455
export const AUTO_UPDATE_MAX_HOLD_MS = 30 * MINUTE;
5556
/** When the launcher's answer was lost (its 30 s reply timeout), how long its record may take to
5657
* show the update pending before the handoff counts as dropped. */
@@ -332,7 +333,8 @@ export const makeAutoUpdater = Effect.fn("cloud.auto_update.make")(function* (
332333
initialAutoUpdateState(yield* Clock.currentTimeMillis),
333334
);
334335
const loggedUnavailable = yield* Ref.make(false);
335-
const markerSettled = yield* Ref.make(false);
336+
const loggedUnsettled = yield* Ref.make(false);
337+
const loggedPastCap = yield* Ref.make(false);
336338
const lastUnreadable = yield* Ref.make<string | null>(null);
337339
const backedOffFailureId = yield* Ref.make<string | undefined>(undefined);
338340
const hookCommand = deps.preferences.pipe(
@@ -363,8 +365,9 @@ export const makeAutoUpdater = Effect.fn("cloud.auto_update.make")(function* (
363365
const command = preferences.autoUpdateQuietCommand;
364366
const hook = yield* deps.askHook(command, targetVersion);
365367
if (hook !== null) return hook;
366-
// Taken before the gate closes: anything requested from here on is either counted below or
367-
// held, and the marker lets startup recovery start the held part.
368+
// Anything saved after the gate closes is either counted below (and stops the update) or held,
369+
// and the marker lets startup recovery keep and later start the held part. Recovery decides by
370+
// that order, not by request times; `heldAt` only tells a fresh marker from a stale one.
368371
const heldAt = DateTime.formatIso(yield* DateTime.now);
369372
const starting = yield* deps.gate.hold;
370373
let reason =
@@ -542,36 +545,45 @@ export const makeAutoUpdater = Effect.fn("cloud.auto_update.make")(function* (
542545
});
543546

544547
/** The gate stays closed until exit. It opens again only when the launcher's record shows it
545-
* will not restart this server, or after AUTO_UPDATE_MAX_HOLD_MS; an unreadable record or a
546-
* lost answer is not a reason to let work start while a restart may still come. */
548+
* will not restart this server, or after AUTO_UPDATE_MAX_HOLD_MS unless the record shows this
549+
* update pending (the restart is still coming: work that started now would be cut off by it).
550+
* Before the cap an unreadable record or a lost answer keeps the hold too. */
547551
const watchHandoff = Effect.fn("cloud.auto_update.watchHandoff")(function* (
548552
current: Extract<AutoUpdateState, { phase: "handed-off" }>,
549553
nowMs: number,
550554
) {
551555
const heldMs = nowMs - current.since;
552-
if (heldMs >= AUTO_UPDATE_MAX_HOLD_MS) {
556+
const pastCap = heldMs >= AUTO_UPDATE_MAX_HOLD_MS;
557+
const record = yield* deps.launcherRecord;
558+
if (
559+
record.type === "update" &&
560+
record.update.status === "pending" &&
561+
record.update.targetVersion === current.version
562+
) {
563+
yield* Ref.set(lastUnreadable, null);
564+
if (!current.confirmed) yield* Ref.set(state, { ...current, confirmed: true });
565+
if (pastCap && !(yield* Ref.getAndSet(loggedPastCap, true))) {
566+
yield* Effect.logWarning(
567+
"Automatic server update: the restart is taking long; new work keeps waiting while the launcher has it pending.",
568+
{ targetVersion: current.version, heldMinutes: Math.round(heldMs / MINUTE) },
569+
);
570+
}
571+
return;
572+
}
573+
if (pastCap) {
553574
return yield* dropHandoff(
554575
current,
555576
nowMs,
556577
`no restart after ${Math.round(heldMs / MINUTE)} minutes`,
557578
);
558579
}
559-
const record = yield* deps.launcherRecord;
560580
if (record.type === "unreadable") {
561581
return yield* logUnreadable(
562582
record.reason,
563583
"Automatic server update cannot read the launcher state; new work keeps waiting.",
564584
);
565585
}
566586
yield* Ref.set(lastUnreadable, null);
567-
if (
568-
record.type === "update" &&
569-
record.update.status === "pending" &&
570-
record.update.targetVersion === current.version
571-
) {
572-
if (!current.confirmed) yield* Ref.set(state, { ...current, confirmed: true });
573-
return;
574-
}
575587
if (!current.confirmed && heldMs < AUTO_UPDATE_UNCONFIRMED_GRACE_MS) return;
576588
yield* dropHandoff(
577589
current,
@@ -582,21 +594,6 @@ export const makeAutoUpdater = Effect.fn("cloud.auto_update.make")(function* (
582594
);
583595
});
584596

585-
/** A marker left by the process before this one served startup recovery. Kept while the
586-
* launcher's trial is pending: a rollback restores the database as it was at the stop, and the
587-
* previous version's recovery needs the marker again. */
588-
const settleMarker = Effect.gen(function* () {
589-
if (yield* Ref.get(markerSettled)) return;
590-
// Startup recovery runs after this loop starts and must see the marker first.
591-
if (!(yield* deps.gate.updateMarkerRead)) return;
592-
if (yield* deps.gate.isHeld) return;
593-
const record = yield* deps.launcherRecord;
594-
if (record.type === "unreadable") return;
595-
if (record.type === "update" && record.update.status === "pending") return;
596-
yield* deps.gate.clearUpdateMarker;
597-
yield* Ref.set(markerSettled, true);
598-
});
599-
600597
const probe = Effect.fn("cloud.auto_update.probe")(function* (
601598
current: Extract<AutoUpdateState, { phase: "waiting" }>,
602599
nowMs: number,
@@ -669,7 +666,6 @@ export const makeAutoUpdater = Effect.fn("cloud.auto_update.make")(function* (
669666
const current = yield* Ref.get(state);
670667
// A handoff in progress is followed whatever the settings say now.
671668
if (current.phase === "handed-off") return yield* watchHandoff(current, nowMs);
672-
yield* settleMarker;
673669
const preferences = yield* deps.preferences;
674670
if (!preferences.autoUpdateServer) {
675671
if (current.phase === "waiting") {
@@ -700,6 +696,16 @@ export const makeAutoUpdater = Effect.fn("cloud.auto_update.make")(function* (
700696
}
701697
return;
702698
}
699+
// Work an earlier update held must start before this server holds work again; startup starts
700+
// it once its own update is committed.
701+
if (!(yield* deps.gate.heldWorkSettled)) {
702+
if (!(yield* Ref.getAndSet(loggedUnsettled, true))) {
703+
yield* Effect.logInfo(
704+
"Automatic server update waits until work held by the last update has started.",
705+
);
706+
}
707+
return;
708+
}
703709
const step = nextAutoUpdateStep(current, nowMs);
704710
if (step.type === "check") return yield* check(nowMs);
705711
if (step.type === "probe" && current.phase === "waiting") return yield* probe(current, nowMs);

0 commit comments

Comments
 (0)