From 709fbb359efbd5e2ece21b41458aef11c7a460de Mon Sep 17 00:00:00 2001 From: Marty Molt Date: Fri, 11 Sep 2026 21:27:08 -0500 Subject: [PATCH 01/14] provisioner: expose the shared slot pool's free count Add SlotReporter, an optional interface for provisioners whose Launch draws from a host-wide pool, and implement it on Firecracker from the ipam. The scheduler needs this to compute a tier's real capacity: the --max-runners budget is shared by every tier, so a tier's own max says nothing about whether the host can start another microVM right now. Refs #9 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/provisioner/firecracker.go | 4 ++++ internal/provisioner/network.go | 7 +++++++ internal/provisioner/network_test.go | 31 ++++++++++++++++++++++++++++ internal/provisioner/provisioner.go | 11 ++++++++++ 4 files changed, 53 insertions(+) diff --git a/internal/provisioner/firecracker.go b/internal/provisioner/firecracker.go index 1ced8b6..698ff02 100644 --- a/internal/provisioner/firecracker.go +++ b/internal/provisioner/firecracker.go @@ -209,6 +209,10 @@ func NewFirecracker(cfg FirecrackerConfig, log *slog.Logger) *Firecracker { // Name implements Provisioner. func (f *Firecracker) Name() string { return "firecracker" } +// FreeSlots implements SlotReporter with the network slot pool, which is the +// shared --max-runners budget every tier draws from. +func (f *Firecracker) FreeSlots() int { return f.ipam.available() } + // Launch implements Provisioner: reflink-clone the golden rootfs, allocate a // per-VM network slot, create a tap, boot the microVM with the JIT config // delivered via MMDS v2, then block until the guest self-destructs (reboot -f) diff --git a/internal/provisioner/network.go b/internal/provisioner/network.go index b13a037..68a5e8d 100644 --- a/internal/provisioner/network.go +++ b/internal/provisioner/network.go @@ -157,6 +157,13 @@ func (p *ipam) acquire() (int, bool) { return slot, true } +// available reports how many slots are currently free. +func (p *ipam) available() int { + p.mu.Lock() + defer p.mu.Unlock() + return len(p.free) +} + // release returns a slot to the pool. func (p *ipam) release(slot int) { p.mu.Lock() diff --git a/internal/provisioner/network_test.go b/internal/provisioner/network_test.go index e29eaff..91e0e12 100644 --- a/internal/provisioner/network_test.go +++ b/internal/provisioner/network_test.go @@ -82,6 +82,37 @@ func TestIPAMAcquireRelease(t *testing.T) { } } +func TestIPAMAvailableTracksPool(t *testing.T) { + p := newIPAM(3) + if got := p.available(); got != 3 { + t.Fatalf("available=%d want 3 for a fresh pool", got) + } + a, _ := p.acquire() + p.acquire() + if got := p.available(); got != 1 { + t.Fatalf("available=%d want 1 after two acquires", got) + } + p.acquire() + if got := p.available(); got != 0 { + t.Fatalf("available=%d want 0 when exhausted", got) + } + p.release(a) + if got := p.available(); got != 1 { + t.Fatalf("available=%d want 1 after release", got) + } +} + +func TestFreeSlotsReportsIPAM(t *testing.T) { + f := NewFirecracker(FirecrackerConfig{MaxVMs: 2}, testLogger()) + if got := f.FreeSlots(); got != 2 { + t.Fatalf("FreeSlots=%d want 2", got) + } + f.ipam.acquire() + if got := f.FreeSlots(); got != 1 { + t.Fatalf("FreeSlots=%d want 1 after one acquire", got) + } +} + func TestSetupNetworkRequiresExtIface(t *testing.T) { f := NewFirecracker(FirecrackerConfig{Egress: EgressConfig{Categories: []string{"open"}}}, testLogger()) f.run = func(context.Context, string, ...string) error { return nil } diff --git a/internal/provisioner/provisioner.go b/internal/provisioner/provisioner.go index a1e192f..ad6b9e8 100644 --- a/internal/provisioner/provisioner.go +++ b/internal/provisioner/provisioner.go @@ -26,3 +26,14 @@ type Provisioner interface { // Name returns the provisioner implementation name (for logging). Name() string } + +// SlotReporter is implemented by provisioners whose Launch draws from a +// host-wide pool that every tier shares (the --max-runners slot budget). The +// scheduler uses it to tell GitHub how many jobs a tier can really hold right +// now, rather than the tier's static max, so a host at the pool cap stops being +// assigned work that a sibling host could run. +type SlotReporter interface { + // FreeSlots returns the number of microVMs that could be launched right + // now before the shared pool is exhausted. + FreeSlots() int +} From c8c3990aef9cbbd7c742fa13dc3d885135fa1703 Mon Sep 17 00:00:00 2001 From: Marty Molt Date: Fri, 11 Sep 2026 21:27:08 -0500 Subject: [PATCH 02/14] scheduler: report live tier capacity as running plus free slots Capacity() is min(Max, running + FreeSlots()): the microVMs already in flight each hold one job, the shared pool bounds how many more could be launched, and the tier's Max caps the sum. Provisioners without a shared pool keep reporting Max. Refs #9 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/scheduler/scheduler.go | 18 +++++++++ internal/scheduler/scheduler_test.go | 60 ++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index 7774c30..fc7b4de 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -338,5 +338,23 @@ func (s *Scheduler) Running() int { return s.running } +// Capacity returns how many jobs this tier can hold concurrently right now: +// the microVMs already in flight (each takes one job) plus whatever the shared +// slot pool could still launch, capped at the tier's Max. It is what the +// listener advertises to GitHub as the scale set's capacity, so the number must +// not exceed what the host can actually serve — otherwise GitHub assigns the +// job here and it queues behind the pool while a sibling host sits idle. A +// provisioner without a shared pool has no such constraint, so Max stands. +func (s *Scheduler) Capacity() int { + s.mu.Lock() + running := s.running + s.mu.Unlock() + sr, ok := s.opts.Provisioner.(provisioner.SlotReporter) + if !ok { + return s.opts.Max + } + return min(s.opts.Max, running+sr.FreeSlots()) +} + // Drain blocks until all in-flight microVMs have exited. func (s *Scheduler) Drain() { s.wg.Wait() } diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index 339e4cc..ccb5086 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -24,6 +24,15 @@ func (p provFunc) Launch(ctx context.Context, name, jit string, spec core.Runner } func (provFunc) Name() string { return "fake" } +// slotProv is a provFunc that also reports a shared-pool free count, like the +// Firecracker provisioner does. +type slotProv struct { + provFunc + free int +} + +func (p slotProv) FreeSlots() int { return p.free } + type jitStub struct{} func (jitStub) Generate(context.Context, core.RunnerSpec) (string, string, error) { @@ -87,6 +96,57 @@ func TestReconcileRespectsMaxAndDrains(t *testing.T) { } } +func TestCapacityIsRunningPlusFreeSlotsCappedAtMax(t *testing.T) { + release := make(chan struct{}) + entered := make(chan struct{}, 16) + launch := provFunc(func(ctx context.Context, name, jit string, spec core.RunnerSpec, onBusy func()) error { + entered <- struct{}{} + <-release + return nil + }) + prov := &slotProv{provFunc: launch, free: 1} + s := New(Options{Max: 4, Provisioner: prov, JIT: jitStub{}, Logger: testLogger()}) + + // Nothing running, one slot free on the host: only one job can be held. + if got := s.Capacity(); got != 1 { + t.Fatalf("capacity=%d want 1 (0 running + 1 free)", got) + } + + s.Reconcile(context.Background(), 2) + for i := 0; i < 2; i++ { + select { + case <-entered: + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for launch to enter") + } + } + // Two in flight (each holds a job) and the pool is now dry: capacity is + // exactly what is already assigned, so GitHub should send nothing more. + prov.free = 0 + if got := s.Capacity(); got != 2 { + t.Fatalf("capacity=%d want 2 (2 running + 0 free)", got) + } + // The pool recovering never lifts capacity above the tier's own max. + prov.free = 10 + if got := s.Capacity(); got != 4 { + t.Fatalf("capacity=%d want 4 (min(max=4, 2+10))", got) + } + + close(release) + s.Drain() + if got := s.Capacity(); got != 4 { + t.Fatalf("capacity=%d want 4 after drain with a full pool", got) + } +} + +func TestCapacityWithoutSlotReporterIsMax(t *testing.T) { + prov := provFunc(func(context.Context, string, string, core.RunnerSpec, func()) error { return nil }) + s := New(Options{Max: 3, Provisioner: prov, JIT: jitStub{}, Logger: testLogger()}) + if got := s.Capacity(); got != 3 { + t.Fatalf("capacity=%d want Max(3) when the provisioner has no shared pool", got) + } +} + func TestReconcileZeroDoesNothing(t *testing.T) { prov := provFunc(func(context.Context, string, string, core.RunnerSpec, func()) error { t.Fatal("Launch should not be called for desired=0") From 250d7ba68318d0bc0fe91fe313dd07ecc00a2c3e Mon Sep 17 00:00:00 2001 From: Marty Molt Date: Fri, 11 Sep 2026 21:27:08 -0500 Subject: [PATCH 03/14] listener: advertise live capacity to GitHub instead of the static tier max The actions/scaleset listener sends X-ScaleSetMaxCapacity on every GetMessage, and GitHub assigns no more jobs to the scale set than that. Until now the value was the tier's static max, which is only honest when the tier has the host to itself. With two hosts serving the same scale-set name in different runner groups, a host whose --max-runners pool was exhausted kept being assigned jobs (six Rails CI jobs queued 5 min on one host while the other host's identical tier polled idle, 2026-09-11T22:33Z). Run now takes a CapacityFunc; the scaler re-reads it after every desired- count callback and pushes it through Listener.SetMaxRunners, clamped to [1, tier max]. The floor avoids sending 0, whose meaning to the back-end is undocumented. Fixes #9 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cmd/firerunner/main.go | 3 +- internal/listener/listener.go | 12 +++- internal/listener/listener_test.go | 105 +++++++++++++++++++++++++++++ internal/listener/scaleset.go | 58 ++++++++++++++-- 4 files changed, 169 insertions(+), 9 deletions(-) diff --git a/cmd/firerunner/main.go b/cmd/firerunner/main.go index 629f1e5..47e1947 100644 --- a/cmd/firerunner/main.go +++ b/cmd/firerunner/main.go @@ -347,10 +347,11 @@ func run(ctx context.Context, cfg *config.Config, log *slog.Logger) error { return sched.Running() } onBusy := func(name string) { sched.MarkBusy(name) } + capacity := func() int { return sched.Capacity() } wg.Add(1) go func() { defer wg.Done() - if err := lis.Run(runCtx, onDesired, onBusy); err != nil && runCtx.Err() == nil { + if err := lis.Run(runCtx, onDesired, onBusy, capacity); err != nil && runCtx.Err() == nil { errCh <- err cancel() } diff --git a/internal/listener/listener.go b/internal/listener/listener.go index b85bb14..90ca013 100644 --- a/internal/listener/listener.go +++ b/internal/listener/listener.go @@ -17,10 +17,18 @@ type DesiredFunc func(ctx context.Context, desired int) (running int) // guest console marker; either source flipping a VM to busy is enough. type BusyFunc func(runnerName string) +// CapacityFunc returns how many jobs the tier can hold concurrently right now. +// It is advertised to GitHub on every poll so that jobs beyond it stay queued +// at GitHub — where a sibling scale set on another host can take them — rather +// than being assigned to a host that cannot start them. +type CapacityFunc func() int + // Listener drives the scheduler from GitHub's assigned-job signal. type Listener interface { // Run blocks until ctx is cancelled, calling onDesired with the latest // desired runner count each time GitHub reports one and onBusy with a - // runner's name each time GitHub reports it started a job. onBusy may be nil. - Run(ctx context.Context, onDesired DesiredFunc, onBusy BusyFunc) error + // runner's name each time GitHub reports it started a job. capacity, when + // non-nil, is re-read after every onDesired and reported to GitHub as the + // scale set's current capacity. onBusy and capacity may be nil. + Run(ctx context.Context, onDesired DesiredFunc, onBusy BusyFunc, capacity CapacityFunc) error } diff --git a/internal/listener/listener_test.go b/internal/listener/listener_test.go index 0d86189..abb0895 100644 --- a/internal/listener/listener_test.go +++ b/internal/listener/listener_test.go @@ -36,6 +36,111 @@ func TestScalerHandleDesiredRunnerCount(t *testing.T) { } } +func TestScalerAdvertisesCapacityAfterEveryDesiredCount(t *testing.T) { + capacity := 5 + var advertised []int + a := &scaler{ + maxRunners: 8, + log: testLogger(), + onDesired: func(context.Context, int) int { return 0 }, + capacity: func() int { return capacity }, + setCapacity: func(c int) { advertised = append(advertised, c) }, + } + for _, c := range []int{5, 2, 2, 7} { + capacity = c + if _, err := a.HandleDesiredRunnerCount(context.Background(), 1); err != nil { + t.Fatal(err) + } + } + // Every callback re-reads capacity and pushes it, so the library's next + // GetMessage always carries the current value, even when unchanged. + want := []int{5, 2, 2, 7} + if len(advertised) != len(want) { + t.Fatalf("advertised %v, want %v", advertised, want) + } + for i := range want { + if advertised[i] != want[i] { + t.Fatalf("advertised %v, want %v", advertised, want) + } + } +} + +func TestScalerClampsAdvertisedCapacity(t *testing.T) { + cases := []struct { + name string + capacity int + want int + }{ + {"above tier max is capped", 50, 8}, + {"at tier max passes", 8, 8}, + {"zero becomes one", 0, 1}, + {"negative becomes one", -3, 1}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + var got int + a := &scaler{ + maxRunners: 8, + log: testLogger(), + onDesired: func(context.Context, int) int { return 0 }, + capacity: func() int { return tc.capacity }, + setCapacity: func(c int) { got = c }, + } + if _, err := a.HandleDesiredRunnerCount(context.Background(), 0); err != nil { + t.Fatal(err) + } + if got != tc.want { + t.Fatalf("advertised %d, want %d", got, tc.want) + } + }) + } +} + +func TestScalerCapacityReadAfterDesired(t *testing.T) { + // onDesired reconciles (launching VMs), so the capacity GitHub sees must be + // sampled after it, not before — otherwise a burst would be advertised + // against a stale, pre-launch count. + var order []string + a := &scaler{ + maxRunners: 4, + log: testLogger(), + onDesired: func(context.Context, int) int { + order = append(order, "desired") + return 1 + }, + capacity: func() int { order = append(order, "capacity"); return 1 }, + setCapacity: func(int) { order = append(order, "set") }, + } + if _, err := a.HandleDesiredRunnerCount(context.Background(), 1); err != nil { + t.Fatal(err) + } + if len(order) != 3 || order[0] != "desired" || order[1] != "capacity" || order[2] != "set" { + t.Fatalf("call order = %v, want [desired capacity set]", order) + } +} + +func TestScalerWithoutCapacityFuncNeverAdvertises(t *testing.T) { + a := &scaler{ + maxRunners: 4, + log: testLogger(), + onDesired: func(context.Context, int) int { return 0 }, + setCapacity: func(int) { t.Fatal("setCapacity must not be called without a CapacityFunc") }, + } + if _, err := a.HandleDesiredRunnerCount(context.Background(), 1); err != nil { + t.Fatal(err) + } + // And a CapacityFunc with nowhere to send it is equally inert. + b := &scaler{ + maxRunners: 4, + log: testLogger(), + onDesired: func(context.Context, int) int { return 0 }, + capacity: func() int { t.Fatal("capacity must not be read without a sink"); return 0 }, + } + if _, err := b.HandleDesiredRunnerCount(context.Background(), 1); err != nil { + t.Fatal(err) + } +} + func TestScalerJobCallbacks(t *testing.T) { var busy []string a := &scaler{ diff --git a/internal/listener/scaleset.go b/internal/listener/scaleset.go index 74c9f78..b4ea49a 100644 --- a/internal/listener/scaleset.go +++ b/internal/listener/scaleset.go @@ -159,7 +159,7 @@ func (s *ScaleSet) JIT() *JITSource { } // Run implements Listener: long-poll GitHub and drive onDesired. -func (s *ScaleSet) Run(ctx context.Context, onDesired DesiredFunc, onBusy BusyFunc) error { +func (s *ScaleSet) Run(ctx context.Context, onDesired DesiredFunc, onBusy BusyFunc, capacity CapacityFunc) error { l, err := sslistener.New(s.session, sslistener.Config{ ScaleSetID: s.scaleSetID, MaxRunners: s.cfg.MaxRunners, @@ -168,7 +168,19 @@ func (s *ScaleSet) Run(ctx context.Context, onDesired DesiredFunc, onBusy BusyFu if err != nil { return fmt.Errorf("create scaleset listener: %w", err) } - return l.Run(ctx, &scaler{onDesired: onDesired, onBusy: onBusy, minRunners: s.cfg.MinRunners, log: s.log}) + a := &scaler{ + onDesired: onDesired, + onBusy: onBusy, + minRunners: s.cfg.MinRunners, + maxRunners: s.cfg.MaxRunners, + log: s.log, + } + if capacity != nil { + a.capacity = capacity + a.setCapacity = l.SetMaxRunners + a.advertiseCapacity() + } + return l.Run(ctx, a) } // Close deregisters the scale set and closes the message session. It deletes the @@ -196,11 +208,22 @@ func (s *ScaleSet) Close(ctx context.Context) error { // dequeues a job (the primary, earliest source), while the scale-set protocol's // JobStarted can arrive later, but OR-ing them is strictly safe — a busy flag // only ever makes the drain wait for a real job, never cancels one early. +// +// capacity/setCapacity close the loop in the other direction: the library sends +// GitHub an X-ScaleSetMaxCapacity header on every poll, and GitHub assigns no +// more jobs than that to the scale set. The header defaults to the tier's static +// max, which is only honest when the tier has the host to itself; advertising +// the live capacity instead lets a host at the shared slot cap decline work +// that a sibling scale set (same name, another runner group) could start now. type scaler struct { - onDesired DesiredFunc - onBusy BusyFunc - minRunners int - log *slog.Logger + onDesired DesiredFunc + onBusy BusyFunc + capacity CapacityFunc + setCapacity func(int) + minRunners int + maxRunners int + log *slog.Logger + lastAdvert int } var _ sslistener.Scaler = (*scaler)(nil) @@ -208,9 +231,32 @@ var _ sslistener.Scaler = (*scaler)(nil) func (a *scaler) HandleDesiredRunnerCount(ctx context.Context, count int) (int, error) { desired := a.minRunners + count running := a.onDesired(ctx, desired) + a.advertiseCapacity() return running, nil } +// advertiseCapacity pushes the current CapacityFunc value to the library for +// its next GetMessage. The value is clamped to [1, maxRunners]: the ceiling is +// the tier's own max, and the floor avoids sending 0, whose meaning to the +// Actions back-end is undocumented (ARC never sends it). +func (a *scaler) advertiseCapacity() { + if a.capacity == nil || a.setCapacity == nil { + return + } + c := a.capacity() + if c > a.maxRunners { + c = a.maxRunners + } + if c < 1 { + c = 1 + } + if c != a.lastAdvert { + a.log.Info("advertising capacity", "capacity", c, "max", a.maxRunners) + a.lastAdvert = c + } + a.setCapacity(c) +} + func (a *scaler) HandleJobStarted(_ context.Context, j *scaleset.JobStarted) error { a.log.Info("job started", "runner", j.RunnerName, "runnerRequestId", j.RunnerRequestID) if a.onBusy != nil && j.RunnerName != "" { From df02cfca2f8ef11085433a28075af2397145658b Mon Sep 17 00:00:00 2001 From: Marty Molt Date: Fri, 11 Sep 2026 21:27:08 -0500 Subject: [PATCH 04/14] docs: describe active-active hosts and live capacity advertising Refs #9 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/README.md b/README.md index 3282205..e83232a 100644 --- a/README.md +++ b/README.md @@ -265,6 +265,25 @@ build/test/lint/SAST jobs that never touch Docker; Docker jobs stay on real CodeQL run it matches the full image's speed at a fraction of the size (CodeQL bundle served from the drive, no per-run download). +#### Several hosts behind one `runs-on` (active-active) + +GitHub lets two scale sets share a name as long as they sit in **different +runner groups**; jobs for that name are then assigned to whichever one is free +and either host keeps serving if the other goes away. Run the same tier catalog +on each host with a different `--runner-group` / `FR_RUNNER_GROUP` and no +workflow has to change. + +What makes this work under load is that each tier advertises its **live** +capacity to GitHub on every poll — the microVMs it already has in flight plus +whatever the host's `--max-runners` pool could still launch, capped at the +tier's `max` — rather than the static `max` alone. A host whose pool is +exhausted therefore reports "no room", GitHub assigns nothing more to it, and +the sibling host picks the job up instead of it queuing behind a full pool. +`firerunner` logs `advertising capacity` whenever the number changes. The +advertised value never drops below 1: the back-end's reading of a zero capacity +is undocumented, so a tier with nothing running on a full host may still be +handed one job, which then waits for the next free slot. + ### Inspecting a deployment (`status`, `doctor`) Two read-only subcommands help operators inspect and diagnose a runner host. From a2906b89ffbc3e8a0fdab009db2540b231b92cfb Mon Sep 17 00:00:00 2001 From: Marty Molt Date: Tue, 15 Sep 2026 15:35:48 -0500 Subject: [PATCH 05/14] docs: state that tiers share, not split, the advertised free slots Every tier advertises running + the same host-wide free count, so a burst across two tiers inside one poll can still be over-assigned by up to that count. Splitting the pool instead would strand a job on a single host the moment the busy tier used up its share. Record the trade-off where the formula lives and in the README. Refs #9 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 6 +++++- internal/scheduler/scheduler.go | 9 +++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index e83232a..b0fa30c 100644 --- a/README.md +++ b/README.md @@ -282,7 +282,11 @@ the sibling host picks the job up instead of it queuing behind a full pool. `firerunner` logs `advertising capacity` whenever the number changes. The advertised value never drops below 1: the back-end's reading of a zero capacity is undocumented, so a tier with nothing running on a full host may still be -handed one job, which then waits for the next free slot. +handed one job, which then waits for the next free slot. And because every tier +on a host counts the same free slots, a burst that hits two tiers within one +poll can still be over-assigned by up to the free count until their next poll — +the pool is not split between tiers, since a split would strand jobs on a single +host whenever the busy tier had exhausted its share while other tiers sat idle. ### Inspecting a deployment (`status`, `doctor`) diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index fc7b4de..dbf6efd 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -345,6 +345,15 @@ func (s *Scheduler) Running() int { // not exceed what the host can actually serve — otherwise GitHub assigns the // job here and it queues behind the pool while a sibling host sits idle. A // provisioner without a shared pool has no such constraint, so Max stands. +// +// Every tier on the host sees the same free count, so the tiers' advertised +// values can sum to more than the pool while several are idle, and a burst +// that lands on two tiers within one poll can still be over-assigned by up to +// the free count. That is deliberate: dividing the free slots between tiers +// would leave a slot unadvertised whenever the tier that wants it has used up +// its share, stranding a job on a single host that has room for it. The +// overlap only lasts until the next poll, and the sum is never larger than the +// static maxima advertised before. func (s *Scheduler) Capacity() int { s.mu.Lock() running := s.running From 00339e41cd4eaa55c4bb82a46240b371490297f4 Mon Sep 17 00:00:00 2001 From: Marty Molt Date: Tue, 15 Sep 2026 15:44:24 -0500 Subject: [PATCH 06/14] scheduler: keep committed launches out of the free-slot count Reconcile adds a VM to running before its goroutine has generated a JIT config and entered the provisioner, which is where the slot is drawn. Capacity is sampled right after Reconcile, so every scale-up briefly counted the new VMs twice: once in running and once in the still-free slots. A tier with one free slot advertised 2 after launching its first VM. PendingLaunches counts committed-but-not-entered launches; every tier on the host shares one instance so a tier also sees the slots its siblings have spoken for. Capacity subtracts it from the free count. Refs #9 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- cmd/firerunner/main.go | 2 + internal/scheduler/scheduler.go | 44 ++++++++++++- internal/scheduler/scheduler_test.go | 96 ++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+), 1 deletion(-) diff --git a/cmd/firerunner/main.go b/cmd/firerunner/main.go index 47e1947..7b86f41 100644 --- a/cmd/firerunner/main.go +++ b/cmd/firerunner/main.go @@ -273,6 +273,7 @@ func run(ctx context.Context, cfg *config.Config, log *slog.Logger) error { var ( listeners []*listener.ScaleSet scheds []*scheduler.Scheduler + pending scheduler.PendingLaunches ) closeAll := func() { for _, l := range listeners { @@ -305,6 +306,7 @@ func run(ctx context.Context, cfg *config.Config, log *slog.Logger) error { Provisioner: prov, JIT: lis.JIT(), Logger: tlog, + Pending: &pending, })) } defer closeAll() diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index dbf6efd..50307db 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -6,6 +6,7 @@ import ( "context" "log/slog" "sync" + "sync/atomic" "time" "github.com/solcreek/firerunner/internal/core" @@ -26,8 +27,27 @@ type Options struct { Provisioner provisioner.Provisioner JIT JITSource Logger *slog.Logger + // Pending, when set, is shared by every Scheduler on the host so that each + // tier's Capacity accounts for launches the other tiers have committed to + // but the provisioner has not yet drawn a slot for. nil gives the + // Scheduler a private counter. + Pending *PendingLaunches } +// PendingLaunches counts microVMs a Scheduler has added to its running total +// whose Launch has not yet been entered, so they are in neither the +// provisioner's free-slot count nor its in-use count. Capacity subtracts them +// from the free count to keep the two numbers a consistent snapshot: between +// Reconcile and the provisioner acquiring a slot (a JIT registration round +// trip to GitHub) a launching VM would otherwise be counted twice. +type PendingLaunches struct{ n atomic.Int64 } + +func (p *PendingLaunches) add(n int) { p.n.Add(int64(n)) } +func (p *PendingLaunches) done() { p.n.Add(-1) } + +// Count returns the number of launches committed but not yet entered. +func (p *PendingLaunches) Count() int { return int(p.n.Load()) } + // Scheduler launches one ephemeral microVM per assigned job, up to Max // concurrently. type Scheduler struct { @@ -58,6 +78,9 @@ type vmHandle struct { // New returns a Scheduler. func New(o Options) *Scheduler { + if o.Pending == nil { + o.Pending = &PendingLaunches{} + } return &Scheduler{opts: o, active: make(map[string]*vmHandle)} } @@ -87,6 +110,7 @@ func (s *Scheduler) Reconcile(ctx context.Context, desired int) { s.mu.Lock() n := plan(desired, s.running, s.opts.Max) s.running += n + s.opts.Pending.add(n) running := s.running stopped := 0 if n == 0 { @@ -152,6 +176,17 @@ func (s *Scheduler) launchOne(ctx context.Context) { s.maintainMinimum(ctx) }() + // The launch stops being pending the moment the provisioner is entered (it + // draws its slot first thing) or when we bail before getting there. + entered := false + enter := func() { + if !entered { + entered = true + s.opts.Pending.done() + } + } + defer enter() + // Once shutdown has begun the listener has stopped accepting work and Drain // is waiting to reap; don't boot a brand-new microVM into a draining host. if ctx.Err() != nil { @@ -193,6 +228,7 @@ func (s *Scheduler) launchOne(ctx context.Context) { } }() + enter() launchErr := s.opts.Provisioner.Launch(vmCtx, name, jit, s.opts.Spec, func() { s.MarkBusy(name) }) switch { case vmCtx.Err() != nil: @@ -319,6 +355,7 @@ func (s *Scheduler) maintainMinimum(ctx context.Context) { s.mu.Lock() n := plan(s.opts.Min, s.running, s.opts.Max) s.running += n + s.opts.Pending.add(n) running := s.running s.mu.Unlock() if n == 0 { @@ -346,6 +383,10 @@ func (s *Scheduler) Running() int { // job here and it queues behind the pool while a sibling host sits idle. A // provisioner without a shared pool has no such constraint, so Max stands. // +// Launches any tier has committed to but not yet entered are still in the +// free count, so they are subtracted first; Capacity is sampled right after +// Reconcile, which is exactly when that window is open. +// // Every tier on the host sees the same free count, so the tiers' advertised // values can sum to more than the pool while several are idle, and a burst // that lands on two tiers within one poll can still be over-assigned by up to @@ -362,7 +403,8 @@ func (s *Scheduler) Capacity() int { if !ok { return s.opts.Max } - return min(s.opts.Max, running+sr.FreeSlots()) + free := max(0, sr.FreeSlots()-s.opts.Pending.Count()) + return min(s.opts.Max, running+free) } // Drain blocks until all in-flight microVMs have exited. diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index ccb5086..c08b505 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -139,6 +139,102 @@ func TestCapacityIsRunningPlusFreeSlotsCappedAtMax(t *testing.T) { } } +// gateJIT blocks Generate until released, holding a launch in the window +// between Reconcile committing to it and the provisioner being entered. +type gateJIT struct{ release chan struct{} } + +func (g gateJIT) Generate(context.Context, core.RunnerSpec) (string, string, error) { + <-g.release + return "runner", "jit", nil +} + +type failJIT struct{} + +func (failJIT) Generate(context.Context, core.RunnerSpec) (string, string, error) { + return "", "", errors.New("jit unavailable") +} + +func TestCapacityExcludesPendingLaunchesFromFreeSlots(t *testing.T) { + jitGate := make(chan struct{}) + release := make(chan struct{}) + entered := make(chan struct{}, 16) + prov := &slotProv{free: 2, provFunc: func(ctx context.Context, name, jit string, spec core.RunnerSpec, onBusy func()) error { + entered <- struct{}{} + <-release + return nil + }} + s := New(Options{Max: 6, Provisioner: prov, JIT: gateJIT{jitGate}, Logger: testLogger()}) + + s.Reconcile(context.Background(), 2) + // Both launches are committed (running=2) but stuck in JIT generation, so + // the provisioner still reports both slots free. Counting them in both + // places would advertise 4; the honest figure is 2. + if got := s.Capacity(); got != 2 { + t.Fatalf("capacity=%d want 2 while 2 launches are pending against 2 free slots", got) + } + + close(jitGate) + for i := 0; i < 2; i++ { + select { + case <-entered: + case <-time.After(2 * time.Second): + t.Fatal("timed out waiting for launch to enter") + } + } + // Entering the provisioner settles the pending count; the fake pool is + // then updated the way the real one would be by the acquire. + if got := s.opts.Pending.Count(); got != 0 { + t.Fatalf("pending=%d want 0 once both launches entered the provisioner", got) + } + prov.free = 0 + if got := s.Capacity(); got != 2 { + t.Fatalf("capacity=%d want 2 with 2 running and a dry pool", got) + } + close(release) + s.Drain() +} + +func TestCapacityPendingIsSharedAcrossTiers(t *testing.T) { + jitGate := make(chan struct{}) + release := make(chan struct{}) + prov := &slotProv{free: 3, provFunc: func(ctx context.Context, name, jit string, spec core.RunnerSpec, onBusy func()) error { + <-release + return nil + }} + var pending PendingLaunches + a := New(Options{Max: 4, Provisioner: prov, JIT: gateJIT{jitGate}, Logger: testLogger(), Pending: &pending}) + b := New(Options{Max: 4, Provisioner: prov, JIT: jitStub{}, Logger: testLogger(), Pending: &pending}) + + a.Reconcile(context.Background(), 2) + // Tier B has nothing running, but two of the three free slots are already + // spoken for by tier A's pending launches. + if got := b.Capacity(); got != 1 { + t.Fatalf("tier B capacity=%d want 1 (3 free - 2 pending elsewhere)", got) + } + if got := a.Capacity(); got != 3 { + t.Fatalf("tier A capacity=%d want 3 (2 running + (3 free - 2 pending))", got) + } + + close(jitGate) + close(release) + a.Drain() + b.Drain() +} + +func TestPendingSettlesWhenLaunchBailsBeforeProvisioner(t *testing.T) { + prov := provFunc(func(context.Context, string, string, core.RunnerSpec, func()) error { + t.Fatal("Launch must not be reached after a JIT failure") + return nil + }) + var pending PendingLaunches + s := New(Options{Max: 2, Provisioner: prov, JIT: failJIT{}, Logger: testLogger(), Pending: &pending}) + s.Reconcile(context.Background(), 1) + s.Drain() + if got := pending.Count(); got != 0 { + t.Fatalf("pending=%d want 0 after the launch bailed on JIT generation", got) + } +} + func TestCapacityWithoutSlotReporterIsMax(t *testing.T) { prov := provFunc(func(context.Context, string, string, core.RunnerSpec, func()) error { return nil }) s := New(Options{Max: 3, Provisioner: prov, JIT: jitStub{}, Logger: testLogger()}) From 18974642e54dedf342b3dcafd04f622c491053d3 Mon Sep 17 00:00:00 2001 From: Marty Molt Date: Tue, 15 Sep 2026 15:50:56 -0500 Subject: [PATCH 07/14] provisioner: draw the network slot as Launch's first act The scheduler clears a launch's pending reservation on entering Launch, so the slot must leave the free count at that same instant. SetupNetwork ran first; move the acquire ahead of it so the window between the two is only the call itself. Refs #9 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/provisioner/firecracker.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/internal/provisioner/firecracker.go b/internal/provisioner/firecracker.go index 698ff02..d41f6d4 100644 --- a/internal/provisioner/firecracker.go +++ b/internal/provisioner/firecracker.go @@ -218,15 +218,18 @@ func (f *Firecracker) FreeSlots() int { return f.ipam.available() } // delivered via MMDS v2, then block until the guest self-destructs (reboot -f) // and reap everything. func (f *Firecracker) Launch(ctx context.Context, name, jitConfig string, spec core.RunnerSpec, onBusy func()) error { - if err := f.SetupNetwork(ctx); err != nil { - return fmt.Errorf("setup network: %w", err) - } - + // Draw the slot before anything else: the scheduler stops counting this + // launch as pending the moment Launch is entered, so FreeSlots must reflect + // it from the first instruction on. slot, ok := f.ipam.acquire() if !ok { return fmt.Errorf("no free network slot (max %d microVMs)", f.cfg.MaxVMs) } defer f.ipam.release(slot) + + if err := f.SetupNetwork(ctx); err != nil { + return fmt.Errorf("setup network: %w", err) + } vnet := slotNet(slot, f.cfg.TapPrefix, f.cfg.NetBase) if err := f.setupNet(ctx, vnet); err != nil { From 86c19092309f3af4fb5e1bc261f33ca492732061 Mon Sep 17 00:00:00 2001 From: Marty Molt Date: Tue, 15 Sep 2026 15:50:56 -0500 Subject: [PATCH 08/14] scheduler: leave cancelled VMs out of advertised capacity A VM that scaleDown has cancelled stays in running until its goroutine exits, but it cannot take a job and its slot is not yet back in the pool. Counting it let a scale-down followed by a demand spike advertise capacity that nothing could serve. Subtract cancelled handles in Capacity and cover the scale-down/scale-up transition. Refs #9 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/scheduler/scheduler.go | 15 ++++++++-- internal/scheduler/scheduler_test.go | 45 ++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 3 deletions(-) diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index 50307db..0ec92b7 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -176,8 +176,9 @@ func (s *Scheduler) launchOne(ctx context.Context) { s.maintainMinimum(ctx) }() - // The launch stops being pending the moment the provisioner is entered (it - // draws its slot first thing) or when we bail before getting there. + // The launch stops being pending the moment the provisioner is entered + // (Firecracker.Launch draws its slot as its first act) or when we bail + // before getting there. entered := false enter := func() { if !entered { @@ -385,7 +386,10 @@ func (s *Scheduler) Running() int { // // Launches any tier has committed to but not yet entered are still in the // free count, so they are subtracted first; Capacity is sampled right after -// Reconcile, which is exactly when that window is open. +// Reconcile, which is exactly when that window is open. A VM that scaleDown +// has cancelled is the mirror case: still in running while it tears down, yet +// unable to take a job and its slot not yet back in the pool, so it is left +// out of both. // // Every tier on the host sees the same free count, so the tiers' advertised // values can sum to more than the pool while several are idle, and a burst @@ -398,6 +402,11 @@ func (s *Scheduler) Running() int { func (s *Scheduler) Capacity() int { s.mu.Lock() running := s.running + for _, h := range s.active { + if h.cancelled { + running-- + } + } s.mu.Unlock() sr, ok := s.opts.Provisioner.(provisioner.SlotReporter) if !ok { diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index c08b505..c38e96c 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -235,6 +235,51 @@ func TestPendingSettlesWhenLaunchBailsBeforeProvisioner(t *testing.T) { } } +func TestCapacityExcludesCancelledVMsWhileTheyTearDown(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + entered := make(chan struct{}, 8) + exit := make(chan struct{}) + prov := &slotProv{free: 1, provFunc: func(vmCtx context.Context, name, jit string, spec core.RunnerSpec, onBusy func()) error { + entered <- struct{}{} + <-vmCtx.Done() + <-exit // a cancelled VM lingers in teardown until the test lets it go + return nil + }} + s := New(Options{Max: 4, Provisioner: prov, JIT: &jitSeq{}, Logger: testLogger()}) + + s.Reconcile(ctx, 3) + for i := 0; i < 3; i++ { + select { + case <-entered: + case <-time.After(2 * time.Second): + t.Fatal("launch never started") + } + } + if got := s.Capacity(); got != 4 { + t.Fatalf("capacity=%d want 4 (3 running + 1 free)", got) + } + + s.Reconcile(ctx, 1) // two idle VMs are cancelled but have not exited yet + if got := s.Running(); got != 3 { + t.Fatalf("running=%d want 3 while cancelled VMs are still tearing down", got) + } + // Those two can neither take a job nor have they returned their slots. + if got := s.Capacity(); got != 2 { + t.Fatalf("capacity=%d want 2 (1 live + 1 free; 2 cancelled excluded)", got) + } + + close(exit) + waitRunning(t, s, 1) + prov.free = 3 + if got := s.Capacity(); got != 4 { + t.Fatalf("capacity=%d want 4 once the cancelled VMs have exited and freed their slots", got) + } + cancel() + s.Drain() +} + func TestCapacityWithoutSlotReporterIsMax(t *testing.T) { prov := provFunc(func(context.Context, string, string, core.RunnerSpec, func()) error { return nil }) s := New(Options{Max: 3, Provisioner: prov, JIT: jitStub{}, Logger: testLogger()}) From aa1baf4538a368face60bd19c3ad5e817f5a79be Mon Sep 17 00:00:00 2001 From: Marty Molt Date: Tue, 15 Sep 2026 15:58:41 -0500 Subject: [PATCH 09/14] scheduler: leave launches sleeping in backoff out of advertised capacity After a failed boot the launch goroutine stays in running while it backs off (up to 30 s) so replenishment stays paced, but there is no VM and no slot behind it. At the pool cap, where 'no free network slot' is the failure, that let a tier keep advertising capacity it could not serve and hold work away from a sibling host. Count goroutines in backoff and subtract them in Capacity. Refs #9 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/scheduler/scheduler.go | 15 ++++++++-- internal/scheduler/scheduler_test.go | 44 ++++++++++++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index 0ec92b7..f3e4a57 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -64,6 +64,10 @@ type Scheduler struct { // golden (which burns a JIT registration per attempt). Reset on any healthy // launch. Guarded by mu. failures int + // backingOff counts launch goroutines sleeping in backoff after a failed + // boot: still in running (so the pacing and replenish bookkeeping hold) + // but with no VM behind them. Capacity leaves them out. Guarded by mu. + backingOff int } // vmHandle is the shutdown-relevant state of one in-flight microVM. @@ -255,8 +259,14 @@ func (s *Scheduler) backoff(ctx context.Context) { } s.mu.Lock() s.failures++ + s.backingOff++ n := s.failures s.mu.Unlock() + defer func() { + s.mu.Lock() + s.backingOff-- + s.mu.Unlock() + }() d := backoffDelay(n) s.opts.Logger.Warn("microVM boot failed; backing off before replenishing", "consecutiveFailures", n, "delay", d) t := time.NewTimer(d) @@ -389,7 +399,8 @@ func (s *Scheduler) Running() int { // Reconcile, which is exactly when that window is open. A VM that scaleDown // has cancelled is the mirror case: still in running while it tears down, yet // unable to take a job and its slot not yet back in the pool, so it is left -// out of both. +// out of both. So is a launch sleeping in backoff after a failed boot: it +// holds a place in running for pacing, but there is no VM behind it. // // Every tier on the host sees the same free count, so the tiers' advertised // values can sum to more than the pool while several are idle, and a burst @@ -401,7 +412,7 @@ func (s *Scheduler) Running() int { // static maxima advertised before. func (s *Scheduler) Capacity() int { s.mu.Lock() - running := s.running + running := s.running - s.backingOff for _, h := range s.active { if h.cancelled { running-- diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index c38e96c..a7344d3 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -280,6 +280,50 @@ func TestCapacityExcludesCancelledVMsWhileTheyTearDown(t *testing.T) { s.Drain() } +func TestCapacityExcludesLaunchesInBackoff(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + failed := make(chan struct{}, 8) + prov := &slotProv{free: 0, provFunc: func(context.Context, string, string, core.RunnerSpec, func()) error { + failed <- struct{}{} + return errors.New("no free network slot (max 4 microVMs)") + }} + s := New(Options{Max: 4, Min: 0, Provisioner: prov, JIT: jitStub{}, Logger: testLogger()}) + + s.Reconcile(ctx, 2) + for i := 0; i < 2; i++ { + select { + case <-failed: + case <-time.After(2 * time.Second): + t.Fatal("launch never attempted") + } + } + // Both goroutines are now sleeping in backoff (first delay 500ms), still + // counted in running for pacing, with no VM and no slot behind them. + waitCapacity(t, s, 0) + if got := s.Running(); got != 2 { + t.Fatalf("running=%d want 2 while the failed launches back off", got) + } + cancel() + s.Drain() +} + +func waitCapacity(t *testing.T, s *Scheduler, want int) { + t.Helper() + deadline := time.After(2 * time.Second) + for { + if got := s.Capacity(); got == want { + return + } + select { + case <-deadline: + t.Fatalf("capacity=%d want %d", s.Capacity(), want) + case <-time.After(5 * time.Millisecond): + } + } +} + func TestCapacityWithoutSlotReporterIsMax(t *testing.T) { prov := provFunc(func(context.Context, string, string, core.RunnerSpec, func()) error { return nil }) s := New(Options{Max: 3, Provisioner: prov, JIT: jitStub{}, Logger: testLogger()}) From aee3621fa542c459e788ea55af5d88360a6ceab0 Mon Sep 17 00:00:00 2001 From: Marty Molt Date: Tue, 15 Sep 2026 16:06:46 -0500 Subject: [PATCH 10/14] scheduler: never discount a failed launch twice in Capacity Two paths made a launch sleeping in backoff count against capacity a second time. Its vmHandle stayed in active until launchOne returned, so a scale-down during the sleep could mark it cancelled as well; drop the handle as soon as Launch returns. And when JIT generation fails, the pending reservation was only settled by the defer after backoff, hiding a free slot from every tier for the whole sleep; settle it before backing off. Refs #9 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/scheduler/scheduler.go | 6 +++ internal/scheduler/scheduler_test.go | 69 ++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+) diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index f3e4a57..13d7047 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -203,6 +203,9 @@ func (s *Scheduler) launchOne(ctx context.Context) { s.opts.Logger.Error("generate JIT config", "err", err) // A failed JIT generation is a GitHub API error; without pacing, the // maintainMinimum defer relaunches at once and spins a tight retry loop. + // Settle the reservation first: backoff already keeps this launch out of + // Capacity, and a slot it will never draw must not stay spoken for. + enter() s.backoff(ctx) return } @@ -235,6 +238,9 @@ func (s *Scheduler) launchOne(ctx context.Context) { enter() launchErr := s.opts.Provisioner.Launch(vmCtx, name, jit, s.opts.Spec, func() { s.MarkBusy(name) }) + // The VM is gone either way; drop its handle now so a scale-down during the + // backoff below cannot mark it cancelled and have Capacity discount it twice. + s.untrack(name) switch { case vmCtx.Err() != nil: // We cancelled this VM ourselves (scale-down or shutdown); any error it diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index a7344d3..0b85265 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -309,6 +309,75 @@ func TestCapacityExcludesLaunchesInBackoff(t *testing.T) { s.Drain() } +func TestCapacityCountsFailedLaunchOnceUnderScaleDown(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + entered := make(chan struct{}, 8) + failed := make(chan struct{}, 8) + var attempts int32 + prov := &slotProv{free: 2, provFunc: func(vmCtx context.Context, name, jit string, spec core.RunnerSpec, onBusy func()) error { + if atomic.AddInt32(&attempts, 1) == 1 { + failed <- struct{}{} + return errors.New("boot failed") // the first launch dies and backs off + } + entered <- struct{}{} + <-vmCtx.Done() + return nil + }} + s := New(Options{Max: 4, Min: 0, Provisioner: prov, JIT: &jitSeq{}, Logger: testLogger()}) + + s.Reconcile(ctx, 2) + select { + case <-failed: + case <-time.After(2 * time.Second): + t.Fatal("failing launch never attempted") + } + select { + case <-entered: + case <-time.After(2 * time.Second): + t.Fatal("healthy launch never entered") + } + // running=2: one live VM, one goroutine in backoff. Demand drops to 0, so + // scaleDown cancels every idle handle it can find — the failed launch must + // not be among them, or it would be discounted for backoff and again for + // cancellation, dragging capacity below the free slots the host has. + s.Reconcile(ctx, 0) + waitRunning(t, s, 1) + waitCapacity(t, s, 2) // 0 live (the survivor was cancelled) + 2 free + + cancel() + s.Drain() +} + +func TestPendingSettlesBeforeJITFailureBackoff(t *testing.T) { + prov := &slotProv{free: 3, provFunc: func(context.Context, string, string, core.RunnerSpec, func()) error { + t.Fatal("Launch must not be reached after a JIT failure") + return nil + }} + var pending PendingLaunches + s := New(Options{Max: 4, Provisioner: prov, JIT: failJIT{}, Logger: testLogger(), Pending: &pending}) + s.Reconcile(context.Background(), 1) + // The goroutine sleeps in backoff for 500ms. Its reservation must be + // released as soon as JIT fails, not when the goroutine exits, otherwise + // the free slot it will never use is hidden from every tier meanwhile. + deadline := time.After(200 * time.Millisecond) + for pending.Count() != 0 { + select { + case <-deadline: + t.Fatalf("pending=%d want 0 while the JIT failure backs off", pending.Count()) + case <-time.After(5 * time.Millisecond): + } + } + if got := s.Running(); got != 1 { + t.Fatalf("running=%d want 1: the goroutine should still be in backoff", got) + } + if got := s.Capacity(); got != 3 { + t.Fatalf("capacity=%d want 3 (0 live + 3 free, none reserved)", got) + } + s.Drain() +} + func waitCapacity(t *testing.T, s *Scheduler, want int) { t.Helper() deadline := time.After(2 * time.Second) From 0bb92d19cf1bb551ded40878ad5e3e53e61d2a03 Mon Sep 17 00:00:00 2001 From: Marty Molt Date: Tue, 15 Sep 2026 16:16:30 -0500 Subject: [PATCH 11/14] scheduler: settle the reservation before tracking, and clamp the whole sum A handle became visible to scaleDown one step before its reservation was settled, so a scale-down landing in that gap would have Capacity discount the launch as both pending and cancelled. Settle first, then track. Capacity also clamped the free count before netting out pending launches, so a callback that committed more launches than the pool had slots kept advertising all of them; only the ones that get a slot can serve. Net pending out of the full sum and clamp the result. Refs #9 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/scheduler/scheduler.go | 14 ++++++++++---- internal/scheduler/scheduler_test.go | 28 ++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 4 deletions(-) diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index 13d7047..4315816 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -180,7 +180,7 @@ func (s *Scheduler) launchOne(ctx context.Context) { s.maintainMinimum(ctx) }() - // The launch stops being pending the moment the provisioner is entered + // The launch stops being pending once it is about to enter the provisioner // (Firecracker.Launch draws its slot as its first act) or when we bail // before getting there. entered := false @@ -225,6 +225,9 @@ func (s *Scheduler) launchOne(ctx context.Context) { // job that overruns the stop timeout. vmCtx, vmCancel := context.WithCancel(context.WithoutCancel(ctx)) defer vmCancel() + // Settle the reservation before the handle becomes visible to scaleDown: + // a handle that is both pending and cancelled would be discounted twice. + enter() s.track(name, vmCancel) defer s.untrack(name) @@ -236,7 +239,6 @@ func (s *Scheduler) launchOne(ctx context.Context) { } }() - enter() launchErr := s.opts.Provisioner.Launch(vmCtx, name, jit, s.opts.Spec, func() { s.MarkBusy(name) }) // The VM is gone either way; drop its handle now so a scale-down during the // backoff below cannot mark it cancelled and have Capacity discount it twice. @@ -429,8 +431,12 @@ func (s *Scheduler) Capacity() int { if !ok { return s.opts.Max } - free := max(0, sr.FreeSlots()-s.opts.Pending.Count()) - return min(s.opts.Max, running+free) + // Pending launches are in running but not yet out of the free count, so + // they net out of the sum. When more launches were committed than the pool + // has slots (they will fail), the sum falls below running, which is right: + // only the ones that get a slot can serve. + total := running + sr.FreeSlots() - s.opts.Pending.Count() + return max(0, min(s.opts.Max, total)) } // Drain blocks until all in-flight microVMs have exited. diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index 0b85265..f9d99d9 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -194,6 +194,34 @@ func TestCapacityExcludesPendingLaunchesFromFreeSlots(t *testing.T) { s.Drain() } +func TestCapacityWhenMoreIsPendingThanFree(t *testing.T) { + jitGate := make(chan struct{}) + release := make(chan struct{}) + prov := &slotProv{free: 1, provFunc: func(ctx context.Context, name, jit string, spec core.RunnerSpec, onBusy func()) error { + <-release + return nil + }} + var pending PendingLaunches + a := New(Options{Max: 4, Provisioner: prov, JIT: gateJIT{jitGate}, Logger: testLogger(), Pending: &pending}) + b := New(Options{Max: 4, Provisioner: prov, JIT: jitStub{}, Logger: testLogger(), Pending: &pending}) + + a.Reconcile(context.Background(), 3) + // Three launches committed against one free slot: only one can ever get + // a slot, so the tier can hold one job, not three. + if got := a.Capacity(); got != 1 { + t.Fatalf("tier A capacity=%d want 1 (3 running + 1 free - 3 pending)", got) + } + // And the sibling tier sees a pool that is already oversubscribed. + if got := b.Capacity(); got != 0 { + t.Fatalf("tier B capacity=%d want 0 (0 running + 1 free - 3 pending, floored)", got) + } + + close(jitGate) + close(release) + a.Drain() + b.Drain() +} + func TestCapacityPendingIsSharedAcrossTiers(t *testing.T) { jitGate := make(chan struct{}) release := make(chan struct{}) From 7d2895a0f5ed8881a45e72b33c078e0b4178d6a7 Mon Sep 17 00:00:00 2001 From: Marty Molt Date: Tue, 15 Sep 2026 16:27:53 -0500 Subject: [PATCH 12/14] scheduler: measure demand and capacity against the same live count Capacity had learned to leave launches in backoff and VMs tearing down out of the count, but Reconcile still planned against raw running, so a job GitHub assigned on that advertised capacity found plan wanting nothing and waited out the backoff or teardown. Give the scheduler one notion of live (running minus backoff and cancelled) and use it on both sides: - plan measures demand against live, and bounds the launch by Max - running so a failing golden holds at most Max attempts; - scaleDown measures excess against live, so a launch in backoff no longer gets a healthy idle VM cancelled in its place; - Capacity mirrors plan: committed VMs, plus pending launches the pool can seat, plus min(Max - running, free slots not yet spoken for); - maintainMinimum keeps measuring the warm-pool floor against running on purpose, so the backoff still paces replenishment. Settle the running count and drop the handle in one critical section as soon as Launch returns, so no sample sees a VM and its freed slot at once. The JIT-failure test now waits for backoff to register before asserting. Refs #9 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/scheduler/scheduler.go | 128 ++++++++++++++------- internal/scheduler/scheduler_test.go | 160 ++++++++++++++++++++++++--- 2 files changed, 229 insertions(+), 59 deletions(-) diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index 4315816..b72f7f3 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -66,8 +66,11 @@ type Scheduler struct { failures int // backingOff counts launch goroutines sleeping in backoff after a failed // boot: still in running (so the pacing and replenish bookkeeping hold) - // but with no VM behind them. Capacity leaves them out. Guarded by mu. + // but with no VM behind them. live leaves them out. Guarded by mu. backingOff int + // pending is this tier's share of opts.Pending: launches in running that + // have not yet drawn a slot. Guarded by mu. + pending int } // vmHandle is the shutdown-relevant state of one in-flight microVM. @@ -89,12 +92,17 @@ func New(o Options) *Scheduler { } // plan returns how many new runners to launch given the desired count reported -// by GitHub, the number already running, and the capacity limit. It returns only -// the scale-up amount (never negative) bounded by available capacity; scaling -// down when desired drops is handled separately by scaleDown, which cancels -// idle VMs rather than relying solely on them self-terminating. -func plan(desired, running, max int) int { - want := desired - running +// by GitHub, the number of microVMs able to take a job (live), the number of +// launch goroutines in flight (running, which also counts VMs tearing down and +// launches sleeping in backoff), and the capacity limit. Demand is measured +// against live, so a job is never left waiting on a VM that cannot serve it; +// the launch is bounded by max - running, so a failing golden can hold at most +// max attempts in backoff at once. It returns only the scale-up amount (never +// negative); scaling down when desired drops is handled separately by +// scaleDown, which cancels idle VMs rather than relying solely on them +// self-terminating. +func plan(desired, live, running, max int) int { + want := desired - live if want < 0 { want = 0 } @@ -107,13 +115,27 @@ func plan(desired, running, max int) int { return want } +// live returns how many in-flight microVMs can take a job: running minus the +// launches sleeping in backoff (no VM behind them) and the VMs scaleDown has +// cancelled (tearing down). Callers must hold s.mu. +func (s *Scheduler) live() int { + n := s.running - s.backingOff + for _, h := range s.active { + if h.cancelled { + n-- + } + } + return n +} + // Reconcile launches microVMs to meet the desired count, bounded by capacity, // and cancels idle VMs when desired drops below the number running. It is safe // for concurrent use; each launched runner handles exactly one job. func (s *Scheduler) Reconcile(ctx context.Context, desired int) { s.mu.Lock() - n := plan(desired, s.running, s.opts.Max) + n := plan(desired, s.live(), s.running, s.opts.Max) s.running += n + s.pending += n s.opts.Pending.add(n) running := s.running stopped := 0 @@ -148,7 +170,7 @@ func (s *Scheduler) scaleDown(desired int) int { if floor < s.opts.Min { floor = s.opts.Min } - excess := s.running - floor + excess := s.live() - floor if excess <= 0 { return 0 } @@ -173,9 +195,16 @@ func (s *Scheduler) launchOne(ctx context.Context) { // This runs before wg.Done above (defers are LIFO), so the WaitGroup counter // stays >=1 while maintainMinimum may Add a replacement — avoiding a // concurrent-Add-during-Drain race. + finished := false + finish := func() { + if !finished { + finished = true + s.running-- + } + } defer func() { s.mu.Lock() - s.running-- + finish() s.mu.Unlock() s.maintainMinimum(ctx) }() @@ -187,6 +216,9 @@ func (s *Scheduler) launchOne(ctx context.Context) { enter := func() { if !entered { entered = true + s.mu.Lock() + s.pending-- + s.mu.Unlock() s.opts.Pending.done() } } @@ -240,9 +272,18 @@ func (s *Scheduler) launchOne(ctx context.Context) { }() launchErr := s.opts.Provisioner.Launch(vmCtx, name, jit, s.opts.Spec, func() { s.MarkBusy(name) }) - // The VM is gone either way; drop its handle now so a scale-down during the - // backoff below cannot mark it cancelled and have Capacity discount it twice. - s.untrack(name) + // The VM is gone and its slot is back in the pool. Drop the handle in the + // same critical section that settles the count, so no sample sees the VM + // and its freed slot at once, and a scale-down during the backoff below + // cannot mark a dead launch cancelled. A failed launch keeps its place in + // running through backoff so plan bounds the retry attempts. + s.mu.Lock() + delete(s.active, name) + failed := launchErr != nil && vmCtx.Err() == nil + if !failed { + finish() + } + s.mu.Unlock() switch { case vmCtx.Err() != nil: // We cancelled this VM ourselves (scale-down or shutdown); any error it @@ -371,9 +412,15 @@ func (s *Scheduler) maintainMinimum(ctx context.Context) { if s.opts.Min <= 0 || ctx.Err() != nil { return } + // The floor is measured against running rather than live on purpose: a + // launch sleeping in backoff is the pool's pending replenishment, and + // counting it keeps a failing golden from being retried faster than backoff + // allows. Demand-driven launches (Reconcile) are the ones that must not + // wait on it. s.mu.Lock() - n := plan(s.opts.Min, s.running, s.opts.Max) + n := plan(s.opts.Min, s.running, s.running, s.opts.Max) s.running += n + s.pending += n s.opts.Pending.add(n) running := s.running s.mu.Unlock() @@ -394,21 +441,24 @@ func (s *Scheduler) Running() int { return s.running } -// Capacity returns how many jobs this tier can hold concurrently right now: -// the microVMs already in flight (each takes one job) plus whatever the shared -// slot pool could still launch, capped at the tier's Max. It is what the -// listener advertises to GitHub as the scale set's capacity, so the number must -// not exceed what the host can actually serve — otherwise GitHub assigns the -// job here and it queues behind the pool while a sibling host sits idle. A -// provisioner without a shared pool has no such constraint, so Max stands. +// Capacity returns how many jobs this tier can hold concurrently right now. +// It is what the listener advertises to GitHub as the scale set's capacity, so +// it must match what Reconcile can actually serve — otherwise GitHub assigns +// the job here and it queues while a sibling host sits idle. Three parts add +// up, mirroring plan: +// +// - VMs that hold a slot and can take a job (live minus this tier's pending +// launches); +// - pending launches that will find a slot (the shared pool has room for +// them; those it does not have room for will fail and are not counted); +// - launches Reconcile could still make: bounded both by Max - running, as +// plan is, and by the slots the pool has left once every tier's pending +// launches have drawn theirs. // -// Launches any tier has committed to but not yet entered are still in the -// free count, so they are subtracted first; Capacity is sampled right after -// Reconcile, which is exactly when that window is open. A VM that scaleDown -// has cancelled is the mirror case: still in running while it tears down, yet -// unable to take a job and its slot not yet back in the pool, so it is left -// out of both. So is a launch sleeping in backoff after a failed boot: it -// holds a place in running for pacing, but there is no VM behind it. +// Launches sleeping in backoff and VMs tearing down after a cancel are in +// running but not in live: they cannot take a job, and their slot is either +// absent or not yet back in the pool. A provisioner without a shared pool has +// no slot constraint, so only Max - running bounds the last term. // // Every tier on the host sees the same free count, so the tiers' advertised // values can sum to more than the pool while several are idle, and a burst @@ -420,23 +470,17 @@ func (s *Scheduler) Running() int { // static maxima advertised before. func (s *Scheduler) Capacity() int { s.mu.Lock() - running := s.running - s.backingOff - for _, h := range s.active { - if h.cancelled { - running-- - } - } - s.mu.Unlock() + defer s.mu.Unlock() + room := max(0, s.opts.Max-s.running) + committed := max(0, s.live()-s.pending) sr, ok := s.opts.Provisioner.(provisioner.SlotReporter) if !ok { - return s.opts.Max + return committed + s.pending + room } - // Pending launches are in running but not yet out of the free count, so - // they net out of the sum. When more launches were committed than the pool - // has slots (they will fail), the sum falls below running, which is right: - // only the ones that get a slot can serve. - total := running + sr.FreeSlots() - s.opts.Pending.Count() - return max(0, min(s.opts.Max, total)) + free := sr.FreeSlots() + willBoot := min(s.pending, free) + headroom := max(0, min(room, free-s.opts.Pending.Count())) + return committed + willBoot + headroom } // Drain blocks until all in-flight microVMs have exited. diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index f9d99d9..cc3ec14 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -41,22 +41,27 @@ func (jitStub) Generate(context.Context, core.RunnerSpec) (string, string, error func TestPlan(t *testing.T) { cases := []struct { - name string - desired, running, max int - want int + name string + desired, live, running, max int + want int }{ - {"none wanted", 0, 0, 4, 0}, - {"scale from zero", 3, 0, 4, 3}, - {"bounded by max", 10, 0, 4, 4}, - {"partial capacity", 10, 3, 4, 1}, - {"at capacity", 4, 4, 4, 0}, - {"desired below running", 1, 3, 4, 0}, - {"over capacity running", 10, 5, 4, 0}, + {"none wanted", 0, 0, 0, 4, 0}, + {"scale from zero", 3, 0, 0, 4, 3}, + {"bounded by max", 10, 0, 0, 4, 4}, + {"partial capacity", 10, 3, 3, 4, 1}, + {"at capacity", 4, 4, 4, 4, 0}, + {"desired below running", 1, 3, 3, 4, 0}, + {"over capacity running", 10, 5, 5, 4, 0}, + // A launch in backoff or a VM tearing down is in running but not live: + // demand still gets a fresh launch, within the concurrency bound. + {"demand ignores a launch in backoff", 1, 0, 1, 4, 1}, + {"but running still bounds the attempts", 3, 0, 4, 4, 0}, + {"room left after dead entries", 3, 1, 2, 4, 2}, } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - if got := plan(tc.desired, tc.running, tc.max); got != tc.want { - t.Fatalf("plan(%d,%d,%d)=%d want %d", tc.desired, tc.running, tc.max, got, tc.want) + if got := plan(tc.desired, tc.live, tc.running, tc.max); got != tc.want { + t.Fatalf("plan(%d,%d,%d,%d)=%d want %d", tc.desired, tc.live, tc.running, tc.max, got, tc.want) } }) } @@ -397,12 +402,12 @@ func TestPendingSettlesBeforeJITFailureBackoff(t *testing.T) { case <-time.After(5 * time.Millisecond): } } + // backoff takes the lock a moment after enter() releases the reservation, + // so wait for the count to settle rather than sampling once. + waitCapacity(t, s, 3) // 0 live + min(Max-1 running, 3 free) if got := s.Running(); got != 1 { t.Fatalf("running=%d want 1: the goroutine should still be in backoff", got) } - if got := s.Capacity(); got != 3 { - t.Fatalf("capacity=%d want 3 (0 live + 3 free, none reserved)", got) - } s.Drain() } @@ -421,12 +426,133 @@ func waitCapacity(t *testing.T, s *Scheduler, want int) { } } -func TestCapacityWithoutSlotReporterIsMax(t *testing.T) { - prov := provFunc(func(context.Context, string, string, core.RunnerSpec, func()) error { return nil }) +func TestReconcileServesDemandPastALaunchInBackoff(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + var attempts int32 + entered := make(chan struct{}, 8) + prov := provFunc(func(vmCtx context.Context, name, jit string, spec core.RunnerSpec, onBusy func()) error { + if atomic.AddInt32(&attempts, 1) == 1 { + return errors.New("boot failed") // first attempt dies and backs off + } + entered <- struct{}{} + <-vmCtx.Done() + return nil + }) + s := New(Options{Max: 4, Min: 0, Provisioner: prov, JIT: &jitSeq{}, Logger: testLogger()}) + + s.Reconcile(ctx, 1) + waitCapacity(t, s, 3) // the failed launch is in backoff: 0 live, room 3 + if got := s.Running(); got != 1 { + t.Fatalf("running=%d want 1 (the launch in backoff)", got) + } + + // GitHub's next callback still reports one assigned job. Before, plan + // measured it against running and launched nothing, leaving the job to + // wait out the backoff; it must get a fresh VM now. + s.Reconcile(ctx, 1) + select { + case <-entered: + case <-time.After(2 * time.Second): + t.Fatal("demand was not served while a failed launch backed off") + } + if got := s.Running(); got != 2 { + t.Fatalf("running=%d want 2 (one live, one in backoff)", got) + } + cancel() + s.Drain() +} + +func TestReconcileServesDemandWhileCancelledVMsTearDown(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + entered := make(chan struct{}, 8) + exit := make(chan struct{}) + prov := provFunc(func(vmCtx context.Context, name, jit string, spec core.RunnerSpec, onBusy func()) error { + entered <- struct{}{} + <-vmCtx.Done() + <-exit + return nil + }) + s := New(Options{Max: 4, Min: 0, Provisioner: prov, JIT: &jitSeq{}, Logger: testLogger()}) + + s.Reconcile(ctx, 2) + <-entered + <-entered + s.Reconcile(ctx, 0) // both cancelled, both stuck in teardown + if got := s.Running(); got != 2 { + t.Fatalf("running=%d want 2 while the cancelled VMs tear down", got) + } + + // Demand returns before they have exited: neither can serve it. + s.Reconcile(ctx, 1) + select { + case <-entered: + case <-time.After(2 * time.Second): + t.Fatal("demand was not served while cancelled VMs tore down") + } + close(exit) + cancel() + s.Drain() +} + +func TestScaleDownDoesNotCancelALiveVMForALaunchInBackoff(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + + var attempts int32 + entered := make(chan struct{}, 8) + cancelled := make(chan struct{}, 8) + prov := provFunc(func(vmCtx context.Context, name, jit string, spec core.RunnerSpec, onBusy func()) error { + if atomic.AddInt32(&attempts, 1) == 1 { + return errors.New("boot failed") + } + entered <- struct{}{} + <-vmCtx.Done() + cancelled <- struct{}{} + return nil + }) + s := New(Options{Max: 4, Min: 0, Provisioner: prov, JIT: &jitSeq{}, Logger: testLogger()}) + + s.Reconcile(ctx, 2) + <-entered + waitCapacity(t, s, 3) // 1 live + room 2 (running=2 with one in backoff) + + // Demand holds at 1: the live VM is exactly what is needed. Measured + // against running (2) it would look like excess and be cancelled. + s.Reconcile(ctx, 1) + select { + case <-cancelled: + t.Fatal("the only live VM was cancelled to make room for a launch in backoff") + case <-time.After(100 * time.Millisecond): + } + cancel() + s.Drain() +} + +func TestCapacityWithoutSlotReporterIsBoundedOnlyByMax(t *testing.T) { + release := make(chan struct{}) + entered := make(chan struct{}, 8) + prov := provFunc(func(ctx context.Context, name, jit string, spec core.RunnerSpec, onBusy func()) error { + entered <- struct{}{} + <-release + return nil + }) s := New(Options{Max: 3, Provisioner: prov, JIT: jitStub{}, Logger: testLogger()}) if got := s.Capacity(); got != 3 { t.Fatalf("capacity=%d want Max(3) when the provisioner has no shared pool", got) } + s.Reconcile(context.Background(), 2) + for i := 0; i < 2; i++ { + <-entered + } + if got := s.Capacity(); got != 3 { + t.Fatalf("capacity=%d want 3 (2 live + 1 room)", got) + } + close(release) + s.Drain() } func TestReconcileZeroDoesNothing(t *testing.T) { From d58c65f55e5e608c2d13b401157d934c72bf4301 Mon Sep 17 00:00:00 2001 From: Marty Molt Date: Tue, 15 Sep 2026 16:35:46 -0500 Subject: [PATCH 13/14] scheduler: state the post-Launch sampling window and pin the no-pool contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The comment claimed the completion update closed the gap between the provisioner freeing its slot and the scheduler settling the count; it only makes the two scheduler-side updates agree. Say so, and why the remaining few-instruction window is left alone. A provisioner without a shared pool is bounded only by Max - running, so a launch in backoff still lowers its capacity — matching what plan would launch into. Cover that transient explicitly. Refs #9 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/scheduler/scheduler.go | 17 +++++++++++------ internal/scheduler/scheduler_test.go | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index b72f7f3..93abdb4 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -272,11 +272,14 @@ func (s *Scheduler) launchOne(ctx context.Context) { }() launchErr := s.opts.Provisioner.Launch(vmCtx, name, jit, s.opts.Spec, func() { s.MarkBusy(name) }) - // The VM is gone and its slot is back in the pool. Drop the handle in the - // same critical section that settles the count, so no sample sees the VM - // and its freed slot at once, and a scale-down during the backoff below - // cannot mark a dead launch cancelled. A failed launch keeps its place in - // running through backoff so plan bounds the retry attempts. + // The VM is gone and its slot is back in the pool. Settle the count and + // drop the handle in one critical section, so the two never disagree and a + // scale-down during the backoff below cannot mark a dead launch cancelled. + // The provisioner released the slot just before returning, so a Capacity + // sample between that release and this lock still sees the VM and its + // freed slot together; the window is a few instructions and closing it + // would tie the provisioner's slot lifecycle to this lock. A failed launch + // keeps its place in running through backoff so plan bounds the retries. s.mu.Lock() delete(s.active, name) failed := launchErr != nil && vmCtx.Err() == nil @@ -458,7 +461,9 @@ func (s *Scheduler) Running() int { // Launches sleeping in backoff and VMs tearing down after a cancel are in // running but not in live: they cannot take a job, and their slot is either // absent or not yet back in the pool. A provisioner without a shared pool has -// no slot constraint, so only Max - running bounds the last term. +// no slot constraint, so only Max - running bounds the last term — which is +// still below Max while such entries hold places in running, exactly as plan +// would refuse to launch into them. // // Every tier on the host sees the same free count, so the tiers' advertised // values can sum to more than the pool while several are idle, and a burst diff --git a/internal/scheduler/scheduler_test.go b/internal/scheduler/scheduler_test.go index cc3ec14..918dfea 100644 --- a/internal/scheduler/scheduler_test.go +++ b/internal/scheduler/scheduler_test.go @@ -555,6 +555,25 @@ func TestCapacityWithoutSlotReporterIsBoundedOnlyByMax(t *testing.T) { s.Drain() } +func TestCapacityWithoutSlotReporterStillDiscountsBackoff(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + defer cancel() + prov := provFunc(func(context.Context, string, string, core.RunnerSpec, func()) error { + return errors.New("boot failed") + }) + s := New(Options{Max: 4, Min: 0, Provisioner: prov, JIT: jitStub{}, Logger: testLogger()}) + s.Reconcile(ctx, 1) + // No pool to constrain it, but the launch in backoff still holds one of + // the Max places plan will hand out, so capacity says 3, not 4, until the + // backoff ends and the goroutine exits. + waitCapacity(t, s, 3) + if got := s.Running(); got != 1 { + t.Fatalf("running=%d want 1 (the launch in backoff)", got) + } + cancel() + s.Drain() +} + func TestReconcileZeroDoesNothing(t *testing.T) { prov := provFunc(func(context.Context, string, string, core.RunnerSpec, func()) error { t.Fatal("Launch should not be called for desired=0") From 065717231695e102890146ac18e14cddead74cfc Mon Sep 17 00:00:00 2001 From: Marty Molt Date: Tue, 15 Sep 2026 16:55:57 -0500 Subject: [PATCH 14/14] scheduler: keep the drain-race comment on the defer it describes Refs #9 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- internal/scheduler/scheduler.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/scheduler/scheduler.go b/internal/scheduler/scheduler.go index 93abdb4..72555f4 100644 --- a/internal/scheduler/scheduler.go +++ b/internal/scheduler/scheduler.go @@ -192,9 +192,6 @@ func (s *Scheduler) scaleDown(desired int) int { func (s *Scheduler) launchOne(ctx context.Context) { defer s.wg.Done() - // This runs before wg.Done above (defers are LIFO), so the WaitGroup counter - // stays >=1 while maintainMinimum may Add a replacement — avoiding a - // concurrent-Add-during-Drain race. finished := false finish := func() { if !finished { @@ -202,6 +199,9 @@ func (s *Scheduler) launchOne(ctx context.Context) { s.running-- } } + // This runs before wg.Done above (defers are LIFO), so the WaitGroup counter + // stays >=1 while maintainMinimum may Add a replacement — avoiding a + // concurrent-Add-during-Drain race. defer func() { s.mu.Lock() finish()