[manaspros]

What type of PR is this?

/kind enhancement

What this PR does / why we need it:

waitForClaimSandboxReadyWithTimeout read the SandboxClaim on a fixed 1 second ticker. A claim that got bound right after the first read still waited a full second before we noticed.

Now the wait starts at 100 ms and doubles up to the old 1 second: 100, 200, 400, 800, 1000, 1000 and so on. After the first second it behaves the same as before. It's a time.Timer reset each round. The context and timeout handling is untouched.

There are two commits. The first only adds the benchmark, so the "before" numbers can be reproduced from it.

go test ./pkg/workloadmanager -run xxx -bench BenchmarkWaitForClaimSandboxReady -benchmem -benchtime=5x -count=10
benchstat before.txt after.txt

                                      │    before    │                after                │
                                      │    sec/op    │   sec/op     vs base                │
WaitForClaimSandboxReady/bind20ms-24    1000.8m ± 0%   100.1m ± 0%  -90.00% (p=0.000 n=10)
WaitForClaimSandboxReady/bind300ms-24   1000.8m ± 0%   300.2m ± 0%  -70.01% (p=0.000 n=10)

                                      │    before    │                after                 │
                                      │   gets/op    │  gets/op    vs base                  │
WaitForClaimSandboxReady/bind20ms-24      3.000 ± 0%   3.000 ± 0%        ~ (p=1.000 n=10)
WaitForClaimSandboxReady/bind300ms-24     3.000 ± 0%   4.000 ± 0%  +33.33% (p=0.000 n=10)

The benchmark uses a fake dynamic client, the same kind the existing tests use. It binds the claim 20 ms or 300 ms after the wait starts, then serves a ready sandbox. So sec/op here is wait latency. I picked those bind delays myself. They aren't measurements of the real controller.

The cost is extra reads early on. A claim that stays unbound gets reads at 0, 0.1, 0.3, 0.7 and 1.5 s instead of 0 and 1 s, so 3 extra GETs in the first 1.5 s. After that it's one per second, as before.

Which issue(s) this PR fixes:
Fixes #482

Special notes for your reviewer:

A watch would be the cleaner end state; agent-sandbox's Go client does that. With auth on, the wait runs with the caller's own client, so a watch would need watch on sandboxclaims for every caller. I went with the smaller change first. I'm happy to do the watch instead if you prefer it.

TestWaitForClaimSandboxReadyNoticesQuickBindingWithoutWaitingASecond checks that a claim bound after 20 ms returns in under 700 ms. It's timing based, so tell me if you'd rather not have it. go test -race ./pkg/... passes.

Not tested: a real cluster, a real warm pool, or API server load with many concurrent creates.

AI use: I used Claude Code (Claude Sonnet 5.5 and Claude Opus 5.5) to find this and draft the code, benchmark and test. I've read and run all of it and can explain each line.

Does this PR introduce a user-facing change?:

Creating a CodeInterpreter session from a warm pool no longer waits a fixed second before noticing that the SandboxClaim is bound.

[volcano-sh-bot]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign yaozengzeng for approval. For more information see the Kubernetes Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• pkg/workloadmanager/OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[codecov-commenter]

⚠️ Please install the to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 61.26%. Comparing base (5903457) to head (24d29f5).
⚠️ Report is 37 commits behind head on main.
❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #483      +/-   ##
==========================================
+ Coverage   59.98%   61.26%   +1.27%     
==========================================
  Files          36       35       -1     
  Lines        3589     3643      +54     
==========================================
+ Hits         2153     2232      +79     
+ Misses       1216     1190      -26     
- Partials      220      221       +1     

Flag	Coverage Δ
unittests	61.26% <100.00%> (+1.27%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[acsoto]

The fake-client benchmark shows the polling delay, but do you have a before/after comparison with a real warm pool to show whether the latency reduction justifies the extra GETs?

[zhzhuang-zju]

A watch would be the cleaner end state; agent-sandbox's Go client does that. With auth on, the wait runs with the caller's own client, so a watch would need watch on sandboxclaims for every caller

I agree that a watch-based approach should remain our long-term goal, even if we keep the backoff for now because of the RBAC implications. Claiming a sandbox is a "fast path", so we should minimize avoidable delays at each step.

The fake-client benchmark shows the polling delay, but do you have a before/after comparison with a real warm pool to show whether the latency reduction justifies the extra GETs?

I found some performance data from upstream; Hope it helps.

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

[manaspros]

Thanks @zhzhuang-zju for the upstream numbers. @acsoto, to be honest I haven't measured against a real warm pool, only the fake client with a chosen bind delay. The upstream data suggests warm-pool binds take a few hundred ms, which is where the 1s ticker hurts most. Extra load is about 3 more GETs in the first 1.5s, then 1/s as before. I agree a watch is the better long-term fix. Happy to do it as a follow-up once we settle the RBAC question.

[acsoto]

/lgtm