From 734195e54eedb0938ea4b9a82eb2c7b309f5326e Mon Sep 17 00:00:00 2001 From: unkin-agent Date: Thu, 13 Aug 2026 19:55:21 +1000 Subject: [PATCH] proxy: snapshot in-flight counts before sorting (least_conn selection O(n)) (#124) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why `least_conn` mirror selection (`baseURLAttemptOrder`) scaled super-linearly. After rotating the pool by the round-robin cursor it `sort.SliceStable`d with a comparator that called `inflightCounter` on **every comparison** — and each call did a `remoteName+"\x00"+url` concat plus a `sync.Map` `LoadOrStore` with a speculative `new(atomic.Int64)`. So each selection cost O(n·log n) map lookups + allocations, all on the cache-miss/upstream path. ## How Snapshot each mirror's in-flight count **once**, then sort the snapshot — O(n) map loads, zero comparator allocations. - Add read-only `inflightCount(name, url) int64`: plain `sync.Map` `Load`, returns 0 when the gauge is absent (no `LoadOrStore`, no speculative allocation). - `least_conn` branch builds a `{url, count}` snapshot via one `inflightCount` per rotated URL, `sort.SliceStable` by `count` ascending, then extracts the URLs. - `beginAttempt`/`endAttempt` keep the create-on-write `inflightCounter` path — they legitimately need to create the gauge. ## Numbers (`BenchmarkBaseURLAttemptOrder_LeastConn`, Ryzen 7 4700U, best of 3) | pool | before ns/op | after ns/op | before allocs | after allocs | before B/op | after B/op | |------|-------------:|------------:|--------------:|-------------:|------------:|-----------:| | 3 | ~2516 | ~1492 | 22 | 8 | 474 | 296 | | 8 | ~14647 | ~3319 | 131–132 | 8 | 2688 | 568 | Allocs are now **constant** regardless of pool size; pool-8 is ~4.8x faster with ~16x fewer allocations. ## Behavior Unchanged: least-loaded first, RR rotation as the stable tie-break, `round_robin` and single-URL paths untouched. Pure internal optimization — no API/schema/DB change. Added a multi-mirror tie-break test asserting all-equal load yields the RR rotation; `make test` (`-race`) green, vet/fmt clean. Reviewed-on: https://git.unkin.net/unkin/artifactapi/pulls/124 Co-authored-by: unkin-agent Co-committed-by: unkin-agent --- internal/proxy/engine.go | 32 +++++++++++++++++++++-- internal/proxy/leastconn_test.go | 45 ++++++++++++++++++++++++++++++++ 2 files changed, 75 insertions(+), 2 deletions(-) diff --git a/internal/proxy/engine.go b/internal/proxy/engine.go index c39ca98..5040ead 100644 --- a/internal/proxy/engine.go +++ b/internal/proxy/engine.go @@ -760,13 +760,41 @@ func (e *Engine) baseURLAttemptOrder(remote models.Remote) []string { ordered[i] = urls[(start+i)%len(urls)] } if remote.MirrorStrategy == models.MirrorStrategyLeastConn { - sort.SliceStable(ordered, func(a, b int) bool { - return e.inflightCounter(remote.Name, ordered[a]).Load() < e.inflightCounter(remote.Name, ordered[b]).Load() + // Snapshot each mirror's in-flight count once, then sort the snapshot. + // Reading the gauge inside the comparator would repeat an allocating + // sync.Map lookup on every comparison (O(n log n) lookups); this is O(n). + snap := make([]inflightSnapshot, len(ordered)) + for i, url := range ordered { + snap[i] = inflightSnapshot{url: url, count: e.inflightCount(remote.Name, url)} + } + sort.SliceStable(snap, func(a, b int) bool { + return snap[a].count < snap[b].count }) + for i := range snap { + ordered[i] = snap[i].url + } } return ordered } +// inflightSnapshot pairs a mirror URL with its sampled in-flight count so the +// least_conn sort compares plain ints instead of re-reading the gauge. +type inflightSnapshot struct { + url string + count int64 +} + +// inflightCount reads the in-flight request gauge for a given (remote, upstream +// URL) without creating it, returning 0 when the counter is absent. This keeps +// the selection read path allocation-free (plain Load, no LoadOrStore). +func (e *Engine) inflightCount(remoteName, url string) int64 { + v, ok := e.inflight.Load(remoteName + "\x00" + url) + if !ok { + return 0 + } + return v.(*atomic.Int64).Load() +} + // inflightCounter returns the shared in-flight request gauge for a given // (remote, upstream URL), creating it on first use. func (e *Engine) inflightCounter(remoteName, url string) *atomic.Int64 { diff --git a/internal/proxy/leastconn_test.go b/internal/proxy/leastconn_test.go index 097f921..e47a28d 100644 --- a/internal/proxy/leastconn_test.go +++ b/internal/proxy/leastconn_test.go @@ -40,6 +40,51 @@ func TestLeastConnPicksLeastLoaded(t *testing.T) { } } +// TestLeastConnStableTieBreak asserts that when every mirror carries equal +// in-flight load, least_conn falls back to the round-robin rotation: the +// snapshot sort is stable, so tied mirrors keep the RR-rotated order and the +// starting pick advances across the whole pool on successive calls. +func TestLeastConnStableTieBreak(t *testing.T) { + e := &Engine{} + r := models.Remote{ + Name: "lc-tie", + BaseURL: "https://a.example", + Mirrorlist: []string{"https://b.example", "https://c.example"}, + MirrorStrategy: models.MirrorStrategyLeastConn, + } + pool := r.UpstreamPool() + + // Equal (zero) load on every mirror: order must equal the RR rotation. + starts := map[string]int{} + for i := 0; i < len(pool); i++ { + order := e.baseURLAttemptOrder(r) + if len(order) != len(pool) { + t.Fatalf("attempt %d: order len = %d, want %d", i, len(order), len(pool)) + } + // A stable sort of an all-tied slice is a pure RR rotation: for the + // call whose cursor selects start s, order must be pool rotated by s. + start := indexOf(pool, order[0]) + for j := range order { + if want := pool[(start+j)%len(pool)]; order[j] != want { + t.Fatalf("attempt %d: order[%d] = %q, want RR-rotated %q", i, j, order[j], want) + } + } + starts[order[0]]++ + } + if len(starts) != len(pool) { + t.Fatalf("tied least_conn did not rotate across the whole pool: %v", starts) + } +} + +func indexOf(s []string, v string) int { + for i := range s { + if s[i] == v { + return i + } + } + return -1 +} + // TestRoundRobinDefaultUnchanged asserts an unset strategy still rotates the // starting mirror across the pool and ignores the in-flight gauge. func TestRoundRobinDefaultUnchanged(t *testing.T) {