tigerquoll opened a new pull request, #1121: URL: https://github.com/apache/yunikorn-core/pull/1121
Part of YUNIKORN-3350, which splits one change into three independent levers. This is the first; YUNIKORN-3352 and YUNIKORN-3353 build on it and will follow. ## What `Application.tryAllocate` calls `sa.queue.GetMaxAppUnschedAskBackoff()` inside the loop that walks the application's asks, so a value that is constant for the whole scheduling cycle is re-fetched — under the queue lock — once per ask walked. Hoist it above the loop. The entry guard changes from `sa.sortedRequests == nil` to `len(sa.sortedRequests) == 0` so the hoisted call still only happens when there is at least one ask to walk, preserving the existing "not called when there is no work" property. Four lines changed. ## Measured effect `BenchmarkScheduling` (`pkg/scheduler/tests`), allocate phase, 10000 pods. Alternating A/B against master, 3 samples per configuration, medians. Linux 6.8 aarch64, 6 CPUs, GOMAXPROCS=6. | Configuration | master (c/s) | this PR (c/s) | speedup | |---|---:|---:|---:| | 500 nodes | 5,481 | 6,747 | 1.23x | | 1000 nodes | 5,517 | 6,578 | 1.19x | | 2000 nodes | 5,396 | 6,597 | 1.22x | | 5000 nodes | 5,353 | 6,697 | 1.25x | Run-to-run spread up to 7%, so read these as "about 1.2x", not three significant figures. In a CPU profile of master, `Queue.GetMaxAppUnschedAskBackoff` is 18% of `Application.tryAllocate` — it is called once per iteration of a loop that, on master, also walks every already-allocated ask, so the two costs multiply. Note what that 18% actually is: **72% of it is acquiring the queue read lock** (0.23s of 0.32s), with another 6% releasing it. Only about a fifth is the field read. So this is mostly a lock-traffic change, not an arithmetic one. Scope: the benchmark drives the core through a mock resource manager with no real bind, so this is core scheduling headroom rather than a real-cluster pods/second figure. The workload is 2 applications x 5,000 asks; the cost removed scales with per-application ask count. Reproduce with: ``` go test ./pkg/scheduler/tests/ -run '^$' -bench 'BenchmarkScheduling' -benchtime=1x -v ``` ## Behaviour Unchanged. The golden decision-trace tests added in YUNIKORN-3338 reproduce byte-for-byte, goldens not regenerated. Full `pkg/scheduler/...` suite green under `-race`. The only semantic question is the guard change, and it moves in the safe direction: a nil slice and an empty slice both mean "no asks to walk", so `len(...) == 0` returns early in strictly more cases than `== nil` did — and in exactly the case where the loop body would not have executed anyway. ## Concurrency Worth spelling out for a change this small, because the interesting part is the locking rather than the arithmetic. `GetMaxAppUnschedAskBackoff` takes the **queue** read lock, and `tryAllocate` calls it while holding the **application** write lock. On master that nested acquisition happens once per ask walked, so the scheduling loop generated queue-lock traffic proportional to an application's ask count — on a lock shared by every application in that queue. After the hoist it happens once per scheduling cycle. No new lock is taken and the application-then-queue ordering is unchanged; there is simply far less of it. One genuine semantic change comes with it. `unschedAskBackoff` is written under the queue write lock by `UpdateQueueProperties`, which runs on a live configuration update. On master a configuration change landing mid-loop would be picked up by the remaining iterations of that same loop; now the whole cycle uses one snapshot and the new value takes effect on the next cycle. That is a change, but in the safer direction: a single scheduling decision now uses one consistent threshold instead of one that can shift underneath it, and the delay is bounded by a single cycle. The remaining per-iteration queue call, `GetBackoffDelay`, is untouched — it sits on the branch that returns immediately, so it runs at most once per cycle anyway. ## Scope and rollback No configuration, REST, scheduler-interface, or metrics changes. No new public API, no new state. Rollback is a single revert. Generated by Author with assistance from Claude Code. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
