Skip to content

review: a golden held for a claim's clone - #335

Merged
CMGS merged 7 commits into
mainfrom
review/whole-repo-1007
Oct 7, 2026
Merged

CMGS merged 7 commits into
mainfrom
review/whole-repo-1007

Conversation

@CMGS

@CMGS CMGS commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor

Whole-repo round on main 252019a (after #332 and #334): every production file in Go, Rust and Python, the shell drivers, the workflows, the Dockerfiles and the docs were read against the code. One bug fix and six small commits.

Fix

A dropped pool keeps its golden while a claim clones from it. Since #334 a pool dropped by PUT /v1/pools has its golden renamed and deleted once no build or refill is in flight. A claim that missed the warm pool clones from the same directory (ClaimProvision → resolveGolden) and held nothing on it, so a drop in that window deleted the golden under the clone and the claim failed.

  • The pool counts those clones (claimClones), and both drop sites wait for settled(): no build, no refill, no claim clone.
  • A removed pool no longer hands its golden to new claims: they cold-boot, as they do once the pool is gone. Without this a busy key could keep a removed pool alive. HasPoolGolden follows the same rule, so routing agrees with the claim path.
  • Cost: one more lock/unlock of the manager mutex per clone-tier claim. The warm claim path is unchanged.
  • TestAClaimCloneHoldsTheGoldenOfItsRemovedPool fails on each half reverted (golden deleted under a claim that is cloning from it; clones=2 colds=0).

Other changes

  • build: refresh Go and cargo dependencies — projecteru2/core v0.1.7, aws-sdk s3 v1.114.0 and transfermanager v0.4.13, puddle, go-colorable, genproto; cargo update for silkd (libc, mio, tokio 1.53.2) and boot/init (libc). All five Go modules tidied.
  • review (Go)
    • The restart sweep of fork-* staging dirs used filepath.Glob on data_dir, which matches nothing when the path holds a glob metacharacter. It now uses utils.RemoveDirEntries, as the dir store's staging sweep already does.
    • The peer transport takes the record's meta.json and export/ names from the store package: the tar is the staging layout the puller hands to store.Publish.
    • os.IsNotExist → errors.Is(err, os.ErrNotExist) at four sites.
    • ReloadConfig's godoc listed two of its three refusal causes.
  • review (Rust) — silkd checked an empty lsp manifest twice; the check stays where argv[0] is indexed.
  • docs
  • build: android flavor — the local-build SILKD_IMAGE default was still 0.1.13. CI passes the pinned carrier, so published images do not change.
  • ci — silkd.yml and boot-init.yml list their own file under pull_request too, like the other gates.

Lines

Go production: 27,435 → 27,441 (review commit −1, fix +7). Go tests +39 (the regression test). Rust −1. Comment lines +4 −4.

Verification

  • GOWORK=off make go-lint: 10 × 0 issues. (5 modules × linux/darwin), fmt --diff clean.
  • asl -forwarder=false ./...: no finding, 5 modules × 2 GOOS.
  • go test -race -count=1 ./... on all five modules: macOS, and linux/arm64 as a non-root user; the pool-removal tests 50 times under -race on Linux.
  • PostgreSQL suites (tenants, poolset, the cell tests in pool) against postgres:17: 26 passed, 0 skipped.
  • TestS3BackendContractRealEndpoint against an S3-compatible endpoint with the new SDK versions: pass.
  • silkd and boot/init: cargo fmt --check, clippy -D warnings, cargo test --locked on linux/arm64 with Rust 1.98.1.
  • Python: ruff 0.15.22 format + check, pytest (234 + 10 + 15 passed), mypy clean.
  • make sh-lint: clean.

Not run: hardware e2e.

CMGS added 7 commits October 7, 2026 02:26
Go: projecteru2/core v0.1.7, aws-sdk-go-v2 service/s3 v1.114.0 and
feature/s3/transfermanager v0.4.13, puddle v2.2.3, go-colorable v0.1.16,
genproto; all five modules tidied. Cargo: libc 0.2.190 in silkd and
boot/init, mio 1.2.4 and tokio 1.53.2 in silkd.
The restart sweep of fork-* staging dirs matched with filepath.Glob on
data_dir, which matches nothing when the path holds a glob metacharacter;
it now uses the ReadDir helper the dir store's staging sweep uses. The peer
transport spelled the record's meta.json and export/ names itself although
they are the staging layout the puller hands to store.Publish. ReloadConfig's
godoc listed two of its three refusal causes.
read_manifest and its only caller both rejected an empty argv.
- sandboxd-api: a reload also answers 409 when it drops a pool's egress
  policy that live claims use.
- deploy: the tenants DDL carries the DEFERRABLE unique constraint the node
  creates, so a table created ahead of time accepts a token swap in one PUT.
- egress: the golden stamp also records the image id and a sized pool's disk.
- sdk-python, README, index: the claim env and the tenant verbs are Go-only,
  like pool retuning.
- README: the credentials e2e driver, the guestserver helper and the mypy
  gate were missing from their lists.
- Go SDK: SetPools says an omitted pool loses its golden once idle.
The fallback stayed at 0.1.13 through three release bumps; CI passes the
pinned carrier, so only a local build took the old daemon.
Both workflows listed their own file under push only, unlike the shell,
python and sandboxd gates.
A claim that missed the warm pool cloned from the pool's golden with no
hold on it, so a PUT /v1/pools that dropped the key in that window renamed
and deleted the directory under the clone and the claim failed. The pool
now counts those clones and is dropped once they are done, like a build
or a refill in flight.

A removed pool no longer hands its golden to new claims: they cold-boot,
as they do once the pool is gone, so a busy key cannot keep it alive.
@CMGS CMGS changed the title review: whole-repo round on main after #334 review: whole-repo round on main after #334, and a golden held for a claim's clone Oct 6, 2026
@CMGS CMGS changed the title review: whole-repo round on main after #334, and a golden held for a claim's clone review: a golden held for a claim's clone Oct 7, 2026
@CMGS
CMGS merged commit ecf7f00 into main Oct 7, 2026
3 checks passed
@CMGS
CMGS deleted the review/whole-repo-1007 branch October 7, 2026 02:24
CMGS added a commit that referenced this pull request Oct 7, 2026
…ure (#336)

The hold #335 added took the manager mutex a second time to release and
allocated a closure per clone-tier claim. The count is now atomic and the
resolution carries the pool, so release is one atomic add: the increment
stays under the mutex with the golden read it pairs with, and a lock-free
decrement can only make a dropped pool settle later, never sooner.
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