proxy: snapshot in-flight counts before sorting (least_conn selection O(n)) (#124)
ci/woodpecker/tag/docker Pipeline was successful
ci/woodpecker/tag/docker Pipeline was successful
## 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: #124
Co-authored-by: unkin-agent <unkin-agent@unkin.net>
Co-committed-by: unkin-agent <unkin-agent@unkin.net>
This commit was merged in pull request #124.
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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) {
|
||||
|
||||
Reference in New Issue
Block a user