Conversation
…de publishes kubectl wait --for=delete and kubectl delete read the sandbox with Get, then open a watch-list with fieldSelector=metadata.name=<name>. Get asks the nodes for a claim the NodeInventory does not hold yet; List and Watch read only the inventories. So a sandbox claimed before its node published (up to 30 s) was found by the Get, missing from the watch's initial sync, and reported deleted at once. Before the watch-list bookmark fix the same wait hung instead. A list pinned to one name in a namespace now resolves through Get. A watch with that pin starts from it, and before it reports a known entry deleted it asks the entry's node, keeping the entry while the node holds it and it still matches the selectors. An absent name polls the inventories only, so the pin adds no node traffic. Fleet lists and watches are unchanged.
The SandboxStore and ClaimIDResolver interfaces, WithClaimRouting and WithWatchPollInterval already state every fact these five godocs repeated; the lifecycle verbs in the same package carry none (Comment Style: interface-implementing methods omit godoc).
…de publishes A read by name now takes the claim id, deadline and claim time from the node's own row, so synthAnnotations, lifecycle.md, usage.md and the lifecycle example no longer say these wait for a publish, and the example limits the lag to fleet List and Watch. scaling-design.md adds that a list or watch pinned to a name no node holds asks every node once when it opens.
Multi-line godoc and const comments become one fact per line. Comments that restated a name, a signature, the interface godoc or scaling-design.md are gone: warmCandidate, scatterGatherStore, its claim-routing fields, matchOnNode, parseSelectors, the embedded SandboxLifecycle note, and the watch cost figures the design doc already measures. The NodeInventoryGVK reason moves onto its var entry. The files drop from 82 and 146 comment lines to 32 and 59.
…ilent-node test runs under synctest heldByNode passes objKey as the claim ref that Claim and lookupName spell with namespacedName, so objKey now calls it. TestASilentNodeBoundsAMiss waited out two real 500 ms timeouts; under synctest they elapse on the fake clock, and dropping the timeout still fails it as a deadlock. unpublishedStore moves below the countingSource type it builds.
…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.
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #46. The base is
fix/name-pinned-reads, so this diff shows only this change.Problem
Every
kubectl waitandkubectl deleteopens a watch pinned to one name. Each one-second tick of that watch calledlistInventories: it read every node's inventory, built aSandboxfor 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 lists this as its follow-up.Change
runWatchpicks its poll once: a pinned watch callspollPinned, and every other watch keepslistInventories.pollPinnedreads only that entry's node throughmatchOnNode.FirstHitand stops at the first hit. It asks no node's sandboxd, so an absent name still costs no node request.listPinnedandpollPinnedshareselectedOne, which returns zero or one sandbox that passes the selectors.docs/scaling-design.mdstates the exception to "one watcher per fleet".Behavior notes:
Cost
The claim path is unchanged. Production code is +24/−5, with no comment added.
BenchmarkClientInventoryWatchTickruns 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.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
TestANamePinnedWatchPollsOnlyTheNodeThatHoldsItsEntryinpkg/scale/sandboxstore_live_test.go, under synctest:With the tick forced back to
listInventories, both fleet-enumeration assertions fail. The #46 pinned-watch tests pass unchanged.Gates, all with
GOWORK=off:make fmt-checkmake lint: 6 ×0 issues.make testasl -forwarder=false ./...on darwin and linuxMerging
Merge #46 first. Squash-merging #46 with its branch deleted closes this PR, and GitHub cannot reopen a PR whose base ref is gone. So retarget this PR to
masterbefore #46's branch is deleted.After the retarget, a squash merge of this PR applies cleanly: #46's commits here and its squash on
mastermake the same changes. Until this branch is rebased, the PR page also lists #46's lines. The rebase is a force push, so it is the owner's call.Hardware verification
Pending. The next two-host round re-runs SBL-06b and SBL-08 on this branch.