perf(controllers): implement warm-pool assignment and async binding by vicentefb · Pull Request #497 · kubernetes-sigs/agent-sandbox

vicentefb · 2026-03-31T20:42:55Z

This PR re-architects how the SandboxClaimReconciler handles massive concurrency bursts (e.g., 300+ simultaneous claims). It shifts the controller from a polling model to a event-driven Watcher model, reducing CPU load, eliminating memory leaks.

In-Memory Sandbox Assignment

During a burst, 1,000 concurrent workers were executing r.List() to scan the entire namespace for available pods. This caused an O(N) memory leak, forcing the Go Garbage Collector to panic and consume 36%+ of the CPU.
The Fix: Built the WarmPoolAssigner to hook directly into the controller-runtime Informer cache. Ready Sandboxes are pushed into a buffered Go channel (chan types.NamespacedName). Workers now execute an O(1) channel pop, eliminating the O(N) memory allocations and database scans.

.Atomic-Like Handoffs & Mitigation of Sandbox Leaks

The old two-step assignment process was non-atomic. If a transient error occurred during the claim status update, sandboxes would permanently leak.
The Fix: By using the Go channel, we guarantee exclusive ownership of a Sandbox ID the exact nanosecond it is popped. The adoption PATCH is now fired in an asynchronous background worker, and the claim UID is locked in an inFlightClaims sync map. This entirely prevents double-assignments and natively mitigates Informer Lag race conditions.

args used:

        args:
        - --leader-elect=true
        - --extensions
        - --enable-pprof-debug
        - --kube-api-qps=600
        - --kube-api-burst=600
        - --sandbox-concurrent-workers=1000
        - --sandbox-claim-concurrent-workers=1000
        - --sandbox-warm-pool-concurrent-workers=20

1 single burst of 300 sandboxclaims with a Warmpool size of 600.

Agent Sandbox Claim Startup Latency (ms)

Run	P50 (ms)	P90 (ms)	P99 (ms)
Run 1	1588.3	3178.1	4817.8
Run 2	1174.9	2265.6	3221.8
Run 3	1369.4	2394.9	4576.4
Run 4	1577.9	3033.7	4803.3
Run 5	1691.1	3507.73	4850.7
Run 6	1744.20	3563.7	4925.2
Run 7	1402.3	2297.93	2499.4
Run 8	2036.2	4298.9	5200.0
Run 9	1723.76	3754.2	4875.4
Run 10	1391.6	2417.21	4630.76

Average Latencies

Percentile	Average (ms)
P50	1569.97
P90	3071.20
P99	4440.08

CPU pprof baseline:

CPU pprof this PR:

netlify · 2026-03-31T20:43:01Z

✅ Deploy Preview for agent-sandbox canceled.

Name	Link
🔨 Latest commit	`bb5c2ac`
🔍 Latest deploy log	https://app.netlify.com/projects/agent-sandbox/deploys/69cd85fd9708be00080577e3

vicentefb · 2026-03-31T20:43:16Z

/ok-to-test

aditya-shantanu · 2026-03-31T23:13:40Z

cmd/agent-sandbox-controller/main.go

