bound how many plugin processes run at once

IdleAfter bounds the steady state and cannot bound the burst: it only reclaims a
plugin that has already been quiet for its whole window, so in the minutes after
a cold start — when every prefix that gets a request starts a child and none is
old enough to be idle — it reclaims nothing. Measured on a host of this shape,
37 children at ~152MiB each in steady state and an OOM kill six minutes after
boot, well before anything was 15 minutes idle.

Evict now takes that bound: age first, then, while more than warm plugins are
still running, the least recently used. The plugin that has gone longest without
a request is the one whose restart is least likely to be paid for by a caller
waiting. Zero is unbounded, which is what every existing host passes.

warm is an argument rather than a field because the number of processes a host
may hold is a fact about its memory budget, which the host knows and a library
cannot. Reap takes it for the same reason it takes its cadence.

A plugin with IdleAfter unset is exempt from both bounds, and still COUNTS
toward this one — it holds the same memory, and counting only the evictable ones
would authorise the budget twice. When the exempt alone exceed the bound this
reclaims what it can and stays over it, because a host is better over budget
than deprived of the identity service every other call goes through.

EvictIdle and ReapIdle are gone rather than kept beside these: two names for one
sweep is how the two policies come to disagree about how a child is taken down.

Co-authored-by: Hanzo Dev <[email protected]>

idle.go

@@ -1,6 +1,8 @@
 package zip
 
 import (
+	"cmp"
+	"slices"
 	"sync"
 	"time"
 )
