commit f2e19f2e87d17eab7a454468fc84fcd7998ad6f7
parent 027d0bf362aafc668d1df68b1d13aa2b53b9087b
Author: I Mean I'm Just Saying <imeanimjustsaying@kiwifarms.st>
Date: Wed, 30 Sep 2026 00:42:16 -0400
common: the overdue refusal tells slow from stalled by the counters, as a timeout does
When every slot on a key is held by a call the watchdog gave up on, a new call
is still refused at once. It now marks the location stalled only when the
disk has completed nothing since the oldest of those calls began: each
overdue call keeps the counters reading taken when it began, and the refusal
reads them again (from /sys, synchronously, never the drive). Four slow units
on a disk that is still completing requests are "drive slow" — refused,
nothing marked — the same test a single timeout already applies (L6). With
the counters unchanged, or no device named, it marks as before (M1).
Tests: four units slower than the budget on a device whose completions move —
the fifth call is refused at once and nothing is marked; with completions
unchanged (after the pass has cleared the location) — refused and marked
again. The late-return test waits for its last calls to return before it
ends, so their releases do not reach the next test's reset state.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Diffstat:
2 files changed, 86 insertions(+), 8 deletions(-)
diff --git a/common/lib/storageHealth.test.ts b/common/lib/storageHealth.test.ts
@@ -488,4 +488,57 @@ test("M4: a call that returns late frees its slot for the calls waiting behind i
for (const o of await Promise.all(slow)) assert.match(o, /^drive slow/);
assert.deepEqual(await Promise.all(behind), Array(4).fill("ran"));
assert.equal(locationHealth("usb")?.state, "ok");
+ // The first return can feed all four waiters; let the other three slow
+ // calls return too, so their releases do not land in the next test's state.
+ await sleep(120);
+ assert.equal(driveCallsInFlight("usb"), 0);
+});
+
+// ── the overdue refusal: slow or stalled, by the counters ──────────────────
+
+test("every slot held by a slow unit on a disk still completing: the next call is refused, nothing marked", async () => {
+ registerLocationHealth([USB]);
+ noteLocationDetector("usb", "counters", "sdz1");
+ let completed = 0;
+ setCounterReader(() => ({ completed: (completed += 3), inFlight: 4 }));
+ setDriveCallBudget(60);
+ // Four units slower than the budget (they return long after): each is
+ // refused as slow at its timeout, and holds its slot, overdue.
+ const slow = Array.from({ length: 4 }, () =>
+ onDrive(USB, never).catch((err: Error) => err.message),
+ );
+ for (const o of await Promise.all(slow)) assert.match(o, /^drive slow/);
+ let made = false;
+ const started = Date.now();
+ await assert.rejects(
+ () => onDrive(USB, async () => {
+ made = true;
+ }),
+ (err: unknown) =>
+ err instanceof DriveNotAnsweringError &&
+ err.health === null &&
+ /^drive slow \(4 reads on it are past 0.06 s/.test(err.message),
+ );
+ assert.ok(Date.now() - started < 60, "refused at once");
+ assert.equal(made, false);
+ assert.equal(locationHealth("usb")?.state, "ok", "slow, not stalled");
+});
+
+test("every slot held by an overdue call on a disk completing nothing: the next call is refused and marks it", async () => {
+ registerLocationHealth([USB]);
+ noteLocationDetector("usb", "counters", "sdz1");
+ setCounterReader(() => ({ completed: 500, inFlight: 4 }));
+ setDriveCallBudget(60);
+ const hung = Array.from({ length: 4 }, () => onDrive(USB, never).catch(() => "gave up"));
+ await Promise.all(hung);
+ assert.equal(locationHealth("usb")?.state, "stalled");
+ // The pass clears it; the four have still not returned.
+ recordLocationHealth(USB, "ok");
+ recordLocationHealth(USB, "ok");
+ await assert.rejects(
+ () => onDrive(USB, async () => 1),
+ (err: unknown) => err instanceof DriveNotAnsweringError && err.health?.id === "usb",
+ );
+ assert.equal(locationHealth("usb")?.state, "stalled");
+ assert.match(String(locationHealth("usb")?.cause), /4 reads on it have not answered/);
});
diff --git a/common/lib/storageHealth.ts b/common/lib/storageHealth.ts
@@ -110,13 +110,18 @@ type Waiter = {
rearm: () => void;
};
+// A call the watchdog gave up on that has not returned yet: when it began,
+// and the device's counters then (null when the counters detector has named
+// no device for its location).
+type OverdueCall = { startedAt: number; before: BlockStatSample | null };
+
type HealthState = {
byId: Map<string, LocationHealth>;
// `onDrive`'s bookkeeping, by slot key (see `resolveWhere`): calls in
- // flight, how many of those the watchdog has already given up on, and the
- // calls waiting for a slot.
+ // flight, the ones among them the watchdog has already given up on, and
+ // the calls waiting for a slot.
inFlight?: Map<string, number>;
- overdue?: Map<string, number>;
+ overdue?: Map<string, OverdueCall[]>;
waiters?: Map<string, Waiter[]>;
// Test seam: the watchdog's budget.
budgetMs?: number;
@@ -387,8 +392,10 @@ export function notAnsweringText(h: Pick<LocationHealth, "since">, now?: number)
// restarts every waiter's deadline, so a deep queue on a drive that is busy
// but answering waits as long as it takes;
// - when every slot is held by a call the watchdog already gave up on, a new
-// call is refused at once and the location marked stalled again: none of
-// those calls has returned, whatever the last pass said.
+// call is refused at once, and the location marked stalled again unless
+// the disk has been completing requests since the oldest of those calls
+// began (slow, not stalled — the same test as a timeout's): none of those
+// calls has returned, whatever the last pass said.
// A slot is released when its call really returns, not when the watchdog gave
// up on it. So on one location at most four threads wait on its drive for the
// calls that come through here — every page and poll path, and the snapshot
@@ -498,7 +505,21 @@ async function acquireSlot(r: Resolved): Promise<void> {
s.inFlight.set(r.key, n + 1);
return;
}
- if ((s.overdue.get(r.key) ?? 0) >= DRIVE_CALLS_IN_FLIGHT) {
+ const overdue = s.overdue.get(r.key) ?? [];
+ if (overdue.length >= DRIVE_CALLS_IN_FLIGHT) {
+ // EVERY SLOT IS HELD BY A CALL THE WATCHDOG GAVE UP ON: refused at once.
+ // Marked stalled only when the disk has not been completing requests
+ // since the oldest of them began — the watchdog's own slow-or-stalled test
+ // (see `onDrive`): four slow reads on a busy disk are not a stall.
+ const oldest = overdue.reduce((a, b) => (b.startedAt < a.startedAt ? b : a));
+ const now = readDeviceCounters(r.loc);
+ if (oldest.before && now && now.completed > oldest.before.completed) {
+ throw new DriveNotAnsweringError(
+ null,
+ `drive slow (${DRIVE_CALLS_IN_FLIGHT} reads on it are past ` +
+ `${driveCallBudget() / 1000} s, while its disk is still completing others)`,
+ );
+ }
let health: LocationHealth | null = null;
if (r.loc) {
recordLocationHealth(r.loc, "stalled", {
@@ -627,6 +648,7 @@ export async function onDrive<T>(where: Where, call: () => Promise<T>): Promise<
released = true;
releaseSlot(r.key);
};
+ const startedAt = Date.now();
const before = readDeviceCounters(r.loc);
let pending: Promise<T>;
try {
@@ -657,9 +679,12 @@ export async function onDrive<T>(where: Where, call: () => Promise<T>): Promise<
}
// The call is still waiting on the drive. Its slot stays held, and counted
// overdue, until it returns; nothing awaits it.
- s.overdue.set(r.key, (s.overdue.get(r.key) ?? 0) + 1);
+ const entry: OverdueCall = { startedAt, before };
+ s.overdue.set(r.key, [...(s.overdue.get(r.key) ?? []), entry]);
const done = () => {
- s.overdue.set(r.key, Math.max(0, (s.overdue.get(r.key) ?? 1) - 1));
+ const left = (s.overdue.get(r.key) ?? []).filter((e) => e !== entry);
+ if (left.length > 0) s.overdue.set(r.key, left);
+ else s.overdue.delete(r.key);
release();
};
pending.then(done, done);
