From e34a233162a437288e0b767454cd392a7cfa2953 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Sat, 17 Oct 2026 13:24:34 +0200 Subject: [PATCH 1/3] test: no device work for a waiter the queue has rejected (#665) --- .../lease-acquisition-coordinator.test.ts | 136 ++++++++++++++++++ 1 file changed, 136 insertions(+) diff --git a/src/leasing/lease-acquisition-coordinator.test.ts b/src/leasing/lease-acquisition-coordinator.test.ts index 1954ac86a..ada27b5dc 100644 --- a/src/leasing/lease-acquisition-coordinator.test.ts +++ b/src/leasing/lease-acquisition-coordinator.test.ts @@ -1423,6 +1423,142 @@ describe("LeaseAcquisitionCoordinator", () => { }); }); + it("performs no provisioning for a waiter the queue times out during its eviction", async () => { + let deadlineDue = false; + const harness = await createHarness({ + lifecycle: (lifecycle) => ({ + bootForLease: (...args) => lifecycle.bootForLease(...args), + dispose: (...args) => { + // why: an eviction is the waiter's own device work, so this is where a real one spends + // the seconds its `timeoutMs` covers. The clock moves here to land the armed deadline + // inside the attempt, which the queue's mid-attempt rule leaves un-rejected; the + // decision that follows the eviction is what finds the waiter past that deadline. + if (!deadlineDue) { + deadlineDue = true; + harness.clock.advance(30); + } + return lifecycle.dispose(...args); + }, + shutdown: (...args) => lifecycle.shutdown(...args), + destroy: (...args) => lifecycle.destroy(...args), + }), + }); + // why: the one device slot holds a device of another model, so the request can only be served + // by evicting it. It is leased first, so the request is queued before it is ever planned: only + // a queued waiter has an armed deadline for the eviction to spend. + const oldDevice = await seedReady(harness, { ...request, model: "iPhone SE" }); + const held = await harness.coordinator.request( + { ...request, model: "iPhone SE" }, + { ownerId: "holder", requesterId: "holder" }, + ); + const acquisition = harness.coordinator.request(request, { + ownerId: "late", + requesterId: "late", + timeoutMs: 20, + }); + void acquisition.catch(() => undefined); + await settle(); + expect(harness.coordinator.queueDepth).toBe(1); + + // why: the lease ends without a reclaim, so the device stays `ready`, still of the old model: + // what the queued request evicts. The waiter's budget is 20 ms and the clock stands at 1_000, + // so the advance inside the eviction takes the deadline with it. + await harness.registry.beginRelease(held.lease.id); + await harness.registry.transitionDevice(oldDevice.id, "ready", { + event: "device.reclaimed", + payload: { deviceId: oldDevice.id, duration: 0, strategy: "wipe" }, + }); + harness.coordinator.kick(); + await settle(); + + await expect(acquisition).rejects.toMatchObject({ name: "QueueTimeoutError" }); + expect({ + createdModels: harness.driver.calls + .filter((call) => call.operation === "provision") + .map((call) => (call.arguments[0] as { readonly model?: string }).model), + deviceStates: harness.registry.snapshot.devices.map((candidate) => candidate.state), + reservedRunning: harness.capacity.runningCapacity([]).global.reserved, + }).toEqual({ + createdModels: ["iPhone SE"], + deviceStates: ["deleted"], + reservedRunning: 0, + }); + }); + + it.each([ + { + limits: { maxDevices: 1, maxRunning: 1 }, + seed: "shutdown" as const, + victim: request, + work: "boot a shut-down device", + }, + { + limits: { maxDevices: 1, maxRunning: 1 }, + seed: "ready" as const, + victim: { ...request, model: "iPhone SE" }, + work: "delete an idle device", + }, + { + limits: { maxDevices: 2, maxRunning: 1 }, + seed: "ready" as const, + victim: { ...request, model: "iPhone SE" }, + work: "shut down an idle running device", + }, + ])( + "does not go on to $work when the queue will not put the plan in flight", + async ({ limits, seed, victim }) => { + const harness = await createHarness(limits); + if (seed === "shutdown") { + await seedShutdown(harness, victim); + } else { + await seedReady(harness, victim); + } + const markProcessing = harness.queue.markProcessing.bind(harness.queue); + let markedProcessingCalls = 0; + let refuses = true; + // why: the queue refuses to put a waiter past its deadline in flight (its own rule, proved + // in wait-queue.test.ts), and the refusal can land on any plan the coordinator performs. + // The test above drives one plan kind through a real timeout; the stub stands at each of the + // others, where the state a real refusal needs is one the queue's own re-queue path creates. + harness.queue.markProcessing = (waiter) => { + markedProcessingCalls += 1; + return refuses ? false : markProcessing(waiter); + }; + + const acquisition = harness.coordinator.request(request, { + ownerId: "refused", + requesterId: "refused", + }); + const outcome = promiseState(acquisition); + await settle(); + + expect({ + deviceStates: harness.registry.snapshot.devices.map((candidate) => candidate.state), + leaseRequesters: harness.registry.snapshot.leases.map((lease) => lease.requesterId), + markedProcessingCalls, + outcome: outcome.state, + queueDepth: harness.coordinator.queueDepth, + }).toEqual({ + deviceStates: [seed === "shutdown" ? "shutdown" : "ready"], + leaseRequesters: [], + markedProcessingCalls: 1, + outcome: "pending", + queueDepth: 0, + }); + + // why: the refused plan gave back the capacity reservation and device claim it held, so the + // next request is served rather than told the worker is full. + refuses = false; + await expect( + harness.coordinator.request(request, { + noWait: true, + ownerId: "next", + requesterId: "next", + }), + ).resolves.toMatchObject({ device: { spec: { model: "iPhone 16" } } }); + }, + ); + it("keeps admission closed until concurrent maintenance callers have all finished", async () => { const harness = await createHarness(); await harness.coordinator.beginMaintenance(); From 858797a65609ea12c909aa7991ef61f8e9b3b6b5 Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Sat, 17 Oct 2026 13:30:13 +0200 Subject: [PATCH 2/3] fix(leasing): no device work for a waiter the queue has just rejected (#665) --- src/leasing/lease-acquisition-coordinator.ts | 21 +++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/src/leasing/lease-acquisition-coordinator.ts b/src/leasing/lease-acquisition-coordinator.ts index 5af648aff..a959a3c8b 100644 --- a/src/leasing/lease-acquisition-coordinator.ts +++ b/src/leasing/lease-acquisition-coordinator.ts @@ -897,7 +897,14 @@ export class LeaseAcquisitionCoordinator implements AcquisitionMaintenance { this.#defer(waiter); return undefined; } - this.options.queue.markProcessing(waiter); + // why: a refusal means the queue has just rejected this waiter at its deadline, so the device + // work this plan stands for would run for a requester who was already answered. The plan holds + // a capacity reservation or a device claim, so it gives both back here rather than at a + // performer that will never be reached. + if (!this.options.queue.markProcessing(waiter)) { + releasePlanResources(operation); + return undefined; + } return operation; } @@ -1286,6 +1293,18 @@ function releaseReclaimEstimateMs(driver: Driver, spec: DeviceSpec): number { return driver.estimate({ clean: "standard", operation: "reclaim" }, spec); } +/** + * why: a plan handed out the capacity or device claim its operation needs; dropping the plan has + * to give it back, or the resource stays held with nobody left to release it. + */ +function releasePlanResources(plan: OperationPlan): void { + if (plan.kind === "provision") plan.reservation.release(); + else if (plan.kind === "boot-shutdown") { + plan.capacityReservation.release(); + plan.claim.release(); + } else plan.claim.release(); +} + function operationPlan(plan: AcquisitionPlan): OperationPlan | undefined { return plan.kind === "grant-ready" || plan.kind === "grant-physical" || From a3d448ae3e49778052ec146fde25cd0c40261f1b Mon Sep 17 00:00:00 2001 From: Szymon Chmal Date: Sat, 17 Oct 2026 14:31:16 +0200 Subject: [PATCH 3/3] docs(leasing): name what a refused plan actually holds (#665) --- src/leasing/lease-acquisition-coordinator.test.ts | 4 ++-- src/leasing/lease-acquisition-coordinator.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/leasing/lease-acquisition-coordinator.test.ts b/src/leasing/lease-acquisition-coordinator.test.ts index ada27b5dc..faaea808e 100644 --- a/src/leasing/lease-acquisition-coordinator.test.ts +++ b/src/leasing/lease-acquisition-coordinator.test.ts @@ -1546,8 +1546,8 @@ describe("LeaseAcquisitionCoordinator", () => { queueDepth: 0, }); - // why: the refused plan gave back the capacity reservation and device claim it held, so the - // next request is served rather than told the worker is full. + // why: the refused plan gave back the capacity or device claim it held, so the next request + // is served rather than told the worker is full. refuses = false; await expect( harness.coordinator.request(request, { diff --git a/src/leasing/lease-acquisition-coordinator.ts b/src/leasing/lease-acquisition-coordinator.ts index a959a3c8b..09a454c84 100644 --- a/src/leasing/lease-acquisition-coordinator.ts +++ b/src/leasing/lease-acquisition-coordinator.ts @@ -899,7 +899,7 @@ export class LeaseAcquisitionCoordinator implements AcquisitionMaintenance { } // why: a refusal means the queue has just rejected this waiter at its deadline, so the device // work this plan stands for would run for a requester who was already answered. The plan holds - // a capacity reservation or a device claim, so it gives both back here rather than at a + // a capacity reservation or a device claim, so it gives those back here rather than at a // performer that will never be reached. if (!this.options.queue.markProcessing(waiter)) { releasePlanResources(operation);