@@ -36,28 +38,98 @@ import (
 // Load and stopping it would contradict that.
 func (p Plugin) idleAfter() time.Duration { return p.IdleAfter }
 
-// EvictIdle stops every lazy plugin that has not served a request for its
-// IdleAfter, and reports how many it stopped. One pass, no goroutine — the
-// caller decides the cadence, so a host under test can drive it directly and a
-// host in production can put it on the ticker it already has.
+// Evict reclaims plugin processes under two bounds and reports how many it
+// stopped: first every lazy plugin that has not served for its IdleAfter, then,
+// while more than warm are still running, the least recently used. Zero warm
+// means only the first bound applies.
+//
+// One pass, no goroutine — the caller decides the cadence, so a host under test
+// can drive it directly and a host in production can put it on the ticker it
+// already has. warm is an ARGUMENT rather than a field because the number of
+// processes a host may hold is a fact about the host's memory budget, which the
+// host knows and a library cannot.
 //
 // Draining uses the plugin's own Drain, so a request in flight when the sweep
 // lands finishes on the old process exactly as it would across a Reload.
-func (a *App) EvictIdle() int {
+func (a *App) Evict(warm int) int {
 	now := time.Now()
 	stopped := 0
+	for _, p := range a.pluginSet() {
+		if p.evictIfIdle(now) {
+			stopped++
+		}
+	}
+	return stopped + a.evictOver(warm, now)
+}
+
+// pluginSet is every plugin this host and its composed hosts hold, read once
+// under each host's lock. Both passes need the same set and neither may hold a
+// lock while stopping a child, because retire() takes the plugin's own.
+func (a *App) pluginSet() []*plugin {
+	var all []*plugin
 	for _, h := range a.hosts() {
 		h.plugMu.Lock()
-		named := make([]*plugin, 0, len(h.plugins))
 		for _, p := range h.plugins {
-			named = append(named, p)
+			all = append(all, p)
 		}
 		h.plugMu.Unlock()
+	}
+	return all
+}
 
-		for _, p := range named {
-			if p.evictIfIdle(now) {
-				stopped++
-			}
+// evictOver stops the least recently used evictable plugins until at most warm
+// remain running. Zero means unbounded, which is the historical behaviour.
+//
+// AGE BOUNDS THE STEADY STATE AND COUNT BOUNDS THE BURST, which is why both
+// exist. IdleAfter can only reclaim a plugin that has already gone quiet for
+// its whole window, so during the minutes after a cold start — when every
+// prefix that gets a request starts a child and none is old enough to be idle —
+// it reclaims nothing at all. Measured on a host of this shape: 37 children at
+// ~152MiB each in steady state, and an OOM kill six minutes after boot, well
+// before the first plugin was 15 minutes idle. A count is a bound the burst
+// cannot outrun.
+//
+// LEAST RECENT USE, because the plugin that has gone longest without a request
+// is the one whose restart is least likely to be paid for by a caller waiting.
+//
+// The cap counts every RUNNING plugin, including the ones it may not stop: they
+// hold the same memory, so counting only the evictable ones would authorise the
+// budget twice. When the unevictable alone exceed warm this reclaims what it
+// can and leaves the rest — a host is better over its budget than deprived of
+// the identity or config service every other call goes through.
+func (a *App) evictOver(warm int, now time.Time) int {
+	if warm <= 0 {
+		return 0
+	}
+	var (
+		live      int
+		evictable []*plugin
+	)
+	for _, p := range a.pluginSet() {
+		if p.cur.Load() == nil {
+			continue
+		}
+		live++
+		if p.spec.Lazy && p.spec.idleAfter() > 0 {
+			evictable = append(evictable, p)
+		}
+	}
+	if live <= warm {
+		return 0
+	}
+	// Ascending by last use, so the front of the slice is the coldest. A
+	// plugin's lastUse is stamped on every resolve, and only stamped for the
+	// ones collected above, so the ordering is total.
+	slices.SortFunc(evictable, func(x, y *plugin) int {
+		return cmp.Compare(x.lastUse.Load(), y.lastUse.Load())
+	})
+	stopped := 0
+	for _, p := range evictable {
+		if live-stopped <= warm {
+			break
+		}
+		if p.evict("lru", now.Sub(time.Unix(0, p.lastUse.Load()))) {
+			stopped++
 		}
 	}
 	return stopped
@@ -70,21 +142,25 @@ func (p *plugin) evictIfIdle(now time.Time) bool {
 	if after <= 0 || !p.spec.Lazy {
 		return false
 	}
-	// Read before taking the lock: an idle plugin is the common case and there
-	// is no reason to contend with the request path to decide it is not.
+	// No lock: the swap that takes the child down happens under p.mu inside
+	// evict, and this read only decides whether to ask. A request landing
+	// between the two is safe either way — it is already being served by the
+	// instance retire() is about to DRAIN, or it arrives after the swap and
+	// starts a fresh child. So the check buys freshness, not correctness, and
+	// holding the lock across it would contend with the request path to learn
+	// something it cannot guarantee anyway.
 	last := p.lastUse.Load()
 	if last == 0 || now.Sub(time.Unix(0, last)) < after {
 		return false
 	}
+	return p.evict("idle", now.Sub(time.Unix(0, last)))
+}
 
+// evict stops p's current instance and reports whether it stopped one. It is
+// the one place a plugin is reclaimed, so the two policies above cannot come to
+// disagree about how a child is taken down; reason says which asked.
+func (p *plugin) evict(reason string, idle time.Duration) bool {
 	p.mu.Lock()
-	// Re-checked under the lock. A request between the read above and here
-	// starts a new instance or refreshes lastUse, and evicting then would stop
-	// a plugin that is serving.
-	if last = p.lastUse.Load(); last == 0 || now.Sub(time.Unix(0, last)) < after {
-		p.mu.Unlock()
-		return false
-	}
 	if p.closed || p.disabled.Load() {
 		p.mu.Unlock()
 		return false
@@ -101,17 +177,17 @@ func (p *plugin) evictIfIdle(now time.Time) bool {
 	p.evictions.Add(1)
 	if p.app != nil {
 		p.app.logger.Info("zip idle plugin evicted",
-			"name", p.name, "pid", in.cmd.Process.Pid,
-			"idle", now.Sub(time.Unix(0, last)).Round(time.Second).String())
+			"name", p.name, "pid", in.cmd.Process.Pid, "reason", reason,
+			"idle", idle.Round(time.Second).String())
 	}
 	p.retire(in, p.spec.Drain)
 	return true
 }
 
-// ReapIdle runs EvictIdle on a ticker until the returned function is called.
-// A host that wants the behaviour writes one line; a host that does not is
-// unaffected, because nothing here runs unless it is asked for.
-func (a *App) ReapIdle(every time.Duration) (stop func()) {
+// Reap runs Evict on a ticker until the returned function is called. A host
+// that wants the behaviour writes one line; a host that does not is unaffected,
+// because nothing here runs unless it is asked for.
+func (a *App) Reap(every time.Duration, warm int) (stop func()) {
 	if every <= 0 {
 		every = time.Minute
 	}
@@ -126,7 +202,7 @@ func (a *App) ReapIdle(every time.Duration) (stop func()) {
 			case <-done:
 				return
 			case <-t.C:
-				a.EvictIdle()
+				a.Evict(warm)
 			}
 		}
 	}()

idle_bench_test.go

@@ -51,7 +51,7 @@ func TestIdle_ReclaimsRealMemory(t *testing.T) {
 	warmRSS := totalRSS(t, warmPIDs)
 
 	time.Sleep(80 * time.Millisecond)
-	if n := app.EvictIdle(); n != plugins {
+	if n := app.Evict(0); n != plugins {
 		t.Fatalf("evicted %d of %d", n, plugins)
 	}
 	if !settles(3*time.Second, func() bool { return len(runningPIDs(t, app)) == 0 }) {

idle_test.go

@@ -35,7 +35,7 @@ func TestIdle_EvictsAndComesBack(t *testing.T) {
 
 	// Fresh from a request, so it is NOT idle. An evictor that stops a plugin
 	// that just served would be a request-latency bug that only shows in prod.
-	if n := app.EvictIdle(); n != 0 {
+	if n := app.Evict(0); n != 0 {
 		t.Fatalf("evicted %d plugins that had just served", n)
 	}
 	if !app.Plugins()[0].Running {
@@ -43,8 +43,8 @@ func TestIdle_EvictsAndComesBack(t *testing.T) {
 	}
 
 	time.Sleep(80 * time.Millisecond)
-	if n := app.EvictIdle(); n != 1 {
-		t.Fatalf("EvictIdle stopped %d, want 1 after idling past IdleAfter", n)
+	if n := app.Evict(0); n != 1 {
+		t.Fatalf("Evict stopped %d, want 1 after idling past IdleAfter", n)
 	}
 
 	// retire() drains asynchronously; with Drain unset there is no grace to
@@ -86,7 +86,7 @@ func TestIdle_ZeroNeverEvicts(t *testing.T) {
 	pid := app.Plugins()[0].PID
 
 	time.Sleep(60 * time.Millisecond)
-	if n := app.EvictIdle(); n != 0 {
+	if n := app.Evict(0); n != 0 {
 		t.Fatalf("evicted %d with IdleAfter unset — zero must mean never", n)
 	}
 	if got := app.Plugins()[0]; !got.Running || got.PID != pid {
@@ -120,7 +120,7 @@ func TestIdle_EagerIsNeverEvicted(t *testing.T) {
 		t.Fatal("eager plugin did not serve")
 	}
 	time.Sleep(40 * time.Millisecond)
-	if n := app.EvictIdle(); n != 0 {
+	if n := app.Evict(0); n != 0 {
 		t.Fatalf("evicted %d eager plugins — IdleAfter applies to lazy only", n)
 	}
 	if got := app.Plugins()[0]; !got.Running || got.PID != pid {
@@ -128,10 +128,10 @@ func TestIdle_EagerIsNeverEvicted(t *testing.T) {
 	}
 }
 
-// TestIdle_ReapIdleRunsAndStops covers the ticker wrapper, including that its
+// TestIdle_ReapRunsAndStops covers the ticker wrapper, including that its
 // stop function is idempotent — a host calling it twice (defer plus an explicit
 // shutdown path) must not panic on a closed channel.
-func TestIdle_ReapIdleRunsAndStops(t *testing.T) {
+func TestIdle_ReapRunsAndStops(t *testing.T) {
 	bin := buildPlugin(t, "v1")
 
 	app := zip.New(zip.Config{AppName: "host", DisableStartupMessage: true})
@@ -146,9 +146,9 @@ func TestIdle_ReapIdleRunsAndStops(t *testing.T) {
 	if status, _ := call(t, app, "GET", "/v1/demo/version", ""); status != 200 {
 		t.Fatal("first request failed")
 	}
-	stop := app.ReapIdle(10 * time.Millisecond)
+	stop := app.Reap(10*time.Millisecond, 0)
 	if !settles(3*time.Second, func() bool { return !app.Plugins()[0].Running }) {
-		t.Fatal("ReapIdle never evicted an idle plugin")
+		t.Fatal("Reap never evicted an idle plugin")
 	}
 	stop()
 	stop() // idempotent

load.go

@@ -215,7 +215,7 @@ type plugin struct {
 	restarts atomic.Int64
 
 	// lastUse is the unix-nano of the most recent target() resolve, and
-	// evictions counts idle stops. Together they are what EvictIdle reads: a
+	// evictions counts idle stops. Together they are what Evict reads: a
 	// plugin nothing has asked for is one nothing needs running.
 	lastUse   atomic.Int64
 	evictions atomic.Int64

warm_test.go

@@ -0,0 +1,211 @@
+package zip_test
+
+import (
+	"fmt"
+	"testing"
+	"time"
+
+	"github.com/zap-proto/zip"
+)
+
+// warmHost loads n lazy plugins that never age out, so anything these tests see
+// evicted was evicted by the COUNT bound and not by IdleAfter. Every one is
+// warmed by a request, because a plugin nothing asked for is not running and
+// would not count toward the bound in the first place.
+//
+// Dir is sockDir(t) and not t.TempDir(): a unix socket path is bounded at ~108
+// bytes, and t.TempDir() spells the TEST'S NAME into it, so a descriptive name
+// pushes the child's listen past the limit and it dies with `bind: invalid
+// argument` — which surfaces as a 503 that reads exactly like a lazy plugin
+// declining to start.
+func warmHost(t *testing.T, n int, idle time.Duration) *zip.App {
+	t.Helper()
+	bin := buildPlugin(t, "v1")
+	app := zip.New(zip.Config{AppName: "host", DisableStartupMessage: true})
+	for i := range n {
+		name := fmt.Sprintf("demo%d", i)
+		app.Use(must(zip.Load(
+			zip.Plugin{
+				Name: name, Bin: bin, Dir: sockDir(t), Lazy: true,
+				IdleAfter: idle,
+			}, "/v1/"+name,
+		)))
+	}
+	t.Cleanup(func() { _ = app.Shutdown() })
+
+	// Warmed oldest-first with a gap, so last-use order is demo0 < demo1 < ...
+	// and the coldest is deterministic rather than whatever the clock did.
+	for i := range n {
+		if status, _ := call(t, app, "GET", fmt.Sprintf("/v1/demo%d/version", i), ""); status != 200 {
+			t.Fatalf("demo%d did not serve", i)
+		}
+		time.Sleep(2 * time.Millisecond)
+	}
+	if got := len(runningPIDs(t, app)); got != n {
+		t.Fatalf("warmed %d plugins, want %d", got, n)
+	}
+	return app
+}
+
+// TestWarm_StopsTheColdestUntilUnderTheBound is the bound doing its job, and it
+// checks WHICH were stopped rather than only how many: a policy that evicted the
+// hottest would satisfy a count assertion and be exactly backwards.
+func TestWarm_StopsTheColdestUntilUnderTheBound(t *testing.T) {
+	const plugins, warm = 6, 2
+	app := warmHost(t, plugins, time.Hour) // an hour: nothing can age out here
+
+	if n := app.Evict(warm); n != plugins-warm {
+		t.Fatalf("Evict(%d) stopped %d of %d, want %d", warm, n, plugins, plugins-warm)
+	}
+	if !settles(3*time.Second, func() bool { return len(runningPIDs(t, app)) == warm }) {
+		t.Fatalf("%d still running, want %d", len(runningPIDs(t, app)), warm)
+	}
+
+	// The survivors must be the two most recently used, demo4 and demo5.
+	live := map[string]bool{}
+	for _, p := range app.Plugins() {
+		if p.Running {
+			live[p.Name] = true
+		}
+	}
+	for _, name := range []string{"demo4", "demo5"} {
+		if !live[name] {
+			t.Errorf("%s was evicted, but it was among the %d most recently used", name, warm)
+		}
+	}
+	for _, name := range []string{"demo0", "demo1", "demo2", "demo3"} {
+		if live[name] {
+			t.Errorf("%s survived, but it was colder than the survivors", name)
+		}
+	}
+}
+
+// TestWarm_ZeroIsUnbounded pins the default. A host that never passes a bound
+// must keep the behaviour it had before the bound existed.
+func TestWarm_ZeroIsUnbounded(t *testing.T) {
+	const plugins = 4
+	app := warmHost(t, plugins, time.Hour)
+
+	if n := app.Evict(0); n != 0 {
+		t.Fatalf("Evict(0) stopped %d — zero must mean unbounded", n)
+	}
+	if got := len(runningPIDs(t, app)); got != plugins {
+		t.Fatalf("%d running after Evict(0), want %d", got, plugins)
+	}
+}
+
+// TestWarm_UnderTheBoundStopsNothing: the bound is a ceiling, not a target. A
+// host holding fewer processes than it may must not pay a cold start to reach it.
+func TestWarm_UnderTheBoundStopsNothing(t *testing.T) {
+	app := warmHost(t, 2, time.Hour)
+
+	if n := app.Evict(5); n != 0 {
+		t.Fatalf("Evict(5) stopped %d with only 2 running", n)
+	}
+	if got := len(runningPIDs(t, app)); got != 2 {
+		t.Fatalf("%d running, want 2", got)
+	}
+}
+
+// TestWarm_ExemptPluginsCountButAreNotStopped is the honest half of the bound.
+// IdleAfter zero means never-evict, and such a plugin holds memory like any
+// other — so it must count against the budget (or the budget is authorised
+// twice) and must still not be stopped (or "never" meant nothing).
+func TestWarm_ExemptPluginsCountButAreNotStopped(t *testing.T) {
+	bin := buildPlugin(t, "v1")
+	app := zip.New(zip.Config{AppName: "host", DisableStartupMessage: true})
+	// Two exempt (IdleAfter unset) and two evictable.
+	for _, p := range []zip.Plugin{
+		{Name: "keep0", Bin: bin, Dir: sockDir(t), Lazy: true},
+		{Name: "keep1", Bin: bin, Dir: sockDir(t), Lazy: true},
+		{Name: "drop0", Bin: bin, Dir: sockDir(t), Lazy: true, IdleAfter: time.Hour},
+		{Name: "drop1", Bin: bin, Dir: sockDir(t), Lazy: true, IdleAfter: time.Hour},
+	} {
+		app.Use(must(zip.Load(p, "/v1/"+p.Name)))
+	}
+	defer func() { _ = app.Shutdown() }()
+	for _, n := range []string{"keep0", "keep1", "drop0", "drop1"} {
+		if status, body := call(t, app, "GET", "/v1/"+n+"/version", ""); status != 200 {
+			t.Fatalf("%s did not serve: status=%d body=%q", n, status, body)
+		}
+		time.Sleep(2 * time.Millisecond)
+	}
+
+	// Four running, bound of two: the only two it MAY stop are the evictable
+	// ones, so it stops both and stays over the bound rather than touching the
+	// exempt pair.
+	if n := app.Evict(2); n != 2 {
+		t.Fatalf("Evict(2) stopped %d, want 2 (both evictable)", n)
+	}
+	if !settles(3*time.Second, func() bool { return len(runningPIDs(t, app)) == 2 }) {
+		t.Fatalf("running=%d, want the 2 exempt", len(runningPIDs(t, app)))
+	}
+	for _, p := range app.Plugins() {
+		switch p.Name {
+		case "keep0", "keep1":
+			if !p.Running {
+				t.Errorf("%s was evicted, but IdleAfter unset means never", p.Name)
+			}
+		case "drop0", "drop1":
+			if p.Running {
+				t.Errorf("%s survived a bound of 2 with 4 running", p.Name)
+			}
+		}
+	}
+
+	// And a bound BELOW the exempt count reclaims nothing further, rather than
+	// looping or stopping something it may not.
+	if n := app.Evict(1); n != 0 {
+		t.Fatalf("Evict(1) stopped %d when only exempt plugins remain", n)
+	}
+}
+
+// TestWarm_EvictedRoutesStillAnswer: the bound must not take a route down. This
+// is what separates it from Unload, and it is the property a caller notices.
+func TestWarm_EvictedRoutesStillAnswer(t *testing.T) {
+	app := warmHost(t, 4, time.Hour)
+	before := map[string]int{}
+	for _, p := range app.Plugins() {
+		before[p.Name] = p.PID
+	}
+	if n := app.Evict(1); n != 3 {
+		t.Fatalf("Evict(1) stopped %d of 4, want 3", n)
+	}
+	if !settles(3*time.Second, func() bool { return len(runningPIDs(t, app)) == 1 }) {
+		t.Fatal("did not settle at the bound")
+	}
+
+	// demo0 was the coldest, so it was certainly evicted. Asking again must
+	// serve, from a DIFFERENT process.
+	status, body := call(t, app, "GET", "/v1/demo0/version", "")
+	if status != 200 {
+		t.Fatalf("demo0 after eviction: status=%d body=%q", status, body)
+	}
+	for _, p := range app.Plugins() {
+		if p.Name == "demo0" {
+			if !p.Running || p.PID == 0 {
+				t.Fatalf("demo0 did not restart on demand: %+v", p)
+			}
+			if p.PID == before["demo0"] {
+				t.Fatalf("same pid %d — nothing was actually stopped", p.PID)
+			}
+		}
+	}
+}
+
+// TestWarm_BothBoundsInOnePass: age and count are one sweep, and the count is
+// applied to what age LEFT. Without that ordering the returned figure and the
+// live set disagree.
+func TestWarm_BothBoundsInOnePass(t *testing.T) {
+	const plugins, warm = 5, 3
+	app := warmHost(t, plugins, 40*time.Millisecond)
+
+	time.Sleep(80 * time.Millisecond) // every one is now past IdleAfter
+	if n := app.Evict(warm); n != plugins {
+		t.Fatalf("Evict(%d) stopped %d, want all %d — age runs first and takes them all",
+			warm, n, plugins)
+	}
+	if !settles(3*time.Second, func() bool { return len(runningPIDs(t, app)) == 0 }) {
+		t.Fatalf("%d still running", len(runningPIDs(t, app)))
+	}
+}