Re-Implement worker thread collision avoidance for warm pool adoption#437
Conversation
k8s-ci-robot
added
the
do-not-merge/work-in-progress
|
Skipping CI for Draft Pull Request. |
✅ Deploy Preview for agent-sandbox canceled.
|
k8s-ci-robot
added
approved
| } | ||
| // Update uses optimistic concurrency (resourceVersion) so concurrent | ||
| // claims racing to adopt the same sandbox will conflict and retry. | ||
| if err := r.Update(ctx, adopted); err != nil { |
There was a problem hiding this comment.
Should we update to server-side apply to the controller as a separate PR?
|
/ok-to-test |
k8s-ci-robot
added
the
ok-to-test
igooch
changed the title
igooch
force-pushed
the
collision-avoidance
branch
from
7305fdd to
ad3adc2
Compare
k8s-ci-robot
removed
the
do-not-merge/work-in-progress
| if len(readyCandidates) == 0 { | ||
| log.Info("No ready warm pool candidates, falling through to cold start", | ||
| "totalCandidates", len(candidates)) | ||
| if len(candidates) == 0 { |
There was a problem hiding this comment.
just to confirm if the warm pool contains a mix of ready and non-ready sandboxes, the hashed startIndex might point to a non-ready sandbox, causing the controller to adopt it even if fully ready ones exist earlier in the list. Is that ok ?
There was a problem hiding this comment.
It depends on the number of worker threads vs. ready sandboxes. The controller selects a starting index from a window size equal to the worker count (MaxConcurrentReconciles). If the window is wider than the number of ready sandboxes, the controller may adopt a non-ready sandbox first, even if ready ones exist earlier in the sorted list.
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: igooch, vicentefb The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
4 participants
This pull request restores the deterministic "window" selection logic for SandboxClaim workers when adopting resources from the warm pool. This mechanism was previously introduced in PR #391 but was inadvertently removed during the refactor in PR #395, which transitioned the warm pool to use full Sandbox CRs instead of bare pods.