+		}
+
+		if isReady {
+			templateName := sandbox.Labels["agents.x-k8s.io/sandbox-template-ref"]


Use constant SandboxTemplateRefAnnotation

aditya-shantanu · 2026-03-31T23:17:35Z

/assign @barney-s

aditya-shantanu · 2026-03-31T23:17:41Z

/priority critical-urgent

krzysied

IMO this 3 changes (assigner, async and patches) shouldn't be in single PR. They mostly independent of each other and can be done as separate changes.
Can you split them into separate PR?

krzysied · 2026-04-01T16:33:16Z

extensions/controllers/warmpool_assigner.go

+	w.Pools[hash] = ch
+
+	var sandboxes v1alpha1.SandboxList
+	if err := w.Client.List(ctx, &sandboxes, client.MatchingLabels{"agents.x-k8s.io/sandbox-template-ref-hash": hash}); err == nil {


Why do we need this?

krzysied · 2026-04-01T16:37:34Z

extensions/controllers/warmpool_assigner.go

+		}
+	}
+
+	if isReady {


In golang we try to omit unneeded nesting. Invert the if and return

krzysied · 2026-04-01T16:39:34Z

extensions/controllers/warmpool_assigner.go

+	}
+
+	isReady := false
+	for _, cond := range sandbox.Status.Conditions {


helper method, pls

krzysied · 2026-04-01T16:40:52Z

extensions/controllers/warmpool_assigner.go

+		ch, exists := w.Pools[templateHash]
+		w.mu.RUnlock()
+
+		if !exists {


Why we drop the entries instead of adding queue when needed?

krzysied · 2026-04-01T16:42:03Z

extensions/controllers/warmpool_assigner.go

+
+		if _, inFlight := w.InFlight.Load(sandbox.Name); !inFlight {
+			select {
+			case ch <- types.NamespacedName{Name: sandbox.Name, Namespace: sandbox.Namespace}:


IIUC this is going to drop the pod key when the capacity is reached. It will work well for small warm pools. With Warm pools above 1000 it will create an issue

krzysied · 2026-04-01T16:43:01Z

extensions/controllers/warmpool_assigner.go

+
+			// Must be Ready
+			isReady := false
+			for _, cond := range sb.Status.Conditions {


helper method

krzysied · 2026-04-01T16:45:40Z

extensions/controllers/warmpool_assigner.go

+
+type WarmPoolAssigner struct {
+	client.Client
+	mu       sync.RWMutex


nit: Let's keep field under mutex separated

field1 w/o mutex

field2 w/o mutex

1 line break

mutex

field3 w/ mutex

krzysied · 2026-04-01T16:46:30Z

extensions/controllers/warmpool_assigner.go

+}
+
+func (w *WarmPoolAssigner) GetOrCreatePool(ctx context.Context, hash string) chan types.NamespacedName {
+	w.mu.RLock()


helper method? It's easier to manage mutex this way

krzysied · 2026-04-01T17:09:26Z

extensions/controllers/sandboxclaim_controller.go

+		if err := r.Get(bgCtx, targetID, freshSandbox); err != nil {
+			logger.Error(err, "Async bind failed to fetch sandbox", "sandbox", targetID.Name)
+			r.inFlightClaims.Delete(owningClaim.UID)
+			r.Assigner.InFlight.Delete(targetID.Name)


The internal details of implementation are spilled across the code. Can we create separation layer? Interface:

Get()

Forget(sandbox)

Done(sanbox)
etc?

krzysied · 2026-04-01T17:10:50Z

extensions/controllers/warmpool_assigner.go

+			return
+		}
+
+		if _, inFlight := w.InFlight.Load(sandbox.Name); !inFlight {


It will be more coherent to keep same key as in the queue

krzysied · 2026-04-01T17:12:24Z

cmd/agent-sandbox-controller/main.go


 	if extensions {
+		// 1. Initialize the Assigner
+		assigner := &extensionscontrollers.WarmPoolAssigner{


Please add NewWarmPoolAssigner(client) method. Pools is internal field - can be initialized with a constructor method

k8s-ci-robot · 2026-04-01T20:51:41Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: aditya-shantanu, vicentefb
Once this PR has been reviewed and has the lgtm label, please ask for approval from barney-s. For more information see the 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:

OWNERS

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

…n all controllers

…have no owner

…oxes with channel pop nit fix fix nit nit nit nit clean up clean up comments nit lint fix

k8s-ci-robot · 2026-04-01T21:33:23Z

@vicentefb: The following tests failed, say /retest to rerun all failed tests or /retest-required to rerun all mandatory failed tests:

Test name	Commit	Details	Required	Rerun command
presubmit-agent-sandbox-lint-go	`bb5c2ac`	link	true	`/test presubmit-agent-sandbox-lint-go`
presubmit-agent-sandbox-e2e-test	`bb5c2ac`	link	true	`/test presubmit-agent-sandbox-e2e-test`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here.

justinsb · 2026-04-02T18:52:48Z

/unassign
/assign

k8s-ci-robot · 2026-04-02T19:58:31Z

PR needs rebase.

Details

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository.

vicentefb · 2026-04-02T20:06:48Z

This PR will be divided into multiple smaller PRs

/hold

k8s-ci-robot requested review from barney-s and soltysh March 31, 2026 20:43

k8s-ci-robot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. labels Mar 31, 2026

k8s-ci-robot added the ok-to-test Indicates a non-member PR verified by an org member that is safe to test. label Mar 31, 2026

vicentefb force-pushed the isolateKubeApiServer branch 2 times, most recently from c719809 to dd1c815 Compare March 31, 2026 21:09

aditya-shantanu approved these changes Mar 31, 2026

View reviewed changes

k8s-ci-robot assigned barney-s Mar 31, 2026

k8s-ci-robot added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Mar 31, 2026

vicentefb changed the title ~~perf(controllers): implement warm-pool assignment, async binding, and API request reduction~~ perf(controllers): implement warm-pool assignment and async binding Apr 1, 2026

krzysied reviewed Apr 1, 2026

View reviewed changes

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 1, 2026

vicentefb added 2 commits April 1, 2026 20:52

optimization: switch to JSON Merge Patches for updateStatus methods i…

35ef692

…n all controllers

optimization: add local informer cache to serve Ready sandboxes that …

5361998

…have no owner

vicentefb force-pushed the isolateKubeApiServer branch from a254f15 to 0db12d0 Compare April 1, 2026 20:53

optimization: replace the r.List() block that iterates over all sandb…

bb5c2ac

…oxes with channel pop nit fix fix nit nit nit nit clean up clean up comments nit lint fix

vicentefb force-pushed the isolateKubeApiServer branch from 0db12d0 to bb5c2ac Compare April 1, 2026 20:54

k8s-ci-robot assigned justinsb Apr 2, 2026

justinsb unassigned barney-s Apr 2, 2026

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Apr 2, 2026

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Apr 2, 2026

justinsb added the next-step:contributor label Apr 3, 2026

Conversation

vicentefb commented Mar 31, 2026

Uh oh!

netlify bot commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for agent-sandbox canceled.

Uh oh!

vicentefb commented Mar 31, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

aditya-shantanu commented Mar 31, 2026

Uh oh!

aditya-shantanu commented Mar 31, 2026

Uh oh!

krzysied left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

k8s-ci-robot commented Apr 1, 2026

Uh oh!

k8s-ci-robot commented Apr 1, 2026

Uh oh!

justinsb commented Apr 2, 2026

Uh oh!

k8s-ci-robot commented Apr 2, 2026

Uh oh!

vicentefb commented Apr 2, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

netlify bot commented Mar 31, 2026 •

edited

Loading