Skip to content

[History Server] Fix cold-load CPU starvation and per-event logging - #5095

Open
Future-Outlier wants to merge 2 commits into
ray-project:masterfrom
Future-Outlier:historyserver-cold-load-cpu
Open

[History Server] Fix cold-load CPU starvation and per-event logging#5095
Future-Outlier wants to merge 2 commits into
ray-project:masterfrom
Future-Outlier:historyserver-cold-load-cpu

Conversation

@Future-Outlier

@Future-Outlier Future-Outlier commented Aug 6, 2026

Copy link
Copy Markdown
Member

Why are these changes needed?

  • CPU limit. Loading a dead session is CPU-bound, and the sample manifest's limits.cpu: "500m" is saturated for the entire load — measured 0.42–0.50 cores in every run, against ~1.2 cores of demand. It costs 2.5×: 50k tasks take 85s at 500m versus 32s at 2 and 34s at 4; 100k takes 151s versus 68s. Worse, it moves the largest openable session from ~200,000 tasks to ~79,000, because a load that exceeds --session-process-timeout is aborted and every retry starts over — so past that line a session is not slow, it cannot be opened at all.
  • Debug instead of Info. storeEvent runs once per event, so its logrus.Infof writes ~436,000 INFO lines per 100k-task cold load, and the binary never calls logrus.SetLevel, leaving operators no way to turn it off. Same session, only this line changed: 97.9s → 85.3s at 50k.

The binding constraint is the CFS quota, not the Go runtime. Holding the limit at 500m and forcing GOMAXPROCS=4 changes nothing (85.3s → 84.7s); pinning GOMAXPROCS=1 at 4 cores still loads 100k in 76.3s; and GOGC=400 cuts the GC share from 6% to 1% while the load still never completes. Cost per task is flat at both limits — 1.51 ms at 500m, 0.60 ms at 2–4 cores, measured from 1k to 200k tasks — so the limit only chooses which constant.

2 is the smallest limit that reaches the plateau; 4 is the same speed with more headroom for the ~2.2-core peaks, and 8 or no limit at all are no faster. requests stays at 500m, so scheduling cost is unchanged.

Worth considering separately: raising DefaultSessionProcessTimeout, or at least documenting it, since at the shipped default a 200k-task session needs 119.9s against a 120s limit.

Measurements, reproduction scripts and raw per-run reports (56 runs): https://github.com/Future-Outlier/historyserver-benchmark

Related issue number

None.

Checks

  • I've made sure the tests are passing.
  • Testing Strategy
    • Unit tests
    • Manual tests
    • This PR is not tested :(

Loading a dead session is CPU-bound, but the sample manifest sets only
`limits.cpu: "500m"`, so Kubernetes pins requests there too and the load
saturates the quota for its entire duration. Measured on kind across 13 runs,
the container sat at 0.42-0.50 cores every time.

Raising the limit to 4 (requests stay at 500m, so scheduling cost is unchanged):

| tasks in session | 500m | 4 cores |
|---|---|---|
| 50,000 | 97.9s | 30.5s |
| 100,000 | 907.3s | 62.7s |

This also removes what looked like superlinear degradation past 50k tasks: at
500m the per-task cost went from 1.96ms to 9.07ms between 50k and 100k, while at
4 cores it is flat (0.61ms and 0.63ms). The load uses ~1.2 cores on average and
peaks at ~2.2, because Go's GC runs concurrently and needs cores of its own.

The second change drops the per-event log line in storeEvent to Debug. It runs
once per event, so a 100k-task session writes ~436,000 INFO lines per cold load,
and the binary never calls logrus.SetLevel, so there is no way to turn it off.
Worth ~13% of load time at 50k tasks (97.9s -> 85.3s).
@Future-Outlier

Copy link
Copy Markdown
Member Author

The two red checks are pre-existing on master, not from this PR.

ray-operator/controllers/ray/rayservice_controller_unit_test.go:2637 calls record.NewFakeRecorder(10), but the file's import block only has k8s.io/client-go/tools/events — every other recorder in that file is events.*. So go vet ./ray-operator/... fails at master (72b9767) with undefined: record, which is what both "Build Operator Binaries" and "Lint (pre-commit)" are reporting. It looks like a semantic merge conflict between two PRs that each passed on their own.

This PR only touches historyserver/, and "Build historyserver" passes.

Comment on lines +67 to +73
requests:
cpu: "500m"
limits:
# Loading a dead session is CPU-bound: it uses ~1.2 cores with bursts
# to ~2.2. A 500m cap is saturated for the whole load, which made a
# 100k-task session take 907s instead of 63s.
cpu: "4"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
requests:
cpu: "500m"
limits:
# Loading a dead session is CPU-bound: it uses ~1.2 cores with bursts
# to ~2.2. A 500m cap is saturated for the whole load, which made a
# 100k-task session take 907s instead of 63s.
cpu: "4"
requests:
cpu: "500m"

Just wondering if we should only set the CPU requests by default to disable CFS quota, and let users configure CPU limits themselves if they want them.

https://x.com/thockin/status/1134193838841401345

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants