Skip to content

perf: a watch pinned to one name polls only the node that holds its entry - #48

Merged
CMGS merged 3 commits into
masterfrom
perf/pinned-watch-polls-one-node
Sep 25, 2026
Merged

CMGS merged 3 commits into
masterfrom
perf/pinned-watch-polls-one-node

Conversation

@CMGS

@CMGS CMGS commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor

Replaces #47. Squashing #46 left #47's branch in conflict with master, and moving it would need a force push, so this is the same commit on the merged master, with the same tree.

Problem

Every kubectl wait and kubectl delete opens a watch pinned to one name. Each one-second tick of that watch called listInventories: it read every node's inventory, built a Sandbox for every entry in the namespace and sorted them, only to follow one name. The cost grows with the whole fleet, and each concurrent wait pays it again. #46 listed this as its follow-up.

Change

  • runWatch picks its poll once: a pinned watch calls pollPinned, and every other watch keeps listInventories.
  • While the watch holds its entry, pollPinned reads only that entry's node through matchOnNode.
  • While it holds none, it sweeps the inventories with FirstHit and stops at the first hit. It asks no node's sandboxd, so an absent name still costs no node request.
  • The keep-while-the-node-holds-it check from fix: a list or watch pinned to one name agrees with Get before the node publishes #46 is unchanged.
  • listPinned and pollPinned share selectedOne, which returns zero or one sandbox that passes the selectors.
  • A repeated Create can leave one name claimed on two nodes. The sweep skips a claim the selectors reject, so it cannot hide the one they accept; the fleet poll it replaces filtered first too.
  • A pinned list whose pick the selectors reject falls back to that sweep, so a pinned list and a pinned watch agree on the same duplicate.
  • docs/scaling-design.md states the exception to "one watcher per fleet".

Behavior notes:

  • A name deleted on one node and claimed again on another is now reported as Deleted, then Added on the next tick. A fleet watch already reports it that way when the two land in different ticks. Synthesized sandboxes carry no UID, so an informer converges either way.
  • A name claimed again on the same node is still reported as Modified.

Cost

The claim path is unchanged. Production code is +32/−5, with no comment added. The list fallback runs only when the pick is rejected, and costs one sweep of the cached inventories.

BenchmarkClientInventoryWatchTick runs one pinned tick through the informer-fed cache that production uses. Each arm ran 6 times, interleaved, 3 rounds in each order. The figures are medians.

fleet (nodes × sandboxes per node) fleet poll pinned poll ratio
26 × 100 2.05 ms, 9.7 MB 0.062 ms, 0.024 MB 33×
26 × 2000 23.2 ms, 192 MB 1.11 ms, 0.36 MB 21×
200 × 2000 164 ms, 1480 MB 1.11 ms, 0.36 MB 147×

The pinned tick grows with the entries on one node, not with the fleet. It still decodes that node's whole inventory, which is the shared decode every read path uses.

Tests

TestANamePinnedWatchPollsOnlyTheNodeThatHoldsItsEntry in pkg/scale/sandboxstore_live_test.go, under synctest:

  • five ticks on a held entry send no event and do not enumerate the fleet;
  • a change on the entry's node is Modified, still without a fleet enumeration;
  • the name claimed again on another node is Deleted, then Added.

With the tick forced back to listInventories, both fleet-enumeration assertions fail. The #46 pinned-watch tests pass unchanged.

TestANamePinnedWatchSweepSkipsADuplicateItsSelectorsReject and TestANamePinnedListSkipsADuplicateItsSelectorsReject put a Paused claim on one node and a Running claim under the same name on another, then select phase=Running. Each fails without its fix (8 of 8 runs) and passes with it.

Gates, all with GOWORK=off:

  • make fmt-check
  • make lint: 6 × 0 issues.
  • go test -race -count=1 ./...
  • asl ./... on darwin and linux: only the two pkg/envdproxy forwarder advisories (withTarget, sandboxHost), which are kept

Hardware verification

Pending. The next two-host round re-runs SBL-06b and SBL-08 on this branch.

…ntry

Every kubectl wait and kubectl delete opens a watch pinned to one name, and each of its one-second ticks re-derived the whole fleet's inventories to follow that name. While the watch holds its entry, a tick now reads only that entry's node. While it holds none, the tick sweeps the inventories until the first hit, without building the rest of the fleet. The keep-while-the-node-holds-it check is unchanged, and fleet watches are unchanged.

BenchmarkClientInventoryWatchTick, informer-fed cache, median of 6 runs, arms interleaved in both orders: 26x100 2.05 ms to 0.062 ms, 26x2000 23.2 ms to 1.11 ms, 200x2000 164 ms to 1.11 ms per tick.
…uplicate

A repeated Create under one name leaves two claims on two nodes. While the watch held no entry, the sweep took whichever claim answered first and filtered it afterwards, so a claim the selectors rejected could hide the one they accept and delay its Added by several ticks. The fleet poll this replaced filtered first. The sweep now skips a claim the selectors reject.
…cate they accept

A repeated Create leaves one name claimed on two nodes. The pinned list resolved like Get and filtered afterwards, so a claim the selectors reject hid the one they accept, while the pinned watch already skipped it. The list now falls back to the watch's selector-aware sweep when its pick is rejected.
@CMGS

CMGS commented Sep 25, 2026 •

Copy link
Copy Markdown
Contributor Author

Two-host hardware A/B, 2026-09-25. Master 34fd93c against the record build: master plus #48 (a428ff8) plus #49, with the patch identical to 1d6c607. Both nodes had sandboxd and vk-sandbox behind request taps.

  • SBL-09. An e2b sandbox is read by id right after its create.
    • master: in 6 of 6 samples, the read also asked the node that did not claim it.
    • record build: that node was asked in 0 of 6 samples.
  • SBL-10. One name is claimed on both nodes, with the index on the paused claim. A pinned list selecting phase=Running is then issued.

The record build also carried the rest of the sandbox chain (41-sbx: 28/28 PASS) with no regression.

Observation, not new with these PRs: kubectl get --watch prints the object as ADDED twice. kubectl lists first, and the store's watch emits Added for its initial view; master has the same code. docs/usage.md does not state that a watch opened after a list replays the listed objects.

(Edited: an earlier version of this comment said the master watch reported the paused claim. It reported the running one.)

@CMGS
CMGS merged commit 48bb08f into master Sep 25, 2026
2 checks passed
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.

1 participant