kayg/calternal
Watch
1
0
Fork
You've already forked calternal
 0
Code Issues 190 Pull requests 61 Projects Releases Packages Wiki Activity Actions 26

Private upload staging and exactly-once crash recovery (#759b) #788

Open
kayg wants to merge 11 commits from fix/private-staging-759b-r2 into dev
pull from: fix/private-staging-759b-r2
merge into: kayg:dev
Conversation 13 Commits 11 Files changed 65 +7374 -289
kayg commented 2026-10-31 15:58:49 +00:00
Owner
Copy link

Round 2 correction complete: ef9a08d9

R2-P1 guard ownership and R2-P2 changed-device regression are fixed. The held record remains shared until durable result confirmation; existing Ready records govern restart. Exact reproduction, 32 await-boundary cancellation cases, focused tests, full workspace, separate 1,000-round kill loop, Clippy and fmt all passed on dev-01. FINAL independent review (round 3) is pending. No merge. Native macOS, the complete #771 matrix and the other documented review gaps remain open.

Latest status table, exact commands, red/green evidence and source-byte provenance: #788 (comment)

Round 2 correction design

Keep each unresolved upload owned by the shared record map throughout repair and engine resolution. Reconciliation borrows a bounded completion snapshot under the placement lock and removes the held record only after the engine confirms a durable result. Cancellation before that point leaves both the live content guard and the existing on-disk Ready record intact; restart continues to honour that record. No new record format, detached lifetime, or disk writes. Extra transient memory: one completion snapshot, at most 16 KiB; unchanged O(Kheld) scan and O(1) per-result bookkeeping.

Failure Effect / detection Recovery Gate
Request cancelled waiting for placement or repair/commit Shared held record and durable Ready remain; no absent-guard window Next retry repairs/replays original result; new keys cannot place these bytes twice Exact reviewer cancellation + failed-index reproduction, cancellation at reconciliation await boundaries
Cancel after durable result, during retirement Result already authoritative; leftover Ready pins receipt Restart replays and retires without a second event Cancellation coverage with retry and restart convergence
Changed device regression fault misses scoped receipt Test must fail if initial upload succeeds or no Ready exists Inject scoped key; require one Ready, zero events, exactly one mutated record Changed-device regression

Final gates requested for this correction: full workspace, separate 1,000-round SIGKILL loop, workspace all-target Clippy with warnings denied, formatting on dev-01. Native macOS and the complete #771 matrix remain open; the prior three-size measurements remain an early signal only. Never merge.

Implements #771 / #759b. Uploads remain private until exclusive publication; every placed file keeps its original result authority until the engine commits it exactly once. Fixes all three carried-forward P1s and the retry/cancellation gaps reproduced during this resume. Do not merge without independent review and the remaining platform/matrix gates.

Plan and costs

Stream N bytes in C paced writes: O(N) I/O/hash work, O(1) per-upload buffering and metadata; completion records are capped at 16 KiB, publication at 1,000 candidates. No per-chunk receipts, hashes, xattrs or added syncs. Publication retains payload fsync and both rename-parent fsyncs; index and engine durability remain intact. Recovery is O(pending records × bounded candidate lookups), plus scratch entries. Live retry reconciliation and unresolved-content guards scan the global held-result map O(Kheld), filtered to the user; large concurrent held-result populations remain unmeasured.

Measured plan adjustment: construction now writes Scratch atomically without fsync, matching Ready replacement. Both are transient metadata with a process-death guarantee, not a power-loss guarantee. The full record is written and closed, then renamed before publication. Scratch deletion authority comes from the payload inode proof, independently of the pending record. This removes two disposable-record flushes measured at about 8 ms/upload in the syscall diagnostic; no acknowledged file/result durability barrier was removed. Retirement remains unsynced.

Failure Effect Detection Recovery Test map
Receive/construction interrupted Incomplete private scratch Owner lock and inode provenance Reclaim only proven scratch fsio scratch construct/deaths/gate; attachments killing_an_upload_at_every_boundary_settles_exactly_once
Ready, publication refused, crash during discard Completion could outlive absent scratch Publication outcome and private payload Retract completion before discarding scratch a_refused_upload_killed_while_being_given_up_leaves_nothing_owed
Recovery cancelled Ready, killed before retirement Second restart could falsely owe a public file Ready plus private payload Atomically replace Ready with Scratch before unlink killing_recovery_while_cancelling_a_ready_upload_still_settles
Rename succeeds, parent flush fails Public file lacks a successful response/index entry Kept completion Resync destination, repair index, commit original result a_retry_after_a_failed_flush_indexes_the_file_so_new_keys_do_not_duplicate_it; a_retry_keeps_its_record_until_both_directory_sync_and_index_repair_succeed
Client replaces key or concurrent uploads race Unindexed public bytes might be saved again Held completion under placement lock Settle earlier uploads and guard unresolved content a_new_key_after_a_failed_flush_settles_the_original_before_deduplicating; concurrent_keys_for_the_same_bytes_keep_one_file_and_one_added_event
Engine commit uncertain/killed Result may already be committed Pinned original key and completion Resolve/replay once attachments recovery/death table; engine resolve tests; store pin tests; ordinary-server kill loop
Device number changes after restart Persisted dev no longer identifies the mount Inode/size within bounded destination candidates Ignore persisted dev (diagnostics only) a_changed_device_number_after_restart_still_finds_the_placed_file
Unknown/replaced/linked/adopted bytes Deletion unauthorized Identity/proof/journal checks Preserve whole; block ambiguity fsio scratch unknown/recovered and guarded-write tests
SIGKILL with fork-only descriptor holder Inherited owner descriptor survives POSIX owner locks and process-local registry New server acquires root locks owners tests; storage_locks; ordinary-server kill loop

Findings status

Finding Status
P1 persisted Completion.dev comparison Fixed; changed-device regression passes
P1 failed publish plus crash leaves Ready without bytes Fixed; attachments refusal/cancellation SIGKILL regressions pass
P1 post-rename flush retry commits without resync/index Fixed; repeated sync/index failures keep original authority, retries deduplicate
Additional cancellation recovery crash, replacement-key duplication, concurrent placement window Fixed with reproduced regressions and placement-lock registration
Index errors swallowed; engine/plugin ownership cycle Fixed; errors retain completion, weak back-references release owner on drop
P2 recovered intent traversal and successive same-name strangers Fixed; damaged intent is refused and distinct inodes get distinct preserved destinations
P2 duplicate pin-source registration silent; fault seam overhead Warning added; keyed fault injection has a fast inactive path
Native macOS ownership/xattr/inheritance proof Follow-up gate; not run
Full #771 cold/warm, split storage, 1/4 clients, 1k/10k/100k Homes, overlapping note saves, RSS/syscalls/metadata matrix Follow-up gate; requested three-size fixture is only an early signal
Multiple independent idempotency pin sources Follow-up; warning does not implement composition
Dev action-scoped receipt integration Fixed; scoped replay, legacy replay and aged-retention crash tests pass in focused run; full workspace and kill-loop reruns passed
Independent review of this final head Pending; no new delegate review was started by this implementation sub-agent

Dev advanced during verification. The branch merges e283b132 and resolves the two conflicts by keeping adapter staging under the raw request key while execution/reconciliation use the same action-scoped key. Pending completions pin both legacy raw and scoped receipts. Added resolving_a_scoped_committed_result_never_records_it_again and resolving_an_unscoped_legacy_receipt_replays_without_a_new_event. The unadapted integration reproduced double journal commits, expired pins and three missed keyed test faults; see the integration evidence comment.

Validation on dev-01

Nix tools, ~/bin/slot, cargo -j4; no builds/tests on control or perf machines. Final integrated head: ba86c6f6762b56ffeffae2decb7687ca91254930, including dev e283b132. All six manual gates passed. VM source matches the pushed head except required uncommitted Nix shell files; control worktree is clean.

Gate Final result
Focused packages 35 attachments + 94 fsio + 21 engine passed; one engine perf test ignored; doc tests passed
Release build Pass, 4m34s
Full workspace, through doc tests Exit 0; 1,284 passed, 20 ignored, zero failures across 90 summaries
Separate SIGKILL loop Exit 0; 1,000 rounds/1,000 uploads, seed 75931002, all settled exactly once; 75.52 s
Clippy, all workspace targets, warnings denied Exit 0
Formatting check Exit 0

Final kill distribution: 333 before any state / 561 in scratch / 22 placed-and-owing / 84 finished. Logs on dev-01: ~/resume759b-{focused,release,workspace,kill-loop,clippy,fmt}.log; orchestration log ~/resume759b-final-gates5.log. Prior pre-integration logs are retained in ~/resume759b-first-gates/ and ~/resume759b-unsynced-gates/.

cargo test --locked -p calternal-attachments -p calternal-fsio -p calternal-engine -j4
cargo build --release --locked -p calternal -j4
CALTERNAL_KILL_LOOP_SEED=75931001 cargo test --workspace --locked -j4 --no-fail-fast
CALTERNAL_KILL_LOOP_N=1000 CALTERNAL_KILL_LOOP_SEED=75931002 cargo test --locked -p calternal --test attachments_kill_loop -j4 -- --nocapture
cargo clippy --workspace --all-targets --locked -j4 -- -D warnings
cargo fmt --all --check

At 7f6a2928: focused tests 35 attachments + 89 fsio passed; full workspace completed through doc tests (1,238 passed, 20 ignored, zero failures across 89 summaries); separate 1,000-round SIGKILL loop passed in 68.76 s (330 before state / 540 scratch / 24 placed-and-owing / 106 finished), all settled exactly once. Initial Clippy caught an inherited disallowed test write; it now uses write_atomic. Red logs and corrected test setup are recorded in the regression-evidence comment. Initial benchmark below is retained.

Perf-01 comparison

Unchanged fixture from wip/10by32-01-impl-759b-scratch-20261030 @ 1e6e32d7, handover/759b/upload_bench.py; SHA256 2f5110702d483678de7a7e91bb09a15398675554d33947c767fa7c366b8856c6.
Unchanged PR #787 / perf/vm-setup harness perf/vm/upload-bench.sh; SHA256 c25c70c8678030218a458d2d037310f7bdc277b17bd7109b11988ebd7ab14956.
Fixed dev runtime baseline d83e54db (same binary as #759), SHA256 2ac46c102712f07402b2327f6b6a0eec66765fe67a1f7106f2d5900260eb66e8. This is a pinned comparison, not a claim to benchmark subsequent moving dev changes.

Each comparison uses 10 balanced alternating pairs (odd dev→head, even head→dev), one harness rep/version/pair, fresh fixture data, one continuous perf lock; ext4 /dev/sdb1. Report median paired head/dev throughput; unchanged target ≥95%. Guest latency is bimodal as already noted on #759. Binaries built on dev-01 and relayed via control with matching SHA256 at all three hops.

Size Initial head 7f6a2928 / dev Final head / dev Target
4 KiB 93.44% — fail 139.02% — pass ≥95%
4 MiB 90.66% — fail 119.75% — pass ≥95%
128 MiB 104.39% — pass 103.11% — pass ≥95%

Initial head binary SHA256 c4e7b813abbab228673ac90fa12e9530c7fc33f80a19f97075759856259c79f3. Initial raw output: perf-01 ~/perf/out-upload-759b-7f6a2928/; all 20 reps rc=0. After removing disposable Scratch flushes, source commit 41b9f382 passed all six gates (including full workspace and a 1,000-round kill loop: 342 before state / 543 scratch / 28 placed-and-owing / 87 finished, 71.61 s). The intermediate ten-pair benchmark reached 129.40% / 118.61% / 101.18%; raw ~/perf/out-upload-759b-unsynced/, binary SHA256 492972965ed982d48d1d7d13dc0ac07ee48d4bfff284defb94cf355d568e6670. These are intermediate results before the subsequent dev integration, not substituted for the final-head results below. Final binary SHA256 22a80c38ff9aac7a40afe036dc116a7a27659ebddc791d7d8beb45000e25d6a5 (dev-01 = control = perf-01). All 20 final reps rc=0. Raw output: perf-01 ~/perf/out-upload-759b-ba86c6f6/, including input hashes and alternating order. Per rep: 200 measured 4 KiB uploads, 40 × 4 MiB, 5 × 128 MiB, plus the fixture's unchanged warmups. The table above uses medians of paired ratios, which need not equal ratios of per-version medians below.

Size Dev p50 / p95 ms Head p50 / p95 ms Dev / head MiB/s Paired p95 head/dev
4KiB 40.140 / 49.216 28.215 / 35.902 0.09732 / 0.13848 0.7589
4MiB 87.476 / 100.825 72.231 / 81.975 45.72760 / 55.37826 0.8018
128MiB 1494.158 / 1521.568 1245.550 / 1494.867 85.68185 / 102.89375 0.9791

Decisions: retain existing r2 history; merge dev deployment tooling (a7dc49d6) and current dev receipt scoping (ba86c6f6) and retain offline lock-family guidance. Process-death recovery is the stated crash scope. Same-UID modifications inside private app-owned scratch remain outside scratch preservation; public/adopted outside bytes remain protected. No deployment, production access or merge performed.

## Round 2 correction complete: `ef9a08d9` R2-P1 guard ownership and R2-P2 changed-device regression are fixed. The held record remains shared until durable result confirmation; existing Ready records govern restart. Exact reproduction, 32 await-boundary cancellation cases, focused tests, full workspace, separate 1,000-round kill loop, Clippy and fmt all passed on dev-01. FINAL independent review (round 3) is pending. No merge. Native macOS, the complete #771 matrix and the other documented review gaps remain open. Latest status table, exact commands, red/green evidence and source-byte provenance: https://git.kayg.org/kayg/calternal/pulls/788#issuecomment-71096 ## Round 2 correction design Keep each unresolved upload owned by the shared record map throughout repair and engine resolution. Reconciliation borrows a bounded completion snapshot under the placement lock and removes the held record only after the engine confirms a durable result. Cancellation before that point leaves both the live content guard and the existing on-disk Ready record intact; restart continues to honour that record. No new record format, detached lifetime, or disk writes. Extra transient memory: one completion snapshot, at most 16 KiB; unchanged O(Kheld) scan and O(1) per-result bookkeeping. | Failure | Effect / detection | Recovery | Gate | |---|---|---|---| | Request cancelled waiting for placement or repair/commit | Shared held record and durable Ready remain; no absent-guard window | Next retry repairs/replays original result; new keys cannot place these bytes twice | Exact reviewer cancellation + failed-index reproduction, cancellation at reconciliation await boundaries | | Cancel after durable result, during retirement | Result already authoritative; leftover Ready pins receipt | Restart replays and retires without a second event | Cancellation coverage with retry and restart convergence | | Changed device regression fault misses scoped receipt | Test must fail if initial upload succeeds or no Ready exists | Inject scoped key; require one Ready, zero events, exactly one mutated record | Changed-device regression | Final gates requested for this correction: full workspace, separate 1,000-round SIGKILL loop, workspace all-target Clippy with warnings denied, formatting on dev-01. Native macOS and the complete #771 matrix remain open; the prior three-size measurements remain an early signal only. Never merge. Implements #771 / #759b. Uploads remain private until exclusive publication; every placed file keeps its original result authority until the engine commits it exactly once. Fixes all three carried-forward P1s and the retry/cancellation gaps reproduced during this resume. Do not merge without independent review and the remaining platform/matrix gates. ## Plan and costs Stream N bytes in C paced writes: O(N) I/O/hash work, O(1) per-upload buffering and metadata; completion records are capped at 16 KiB, publication at 1,000 candidates. No per-chunk receipts, hashes, xattrs or added syncs. Publication retains payload fsync and both rename-parent fsyncs; index and engine durability remain intact. Recovery is O(pending records × bounded candidate lookups), plus scratch entries. Live retry reconciliation and unresolved-content guards scan the global held-result map O(Kheld), filtered to the user; large concurrent held-result populations remain unmeasured. **Measured plan adjustment:** construction now writes Scratch atomically without fsync, matching Ready replacement. Both are transient metadata with a process-death guarantee, not a power-loss guarantee. The full record is written and closed, then renamed before publication. Scratch deletion authority comes from the payload inode proof, independently of the pending record. This removes two disposable-record flushes measured at about 8 ms/upload in the syscall diagnostic; no acknowledged file/result durability barrier was removed. Retirement remains unsynced. | Failure | Effect | Detection | Recovery | Test map | |---|---|---|---|---| | Receive/construction interrupted | Incomplete private scratch | Owner lock and inode provenance | Reclaim only proven scratch | fsio scratch construct/deaths/gate; attachments `killing_an_upload_at_every_boundary_settles_exactly_once` | | Ready, publication refused, crash during discard | Completion could outlive absent scratch | Publication outcome and private payload | Retract completion before discarding scratch | `a_refused_upload_killed_while_being_given_up_leaves_nothing_owed` | | Recovery cancelled Ready, killed before retirement | Second restart could falsely owe a public file | Ready plus private payload | Atomically replace Ready with Scratch before unlink | `killing_recovery_while_cancelling_a_ready_upload_still_settles` | | Rename succeeds, parent flush fails | Public file lacks a successful response/index entry | Kept completion | Resync destination, repair index, commit original result | `a_retry_after_a_failed_flush_indexes_the_file_so_new_keys_do_not_duplicate_it`; `a_retry_keeps_its_record_until_both_directory_sync_and_index_repair_succeed` | | Client replaces key or concurrent uploads race | Unindexed public bytes might be saved again | Held completion under placement lock | Settle earlier uploads and guard unresolved content | `a_new_key_after_a_failed_flush_settles_the_original_before_deduplicating`; `concurrent_keys_for_the_same_bytes_keep_one_file_and_one_added_event` | | Engine commit uncertain/killed | Result may already be committed | Pinned original key and completion | Resolve/replay once | attachments recovery/death table; engine resolve tests; store pin tests; ordinary-server kill loop | | Device number changes after restart | Persisted dev no longer identifies the mount | Inode/size within bounded destination candidates | Ignore persisted dev (diagnostics only) | `a_changed_device_number_after_restart_still_finds_the_placed_file` | | Unknown/replaced/linked/adopted bytes | Deletion unauthorized | Identity/proof/journal checks | Preserve whole; block ambiguity | fsio scratch unknown/recovered and guarded-write tests | | SIGKILL with fork-only descriptor holder | Inherited owner descriptor survives | POSIX owner locks and process-local registry | New server acquires root locks | owners tests; `storage_locks`; ordinary-server kill loop | ## Findings status | Finding | Status | |---|---| | P1 persisted `Completion.dev` comparison | Fixed; changed-device regression passes | | P1 failed publish plus crash leaves Ready without bytes | Fixed; attachments refusal/cancellation SIGKILL regressions pass | | P1 post-rename flush retry commits without resync/index | Fixed; repeated sync/index failures keep original authority, retries deduplicate | | Additional cancellation recovery crash, replacement-key duplication, concurrent placement window | Fixed with reproduced regressions and placement-lock registration | | Index errors swallowed; engine/plugin ownership cycle | Fixed; errors retain completion, weak back-references release owner on drop | | P2 recovered intent traversal and successive same-name strangers | Fixed; damaged intent is refused and distinct inodes get distinct preserved destinations | | P2 duplicate pin-source registration silent; fault seam overhead | Warning added; keyed fault injection has a fast inactive path | | Native macOS ownership/xattr/inheritance proof | Follow-up gate; not run | | Full #771 cold/warm, split storage, 1/4 clients, 1k/10k/100k Homes, overlapping note saves, RSS/syscalls/metadata matrix | Follow-up gate; requested three-size fixture is only an early signal | | Multiple independent idempotency pin sources | Follow-up; warning does not implement composition | | Dev action-scoped receipt integration | Fixed; scoped replay, legacy replay and aged-retention crash tests pass in focused run; full workspace and kill-loop reruns passed | | Independent review of this final head | Pending; no new delegate review was started by this implementation sub-agent | Dev advanced during verification. The branch merges `e283b132` and resolves the two conflicts by keeping adapter staging under the raw request key while execution/reconciliation use the same action-scoped key. Pending completions pin both legacy raw and scoped receipts. Added `resolving_a_scoped_committed_result_never_records_it_again` and `resolving_an_unscoped_legacy_receipt_replays_without_a_new_event`. The unadapted integration reproduced double journal commits, expired pins and three missed keyed test faults; see the integration evidence comment. ## Validation on dev-01 Nix tools, `~/bin/slot`, cargo `-j4`; no builds/tests on control or perf machines. Final integrated head: `ba86c6f6762b56ffeffae2decb7687ca91254930`, including dev `e283b132`. **All six manual gates passed.** VM source matches the pushed head except required uncommitted Nix shell files; control worktree is clean. | Gate | Final result | |---|---| | Focused packages | 35 attachments + 94 fsio + 21 engine passed; one engine perf test ignored; doc tests passed | | Release build | Pass, 4m34s | | Full workspace, through doc tests | Exit 0; 1,284 passed, 20 ignored, zero failures across 90 summaries | | Separate SIGKILL loop | Exit 0; 1,000 rounds/1,000 uploads, seed 75931002, all settled exactly once; 75.52 s | | Clippy, all workspace targets, warnings denied | Exit 0 | | Formatting check | Exit 0 | Final kill distribution: 333 before any state / 561 in scratch / 22 placed-and-owing / 84 finished. Logs on dev-01: `~/resume759b-{focused,release,workspace,kill-loop,clippy,fmt}.log`; orchestration log `~/resume759b-final-gates5.log`. Prior pre-integration logs are retained in `~/resume759b-first-gates/` and `~/resume759b-unsynced-gates/`. ```sh cargo test --locked -p calternal-attachments -p calternal-fsio -p calternal-engine -j4 cargo build --release --locked -p calternal -j4 CALTERNAL_KILL_LOOP_SEED=75931001 cargo test --workspace --locked -j4 --no-fail-fast CALTERNAL_KILL_LOOP_N=1000 CALTERNAL_KILL_LOOP_SEED=75931002 cargo test --locked -p calternal --test attachments_kill_loop -j4 -- --nocapture cargo clippy --workspace --all-targets --locked -j4 -- -D warnings cargo fmt --all --check ``` At `7f6a2928`: focused tests 35 attachments + 89 fsio passed; full workspace completed through doc tests (1,238 passed, 20 ignored, zero failures across 89 summaries); separate 1,000-round SIGKILL loop passed in 68.76 s (330 before state / 540 scratch / 24 placed-and-owing / 106 finished), all settled exactly once. Initial Clippy caught an inherited disallowed test write; it now uses `write_atomic`. Red logs and corrected test setup are recorded in the regression-evidence comment. Initial benchmark below is retained. ## Perf-01 comparison Unchanged fixture from `wip/10by32-01-impl-759b-scratch-20261030` @ `1e6e32d7`, `handover/759b/upload_bench.py`; SHA256 `2f5110702d483678de7a7e91bb09a15398675554d33947c767fa7c366b8856c6`. Unchanged PR #787 / `perf/vm-setup` harness `perf/vm/upload-bench.sh`; SHA256 `c25c70c8678030218a458d2d037310f7bdc277b17bd7109b11988ebd7ab14956`. Fixed dev runtime baseline `d83e54db` (same binary as #759), SHA256 `2ac46c102712f07402b2327f6b6a0eec66765fe67a1f7106f2d5900260eb66e8`. This is a pinned comparison, not a claim to benchmark subsequent moving dev changes. Each comparison uses 10 balanced alternating pairs (odd dev→head, even head→dev), one harness rep/version/pair, fresh fixture data, one continuous perf lock; ext4 `/dev/sdb1`. Report median paired head/dev throughput; unchanged target ≥95%. Guest latency is bimodal as already noted on #759. Binaries built on dev-01 and relayed via control with matching SHA256 at all three hops. | Size | Initial head `7f6a2928` / dev | Final head / dev | Target | |---|---:|---:|---:| | 4 KiB | 93.44% — fail | **139.02% — pass** | ≥95% | | 4 MiB | 90.66% — fail | **119.75% — pass** | ≥95% | | 128 MiB | 104.39% — pass | **103.11% — pass** | ≥95% | Initial head binary SHA256 `c4e7b813abbab228673ac90fa12e9530c7fc33f80a19f97075759856259c79f3`. Initial raw output: perf-01 `~/perf/out-upload-759b-7f6a2928/`; all 20 reps rc=0. After removing disposable Scratch flushes, source commit `41b9f382` passed all six gates (including full workspace and a 1,000-round kill loop: 342 before state / 543 scratch / 28 placed-and-owing / 87 finished, 71.61 s). The intermediate ten-pair benchmark reached 129.40% / 118.61% / 101.18%; raw `~/perf/out-upload-759b-unsynced/`, binary SHA256 `492972965ed982d48d1d7d13dc0ac07ee48d4bfff284defb94cf355d568e6670`. These are intermediate results before the subsequent dev integration, not substituted for the final-head results below. Final binary SHA256 `22a80c38ff9aac7a40afe036dc116a7a27659ebddc791d7d8beb45000e25d6a5` (dev-01 = control = perf-01). All 20 final reps rc=0. Raw output: perf-01 `~/perf/out-upload-759b-ba86c6f6/`, including input hashes and alternating order. Per rep: 200 measured 4 KiB uploads, 40 × 4 MiB, 5 × 128 MiB, plus the fixture's unchanged warmups. The table above uses medians of **paired ratios**, which need not equal ratios of per-version medians below. | Size | Dev p50 / p95 ms | Head p50 / p95 ms | Dev / head MiB/s | Paired p95 head/dev | |---|---:|---:|---:|---:| | 4KiB | 40.140 / 49.216 | 28.215 / 35.902 | 0.09732 / 0.13848 | 0.7589 | | 4MiB | 87.476 / 100.825 | 72.231 / 81.975 | 45.72760 / 55.37826 | 0.8018 | | 128MiB | 1494.158 / 1521.568 | 1245.550 / 1494.867 | 85.68185 / 102.89375 | 0.9791 | Decisions: retain existing r2 history; merge dev deployment tooling (`a7dc49d6`) and current dev receipt scoping (`ba86c6f6`) and retain offline lock-family guidance. Process-death recovery is the stated crash scope. Same-UID modifications inside private app-owned scratch remain outside scratch preservation; public/adopted outside bytes remain protected. No deployment, production access or merge performed.
kayg added 11 commits 2026-10-31 15:58:49 +00:00 
Handoff (paused at kayg's request; no PR opened, nothing merged or deployed).

Done (plan f8b1db01, issue #771):
- fsio::owners: process-wide POSIX F_SETLK registry keyed by dev/inode,
  local shared/exclusive gate, fork refusal, non-blocking flock probe.
  DataLock now uses it; startup refuses while an old flock holds a root.
- fsio::scratch: private .calternal/tmp operations, provenance xattr,
  exclusive-rename publish with destination inheritance (group/setgid/
  default ACL, recheck, refuse on mismatch), two parent syncs, recovery,
  recovered/ with replayable intent, read-only listing.
- fsio::stops: test seam (boundaries, thread hooks, injected faults).
- PreparedDirectory root gate; usage scan skips scratch.
- store: idempotency pins (first prune included); engine: Engine::resolve,
  pin_idempotency; server: /healthz degraded registry.
- attachments: completion records, recovery/reconciliation before a
  person's uploads, retire after durable commit, `calternal storage
  recovered` diagnostic; docs/storage.md section.
- Tests: owners (13), scratch (death at every boundary, gate, inherit,
  unknown/recovered, ENOSPC/EACCES), store pins, engine resolve,
  attachments deaths/recovery, storage_locks (fork-child restart, flock
  holder blocks startup, flock negative control), 1000-round SIGKILL loop.
- Adapted (not weakened) attachments_crash and reliability_round3 to look
  at scratch / ignore the persistent .root lock file.

Gates: fmt OK; clippy --workspace --all-targets -D warnings OK; web lint,
check, editor+ui unit OK (bun). Full `cargo test --workspace --locked`:
interrupted by this pause inside calternal-notes; every crate before it
(attachments, auth, backdrops, calternal, contacts, ...) passed with zero
failures; crates after notes (perf, plugin-hello, prefs, search, server,
store, tasks, testkit) not run in the final pass (they passed earlier in
targeted runs: server, store, fsio, engine, attachments).

Next:
1. Re-run `cargo test --workspace --locked` verbatim to completion.
2. Local release 4KiB/4MiB/128MiB upload numbers vs dev: baseline worktree
   /home/cua/wt/base-759b (detached origin/dev), client /tmp/impl759b/
   upload_bench.py (may be lost); build both with --release via build-slot.
3. Put the deploy boundary into docs/deploy.md once #773 lands (the file
   is not on dev; boundary is documented in docs/storage.md meanwhile).
4. Open the Forgejo PR -> dev "Private staging redesign (#759b)" with the
   failure-table -> test mapping; list native macOS tests as pending
   (owner drop, duplicates, fork-only child alive during SIGKILL, real
   restart acquiring Home/state locks, shared guards, alias close, xattr
   name, inheritance, PreparedDirectory decline). macOS not compiled here.
5. Delete target/ at the end; skip the perf VM run.
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01Xi6YxsuZ7m2ESGC7aS91AN
Merge dev deployment tooling and retain offline lock upgrade guidance
Some checks failed
CI / server (pull_request) Has been cancelled
Details
CI / rust (pull_request) Has been cancelled
Details
CI / web (pull_request) Has been cancelled
Details
a7dc49d6a3
kayg commented 2026-10-31 15:59:09 +00:00
Author
Owner
Copy link

Round-1 independent implementation review, carried forward from the resume brief. The full original review text was not found on an existing implementation PR (this branch had none) or in the #771/#759 comments; this is a summary of its three P1s, not a new independent review.

  1. P1 — device number persisted across restart: recovery.rs:437 compared Completion.dev. Device numbers may change across reboots on btrfs/ZFS/NFS/overlay, so a placed upload could become unresolvable. Locate within the recorded destination by inode/size without trusting persisted dev.
  2. P1 — failed publication then crash: place.rs:190 / publish.rs:62 could delete scratch while leaving a Ready record and no public file. Restart could not distinguish this from a user moving a published file. Retract/mark cancellation before scratch removal, and cover it at the attachments layer with SIGKILL tests.
  3. P1 — retry after post-rename flush failure: reconcile_key at recovery.rs:362 committed the result without re-syncing the destination or updating the index. A later same-bytes upload could make a second copy. Repeat durability/index work before commit, and test same-key retry followed by a new key.

These are fix-first findings. Status and regression evidence are tracked in the PR description. Cheap additional fixes already on r2 include recovered-intent component validation, distinct recovery destinations for successive same-name strangers, and the idempotency-pin registration diagnostic. Remaining gaps will be listed as follow-ups rather than represented as verified.

Round-1 independent implementation review, carried forward from the resume brief. The full original review text was not found on an existing implementation PR (this branch had none) or in the #771/#759 comments; this is a summary of its three P1s, not a new independent review. 1. **P1 — device number persisted across restart:** `recovery.rs:437` compared `Completion.dev`. Device numbers may change across reboots on btrfs/ZFS/NFS/overlay, so a placed upload could become unresolvable. Locate within the recorded destination by inode/size without trusting persisted dev. 2. **P1 — failed publication then crash:** `place.rs:190` / `publish.rs:62` could delete scratch while leaving a Ready record and no public file. Restart could not distinguish this from a user moving a published file. Retract/mark cancellation before scratch removal, and cover it at the attachments layer with SIGKILL tests. 3. **P1 — retry after post-rename flush failure:** `reconcile_key` at `recovery.rs:362` committed the result without re-syncing the destination or updating the index. A later same-bytes upload could make a second copy. Repeat durability/index work before commit, and test same-key retry followed by a new key. These are fix-first findings. Status and regression evidence are tracked in the PR description. Cheap additional fixes already on r2 include recovered-intent component validation, distinct recovery destinations for successive same-name strangers, and the idempotency-pin registration diagnostic. Remaining gaps will be listed as follow-ups rather than represented as verified.
kayg commented 2026-10-31 16:24:33 +00:00
Author
Owner
Copy link

Regression evidence for implementation head 7f6a2928f02921dfbfea6297993927e6572cb729 (dev-01, Nix tools, build-slot wrapper, cargo -j4).

The inherited red-state logs were preserved before restoring the pushed fixes. r2-before.log shows all three original P1 regressions failing: changed device number becomes a missing placed file; retry after failed flush saves flush-eio 2.jpg; failed publication plus crash leaves the person paused. r2-before2.log shows both additional fsio regressions failing without r2's intent-name validation and per-inode stranger destination fixes.

This resume also reproduced:

  • resume759b-cancel-red.log: a second kill during cancellation recovery leaves Ready with no payload/public file. Recovery now atomically downgrades never-published Ready to Scratch before unlinking payload.
  • resume759b-newkey-red.log: a replacement key immediately after the failed flush saves new-key-after-flush 2.jpg; dropping the old server also fails to release its attachment owner because of the engine/plugin Arc cycle. Reconciliation now settles earlier placements before any upload, tracks public results under the placement lock, and uses weak engine/pin back-references.
  • Repeated directory-sync/index failures now keep the record and original result unresolved. Index open/write errors propagate; they cannot silently retire the completion.

Fresh corrected focused command:

cargo test --locked -p calternal-attachments -p calternal-fsio -j4

attachments: 35 passed; 0 failed; 0 ignored
fsio:        89 passed; 0 failed; 0 ignored
doc-tests:    0 failed

Test setup corrections are disclosed: the new concurrent-upload check initially exceeded the existing four-upload per-person admission limit (eight requests produced the expected 429); it now tests four admitted concurrent clients. The strict publication-order expectation now includes the new publish.flush fault boundary between rename and after-sync. Neither product admission nor durability assertions were relaxed. The original failed logs remain preserved.

Full workspace, separate 1,000-round SIGKILL rerun, release build and alternating perf-01 comparison are still running/pending; this comment does not claim those gates passed.

Regression evidence for implementation head `7f6a2928f02921dfbfea6297993927e6572cb729` (dev-01, Nix tools, build-slot wrapper, cargo `-j4`). The inherited red-state logs were preserved before restoring the pushed fixes. `r2-before.log` shows all three original P1 regressions failing: changed device number becomes a missing placed file; retry after failed flush saves `flush-eio 2.jpg`; failed publication plus crash leaves the person paused. `r2-before2.log` shows both additional fsio regressions failing without r2's intent-name validation and per-inode stranger destination fixes. This resume also reproduced: - `resume759b-cancel-red.log`: a second kill during cancellation recovery leaves Ready with no payload/public file. Recovery now atomically downgrades never-published Ready to Scratch before unlinking payload. - `resume759b-newkey-red.log`: a replacement key immediately after the failed flush saves `new-key-after-flush 2.jpg`; dropping the old server also fails to release its attachment owner because of the engine/plugin Arc cycle. Reconciliation now settles earlier placements before any upload, tracks public results under the placement lock, and uses weak engine/pin back-references. - Repeated directory-sync/index failures now keep the record and original result unresolved. Index open/write errors propagate; they cannot silently retire the completion. Fresh corrected focused command: ```sh cargo test --locked -p calternal-attachments -p calternal-fsio -j4 ``` ```text attachments: 35 passed; 0 failed; 0 ignored fsio: 89 passed; 0 failed; 0 ignored doc-tests: 0 failed ``` Test setup corrections are disclosed: the new concurrent-upload check initially exceeded the existing four-upload per-person admission limit (eight requests produced the expected 429); it now tests four admitted concurrent clients. The strict publication-order expectation now includes the new `publish.flush` fault boundary between rename and after-sync. Neither product admission nor durability assertions were relaxed. The original failed logs remain preserved. Full workspace, separate 1,000-round SIGKILL rerun, release build and alternating perf-01 comparison are still running/pending; this comment does not claim those gates passed.
kayg commented 2026-10-31 16:50:46 +00:00
Author
Owner
Copy link

Initial perf gate for 7f6a2928 missed the ≥95% target: median paired throughput was 93.44% of dev for 4 KiB, 90.66% for 4 MiB, and 104.39% for 128 MiB. Ten balanced alternating pairs on perf-01, unchanged #759 fixture and #787 harness, all 20 reps rc=0. Raw evidence: ~/perf/out-upload-759b-7f6a2928/. Baseline is pinned dev runtime d83e54db.

A separate syscall diagnostic (not substituted for the budget run) found two durable pending-record flushes costing about 8 ms/upload. The concrete plan was updated in this PR before changing Scratch construction to the existing atomic unsynced record helper. Ready already uses that helper. Process death leaves a fully installed old/new record; inode provenance independently authorizes scratch cleanup. The payload, publication directories, index and engine retain their durability barriers. This narrows the metadata work within the existing process-death claim; it does not claim power-loss recovery of transient records.

Focused 35 attachments + 89 fsio tests and the release build passed after that change. Full workspace, separate 1,000-round SIGKILL rerun, Clippy and a fresh ten-pair perf comparison are running. The original failed perf results remain in the PR description regardless of rerun outcome.

Initial perf gate for `7f6a2928` **missed the ≥95% target**: median paired throughput was 93.44% of dev for 4 KiB, 90.66% for 4 MiB, and 104.39% for 128 MiB. Ten balanced alternating pairs on perf-01, unchanged #759 fixture and #787 harness, all 20 reps rc=0. Raw evidence: `~/perf/out-upload-759b-7f6a2928/`. Baseline is pinned dev runtime `d83e54db`. A separate syscall diagnostic (not substituted for the budget run) found two durable pending-record flushes costing about 8 ms/upload. The concrete plan was updated in this PR before changing Scratch construction to the existing atomic unsynced record helper. Ready already uses that helper. Process death leaves a fully installed old/new record; inode provenance independently authorizes scratch cleanup. The payload, publication directories, index and engine retain their durability barriers. This narrows the metadata work within the existing process-death claim; it does not claim power-loss recovery of transient records. Focused 35 attachments + 89 fsio tests and the release build passed after that change. Full workspace, separate 1,000-round SIGKILL rerun, Clippy and a fresh ten-pair perf comparison are running. The original failed perf results remain in the PR description regardless of rerun outcome.
kayg commented 2026-10-31 17:02:30 +00:00
Author
Owner
Copy link

Dev-integration regression evidence at final integrated source ba86c6f6762b56ffeffae2decb7687ca91254930.

While gates were running, dev advanced to e283b132 with action-scoped idempotency receipts. A mechanical merge was unsafe: recovery's raw key missed a normally committed scoped receipt and recorded a second event; raw pins no longer protected scoped receipts past retention. resume759b-integration-red.log reproduced failures in killing_an_upload_at_every_boundary_settles_exactly_once (recovery committed twice) and a_pinned_key_is_still_there_after_the_engine_restarts_past_retention. Three attachment recovery tests also exposed that their keyed commit fault injections needed the new internal scoped key: an_uncertain_commit_keeps_the_file_the_record_and_the_retry_gate_until_it_resolves, the_same_key_with_other_content_conflicts_instead_of_committing_the_wrong_thing, and a_placed_file_that_cannot_be_found_blocks_only_that_person_and_says_why.

The added resolving_a_scoped_committed_result_never_records_it_again was run red independently (resume759b-scoped-red.log, assertion: recovery finds the normal action's receipt). Resolution now shares execute's scoped claim/storage key and legacy raw-receipt fallback. Attachments correlate adapter staging by the original public key and pin both receipt formats. Test fault injections were moved to the actual scoped store key; original assertions remain intact. Added legacy receipt replay coverage too.

Focused integration command cargo test --locked -p calternal-engine -p calternal-attachments -p calternal-fsio -j4 --no-fail-fast passed (35 attachments, 94 fsio, 21 engine; one engine perf-only test ignored). Final source is pushed; full workspace, 1,000-round kill loop, Clippy, formatting, release and alternating perf reruns are pending. No claim of final acceptance yet.

Dev-integration regression evidence at final integrated source `ba86c6f6762b56ffeffae2decb7687ca91254930`. While gates were running, dev advanced to `e283b132` with action-scoped idempotency receipts. A mechanical merge was unsafe: recovery's raw key missed a normally committed scoped receipt and recorded a second event; raw pins no longer protected scoped receipts past retention. `resume759b-integration-red.log` reproduced failures in `killing_an_upload_at_every_boundary_settles_exactly_once` (recovery committed twice) and `a_pinned_key_is_still_there_after_the_engine_restarts_past_retention`. Three attachment recovery tests also exposed that their keyed commit fault injections needed the new internal scoped key: `an_uncertain_commit_keeps_the_file_the_record_and_the_retry_gate_until_it_resolves`, `the_same_key_with_other_content_conflicts_instead_of_committing_the_wrong_thing`, and `a_placed_file_that_cannot_be_found_blocks_only_that_person_and_says_why`. The added `resolving_a_scoped_committed_result_never_records_it_again` was run red independently (`resume759b-scoped-red.log`, assertion: recovery finds the normal action's receipt). Resolution now shares execute's scoped claim/storage key and legacy raw-receipt fallback. Attachments correlate adapter staging by the original public key and pin both receipt formats. Test fault injections were moved to the actual scoped store key; original assertions remain intact. Added legacy receipt replay coverage too. Focused integration command `cargo test --locked -p calternal-engine -p calternal-attachments -p calternal-fsio -j4 --no-fail-fast` passed (35 attachments, 94 fsio, 21 engine; one engine perf-only test ignored). Final source is pushed; full workspace, 1,000-round kill loop, Clippy, formatting, release and alternating perf reruns are pending. No claim of final acceptance yet.
kayg changed title from WIP: Private upload staging and exactly-once crash recovery (#759b) to Private upload staging and exactly-once crash recovery (#759b) 2026-10-31 17:18:30 +00:00
kayg commented 2026-10-31 17:18:48 +00:00
Author
Owner
Copy link

Final verification for ba86c6f6762b56ffeffae2decb7687ca91254930 is complete. All six manual dev-01 gates passed, including the full workspace through doc tests (1,284 passed / 20 ignored / 0 failed) and a separate seeded 1,000-round SIGKILL loop (1,000 uploads all settled exactly once; 333 before state / 561 scratch / 22 placed-and-owing / 84 finished; 75.52 s). Focused tests: 35 attachments, 94 fsio, 21 engine passed, one engine perf test ignored. Release build, workspace Clippy with warnings denied, and formatting passed. Exact commands are in the PR description.

Final ten-pair alternating perf-01 comparison against fixed dev runtime d83e54db passed the unchanged ≥95% throughput target at every size: 4 KiB 139.02%, 4 MiB 119.75%, 128 MiB 103.11% (median paired head/dev ratios). All 20 reps rc=0. Exact unchanged fixture/harness, hashes, medians, paired p95 ratios, initial failed measurements and intermediate measurements are retained in the description. Final binary SHA256: 22a80c38ff9aac7a40afe036dc116a7a27659ebddc791d7d8beb45000e25d6a5; verified at dev-01, control and perf-01. Raw final output: perf-01 ~/perf/out-upload-759b-ba86c6f6/.

The branch includes dev e283b132, with scoped/legacy idempotency reconciliation and pins covered. Control worktree is clean; VM source is the same head except uncommitted required Nix shell files. The first PR comment remains the original round-1 review summary. Independent review of this final head, native macOS proof, full #771 matrix, held-result-map scaling and multi-source pin composition remain explicitly open. No merge or deployment performed.

Final verification for `ba86c6f6762b56ffeffae2decb7687ca91254930` is complete. All six manual dev-01 gates passed, including the full workspace through doc tests (1,284 passed / 20 ignored / 0 failed) and a separate seeded 1,000-round SIGKILL loop (1,000 uploads all settled exactly once; 333 before state / 561 scratch / 22 placed-and-owing / 84 finished; 75.52 s). Focused tests: 35 attachments, 94 fsio, 21 engine passed, one engine perf test ignored. Release build, workspace Clippy with warnings denied, and formatting passed. Exact commands are in the PR description. Final ten-pair alternating perf-01 comparison against fixed dev runtime `d83e54db` passed the unchanged ≥95% throughput target at every size: 4 KiB **139.02%**, 4 MiB **119.75%**, 128 MiB **103.11%** (median paired head/dev ratios). All 20 reps rc=0. Exact unchanged fixture/harness, hashes, medians, paired p95 ratios, initial failed measurements and intermediate measurements are retained in the description. Final binary SHA256: `22a80c38ff9aac7a40afe036dc116a7a27659ebddc791d7d8beb45000e25d6a5`; verified at dev-01, control and perf-01. Raw final output: perf-01 `~/perf/out-upload-759b-ba86c6f6/`. The branch includes dev `e283b132`, with scoped/legacy idempotency reconciliation and pins covered. Control worktree is clean; VM source is the same head except uncommitted required Nix shell files. The first PR comment remains the original round-1 review summary. Independent review of this final head, native macOS proof, full #771 matrix, held-result-map scaling and multi-source pin composition remain explicitly open. No merge or deployment performed.
kayg commented 2026-10-31 17:28:47 +00:00
Author
Owner
Copy link

Independent review, round 2

verdict: fix first at ba86c6f6762b56ffeffae2decb7687ca91254930. no new P0; one newly reproduced P1 (duplicate save after cancellation) and one new P2 test regression. the normal retry fixes work, but the retry's own cancellation is unsafe. native macOS and the full #771 matrix also remain acceptance gaps.

reviewed the PR body/comments, approved plan including pinning/offline upgrade, original round-1 report recovered from its saved agent transcript, and implementation. no product changes or pushes. control host used only for reading/orchestration; tests ran on dev-01 in an isolated reviewer worktree.

round-1 status

IDs below refer to the original report's numbered findings, including the P2/P3 items omitted from the first PR comment. “verified” means the stated fix is present; test limitations are explicit.

finding status at reviewed head does the evidence prove it?
P1-1 persisted device comparison verified code fix; regressed test locate uses inode/size and regular-file type. the shipped changed-device test is vacuous after receipt scoping; see R2-P2 below. correcting its fault key and requiring a pending completion makes the test pass.
P1-2 failed publish, crash with Ready but no file verified publish_with retracts before discarding; refusal SIGKILL tests now include the attachments record. recovery also atomically downgrades Ready before deleting never-published payload. both refusal and second-recovery-crash tests pass.
P1-3 retry omits destination sync/index verified for completed retries same-key/new-key and repeated sync/index failure tests pass and assert one file/event plus retained authority. cancellation during this new repair path is missing, R2-P1 below.
P2-1 unreadable record disables pruning not fixed; intentionally fail closed keep-all remains. warning and per-user recovery degradation exist; the original claim of no diagnostics was too broad. never expire possibly-needed commit evidence merely to bound this case.
P2-2 unsynced Ready under power loss not fixed; documented scope exclusion Scratch is now unsynced too. approved claim is process death, so this is not a new process-death defect. SIGKILL tests cannot prove power-loss ordering.
P2-3 successive same-name strangers collide verified for distinct inodes ID now includes inode. a_second_stranger_with_the_same_name_is_kept_beside_the_first asserts separate recovered destinations and passes. not a proof against returning the same inode to the same source.
P2-4 admin tidying recovered/ blocks uploads not fixed non-directory entries and payload folders without intent still become blocked. no new test proves tolerance.
P2-5 recovered-intent traversal verified operation names/components validated; Home sources pass through open_sub/plain_parts. forged ../payload regression preserves source and reports damaged intent; passes. test specifically covers entry traversal, not every source variant.
P2-6 provenance xattr remains public not fixed no removal at publication. harmless to deletion authority under approved scratch boundary; no test/documented public-metadata disposition added.
P2-7 capability preflight not fixed no startup xattr/exclusive-rename destination probe; incompatible filesystems/nested mounts still fail on upload. safe refusal, no preflight proof.
P2-8 labels/large xattrs/Apple ACL inheritance not fixed label differences refuse safely, buffers remain 4096 bytes, non-Linux ACL policy is still None. Linux ACL tests do not prove Darwin inheritance.
P2-9 registry close race/global syscall mutex not fixed owners.rs:224–239 removes registry entry while self.entry still owns the old descriptors; automatic field drop happens after the registry guard is gone. open_in still performs stat/open under global mutex. existing lock tests do not force the final-drop/reopen interval.
P2-10 split Home/state attachment recovery not fixed attachment Env/death table and ordinary kill loop still use a common root. storage lock tests with two roots do not prove attachment paths/pins. aging helpers still silently skip absent DBs.
P2-11 actual-server inherited-FD recovery/macOS not fixed lock helper proves inherited descriptors; kill-loop strays independently reopen locks. neither is the requested combined real-server case. Darwin flock assertion and native execution remain unchecked.
P2-12 first-PR gates/adoption mapping not fixed in full deploy documentation and streaming perf scenario now exist. three-size early signal is measured, but required matrix/adoption-row mapping/native macOS evidence remain absent.
P2-13 stuck-state retry rescans not fixed every attempt with clean == false still repeats full recovery; no backoff or retry-load evidence.
P2-14 invalid owner skips liveness lock not fixed recover_entry still takes the owner lock only for a passing judge_owner; no new live-invalid-owner test. same-UID interference inside scratch is excluded by the approved boundary, which limits its relevance.
round-1 P3 status evidence/limit
1 destructured drop-order comment not fixed comment/order discrepancy remains; inode proof is independent of pending metadata.
2 inheritance after payload sync not fixed chown/ACL changes still follow payload sync_all. no durability-order regression test.
3 non-UTF-8 recovery names not fixed lossy string enumeration remains.
4 pin registration diagnostic not fixed in full duplicate registration now warns, but runtimes opened before registration still retain pins=None; warning does not detect/repair that case or compose sources.
5 production fault-seam mutex verified mitigation inactive atomic fast path avoids the mutex; fsio dependency/hooks remain.
6 idle pending temporary cleanup not fixed idle return still bypasses tidy_pending.
7 permanent Scratch cache/fds not fixed no eviction/revalidation.
8 kill-loop coverage/salt verified ≥500 rounds assert placed-and-owing coverage; salt is u16. reported final 1000-round run has 22 owed placements.
9 unsupported/leaked old flock not fixed in full leaked-child upgrade limitation is documented; unsupported flock still fails closed.
10 offline rollback verified current deploy script stops before forward backup/start and before rollback restart; docs describe both directions. not a live-upgrade fence, as approved.
11 shutdown blocking recovery not fixed aborting async task still does not cancel spawn_blocking/root wait.
12 all Home walkers exclude private files not verified/fixed serving and scratch usage guards exist; no exhaustive walker/lock-descriptor audit evidence added.

new findings

R2-P1: cancellation drops the held-result guard and permits a second save

crates/attachments/src/recovery.rs:390 and :397; caller crates/attachments/src/http.rs:152. reconcile_key removes Kept from the map before awaiting repair. HTTP reconciliation runs before the detached task at http.rs:180. if that request future is cancelled during repair, kept drops without being reinserted, the placement guard releases, and the user's cached recovery state stays clean. the Ready record survives on disk, but subsequent live requests consult neither it nor the lost held-content guard.

reproduced on dev-01: inject publish.flush EIO so cancel.jpg is public and unindexed; begin a retry; pause at resolve.sync; cancel the route future; release the blocking repair with resolve.index EIO; upload the same bytes with a new key. response is 201, already=false, path cancel 2.jpg; both public files exist and the original completion remains. no restart is needed. the ordinary server kill loop does not test cancellation of a reconciliation future while the process stays alive.

fix: make reconciliation cancellation-safe. retain authority in the shared map through every await (with a resolving state), or use a drop guard that reinserts on cancellation, or run the whole reconciliation with its placement guard in a detached task. neither another key nor has_kept_content may observe the obligation as absent until it is settled. add this exact cancellation + failed-index regression, including retry/restart convergence.

R2-P2: changed-device regression never changes any completion

crates/attachments/src/tests/recovery.rs:201–216. the injected store.before-commit key is raw k-dev, but the integrated store receives action_key("attachments.add", "k-dev"). the first upload succeeds and retires its record; the loop changing dev executes zero times. the assertions then replay an ordinary completed upload. this is a test regression introduced by receipt-scoping integration, despite the product fix being correct.

reproduced: adding only assert_eq!(e.markers(user), 1) immediately after the first upload fails 0 vs 1. changing the fault to the scoped key makes that assertion and the full changed-device scenario pass.

fix: inject the scoped key, assert initial response failure, exactly one Ready record and zero committed events before restart; assert the record mutation count is one. keep raw-key behavior in the separate legacy-receipt test.

benchmark audit

read raw final perf-01 ~/perf/out-upload-759b-ba86c6f6/, order.txt, input hashes, all 20 JSON/return-code files, fixture and harness. independently recomputed median paired head/dev ratios: 139.017948% / 119.746720% / 103.110232%, matching the PR for 4 KiB / 4 MiB / 128 MiB. all reps rc=0; ten balanced alternating pairs (odd dev→head, even head→dev); same harness and fresh fixtures. recorded hashes match the PR's head/baseline/fixture/harness hashes. the three-size comparison is fair against the explicitly pinned dev d83e54db, not a measurement against later dev. median paired ratios are correctly distinguished from ratios of per-version medians.

passes the ≥95% threshold for those three early-signal measurements only. does not meet the complete #771 first-PR gate. this fixture reports bytes divided by median request latency, not aggregate workload throughput, uses one client/common-root storage, and measures 200/40/5 uploads. it does not provide the required ≥60 s and ≥50 uploads/≥50 overlapping saves per point, cold/warm, split storage, 1/4 clients, 1k/10k/100k Homes, through 1 GiB, RSS/syscalls/metadata matrix. the new Rust harness is not the harness used for these measurements. initial failures are properly retained rather than erased. no benchmark rerun was needed for this audit.

independent checks and boundaries

on dev-01, reviewer worktree ~/Developer/Worktrees/review-788-r2, Nix tools and ~/bin/slot, -j4:

  • new cancellation regression: fails, two files versus one (~/review788-cancellation.log).
  • original device test plus pending precondition only: fails, zero pending versus one (~/review788-device-red.log).
  • scoped fault + pending precondition, then focused packages: 35 attachments, 94 fsio, 21 engine passed, one engine performance test ignored, doc tests pass (~/review788-focused.log). the known failing cancellation test was explicitly skipped in this positive run.
cargo test --locked -p calternal-attachments -j4 cancelled_reconciliation_keeps_the_duplicate_guard -- --nocapture
cargo test --locked -p calternal-attachments -j4 a_changed_device_number -- --nocapture
cargo test --locked -p calternal-attachments -p calternal-fsio -p calternal-engine -j4 -- --skip cancelled_reconciliation_keeps_the_duplicate_guard

reviewer test/precondition changes are uncommitted only in that isolated VM worktree; product source is unchanged. no full-workspace or native macOS rerun claimed. process-death ordering of unsynced atomic records is consistent with the approved scope; it is not a power-loss guarantee. normal publication syncs both rename parents, but retry repair still only resyncs the destination, so it does not independently demonstrate recovery from a failed source-parent fsync. pinning now covers scoped and legacy receipts, and aged SIGKILL cases pass; that proof remains conditional on registering the pin source before any runtime opens.

fix R2-P1 and R2-P2 before round 3. close or explicitly resolve the remaining plan acceptance gaps; do not label the three-size early signal as the full performance gate.

# Independent review, round 2 **verdict: fix first** at `ba86c6f6762b56ffeffae2decb7687ca91254930`. no new P0; one newly reproduced P1 (duplicate save after cancellation) and one new P2 test regression. the normal retry fixes work, but the retry's own cancellation is unsafe. native macOS and the full #771 matrix also remain acceptance gaps. reviewed the PR body/comments, approved plan including pinning/offline upgrade, original round-1 report recovered from its saved agent transcript, and implementation. no product changes or pushes. control host used only for reading/orchestration; tests ran on dev-01 in an isolated reviewer worktree. ## round-1 status IDs below refer to the original report's numbered findings, including the P2/P3 items omitted from the first PR comment. “verified” means the stated fix is present; test limitations are explicit. | finding | status at reviewed head | does the evidence prove it? | |---|---|---| | P1-1 persisted device comparison | **verified code fix; regressed test** | `locate` uses inode/size and regular-file type. the shipped changed-device test is vacuous after receipt scoping; see R2-P2 below. correcting its fault key and requiring a pending completion makes the test pass. | | P1-2 failed publish, crash with Ready but no file | **verified** | `publish_with` retracts before discarding; refusal SIGKILL tests now include the attachments record. recovery also atomically downgrades Ready before deleting never-published payload. both refusal and second-recovery-crash tests pass. | | P1-3 retry omits destination sync/index | **verified for completed retries** | same-key/new-key and repeated sync/index failure tests pass and assert one file/event plus retained authority. cancellation during this new repair path is missing, R2-P1 below. | | P2-1 unreadable record disables pruning | **not fixed; intentionally fail closed** | keep-all remains. warning and per-user recovery degradation exist; the original claim of no diagnostics was too broad. never expire possibly-needed commit evidence merely to bound this case. | | P2-2 unsynced Ready under power loss | **not fixed; documented scope exclusion** | Scratch is now unsynced too. approved claim is process death, so this is not a new process-death defect. SIGKILL tests cannot prove power-loss ordering. | | P2-3 successive same-name strangers collide | **verified for distinct inodes** | ID now includes inode. `a_second_stranger_with_the_same_name_is_kept_beside_the_first` asserts separate recovered destinations and passes. not a proof against returning the same inode to the same source. | | P2-4 admin tidying recovered/ blocks uploads | **not fixed** | non-directory entries and payload folders without intent still become blocked. no new test proves tolerance. | | P2-5 recovered-intent traversal | **verified** | operation names/components validated; Home sources pass through `open_sub`/`plain_parts`. forged `../payload` regression preserves source and reports damaged intent; passes. test specifically covers entry traversal, not every source variant. | | P2-6 provenance xattr remains public | **not fixed** | no removal at publication. harmless to deletion authority under approved scratch boundary; no test/documented public-metadata disposition added. | | P2-7 capability preflight | **not fixed** | no startup xattr/exclusive-rename destination probe; incompatible filesystems/nested mounts still fail on upload. safe refusal, no preflight proof. | | P2-8 labels/large xattrs/Apple ACL inheritance | **not fixed** | label differences refuse safely, buffers remain 4096 bytes, non-Linux ACL policy is still `None`. Linux ACL tests do not prove Darwin inheritance. | | P2-9 registry close race/global syscall mutex | **not fixed** | `owners.rs:224–239` removes registry entry while `self.entry` still owns the old descriptors; automatic field drop happens after the registry guard is gone. `open_in` still performs stat/open under global mutex. existing lock tests do not force the final-drop/reopen interval. | | P2-10 split Home/state attachment recovery | **not fixed** | attachment Env/death table and ordinary kill loop still use a common root. storage lock tests with two roots do not prove attachment paths/pins. aging helpers still silently skip absent DBs. | | P2-11 actual-server inherited-FD recovery/macOS | **not fixed** | lock helper proves inherited descriptors; kill-loop strays independently reopen locks. neither is the requested combined real-server case. Darwin flock assertion and native execution remain unchecked. | | P2-12 first-PR gates/adoption mapping | **not fixed in full** | deploy documentation and streaming perf scenario now exist. three-size early signal is measured, but required matrix/adoption-row mapping/native macOS evidence remain absent. | | P2-13 stuck-state retry rescans | **not fixed** | every attempt with `clean == false` still repeats full recovery; no backoff or retry-load evidence. | | P2-14 invalid owner skips liveness lock | **not fixed** | `recover_entry` still takes the owner lock only for a passing `judge_owner`; no new live-invalid-owner test. same-UID interference inside scratch is excluded by the approved boundary, which limits its relevance. | | round-1 P3 | status | evidence/limit | |---|---|---| | 1 destructured drop-order comment | not fixed | comment/order discrepancy remains; inode proof is independent of pending metadata. | | 2 inheritance after payload sync | not fixed | chown/ACL changes still follow payload `sync_all`. no durability-order regression test. | | 3 non-UTF-8 recovery names | not fixed | lossy string enumeration remains. | | 4 pin registration diagnostic | not fixed in full | duplicate registration now warns, but runtimes opened before registration still retain `pins=None`; warning does not detect/repair that case or compose sources. | | 5 production fault-seam mutex | verified mitigation | inactive atomic fast path avoids the mutex; fsio dependency/hooks remain. | | 6 idle pending temporary cleanup | not fixed | idle return still bypasses `tidy_pending`. | | 7 permanent Scratch cache/fds | not fixed | no eviction/revalidation. | | 8 kill-loop coverage/salt | verified | ≥500 rounds assert placed-and-owing coverage; salt is u16. reported final 1000-round run has 22 owed placements. | | 9 unsupported/leaked old flock | not fixed in full | leaked-child upgrade limitation is documented; unsupported flock still fails closed. | | 10 offline rollback | verified | current deploy script stops before forward backup/start and before rollback restart; docs describe both directions. not a live-upgrade fence, as approved. | | 11 shutdown blocking recovery | not fixed | aborting async task still does not cancel spawn_blocking/root wait. | | 12 all Home walkers exclude private files | not verified/fixed | serving and scratch usage guards exist; no exhaustive walker/lock-descriptor audit evidence added. | ## new findings ### R2-P1: cancellation drops the held-result guard and permits a second save **`crates/attachments/src/recovery.rs:390` and `:397`; caller `crates/attachments/src/http.rs:152`.** `reconcile_key` removes `Kept` from the map before awaiting repair. HTTP reconciliation runs before the detached task at `http.rs:180`. if that request future is cancelled during repair, `kept` drops without being reinserted, the placement guard releases, and the user's cached recovery state stays clean. the Ready record survives on disk, but subsequent live requests consult neither it nor the lost held-content guard. **reproduced on dev-01:** inject `publish.flush` EIO so `cancel.jpg` is public and unindexed; begin a retry; pause at `resolve.sync`; cancel the route future; release the blocking repair with `resolve.index` EIO; upload the same bytes with a new key. response is **201**, `already=false`, path **`cancel 2.jpg`**; both public files exist and the original completion remains. no restart is needed. the ordinary server kill loop does not test cancellation of a reconciliation future while the process stays alive. **fix:** make reconciliation cancellation-safe. retain authority in the shared map through every await (with a resolving state), or use a drop guard that reinserts on cancellation, or run the whole reconciliation with its placement guard in a detached task. neither another key nor `has_kept_content` may observe the obligation as absent until it is settled. add this exact cancellation + failed-index regression, including retry/restart convergence. ### R2-P2: changed-device regression never changes any completion **`crates/attachments/src/tests/recovery.rs:201–216`.** the injected `store.before-commit` key is raw `k-dev`, but the integrated store receives `action_key("attachments.add", "k-dev")`. the first upload succeeds and retires its record; the loop changing `dev` executes zero times. the assertions then replay an ordinary completed upload. this is a test regression introduced by receipt-scoping integration, despite the product fix being correct. **reproduced:** adding only `assert_eq!(e.markers(user), 1)` immediately after the first upload fails **0 vs 1**. changing the fault to the scoped key makes that assertion and the full changed-device scenario pass. **fix:** inject the scoped key, assert initial response failure, exactly one Ready record and zero committed events before restart; assert the record mutation count is one. keep raw-key behavior in the separate legacy-receipt test. ## benchmark audit read raw final perf-01 `~/perf/out-upload-759b-ba86c6f6/`, `order.txt`, input hashes, all 20 JSON/return-code files, fixture and harness. independently recomputed median paired head/dev ratios: **139.017948% / 119.746720% / 103.110232%**, matching the PR for 4 KiB / 4 MiB / 128 MiB. all reps `rc=0`; ten balanced alternating pairs (odd dev→head, even head→dev); same harness and fresh fixtures. recorded hashes match the PR's head/baseline/fixture/harness hashes. the three-size comparison is fair against the explicitly pinned dev `d83e54db`, not a measurement against later dev. median paired ratios are correctly distinguished from ratios of per-version medians. **passes the ≥95% threshold for those three early-signal measurements only. does not meet the complete #771 first-PR gate.** this fixture reports bytes divided by median request latency, not aggregate workload throughput, uses one client/common-root storage, and measures 200/40/5 uploads. it does not provide the required ≥60 s and ≥50 uploads/≥50 overlapping saves per point, cold/warm, split storage, 1/4 clients, 1k/10k/100k Homes, through 1 GiB, RSS/syscalls/metadata matrix. the new Rust harness is not the harness used for these measurements. initial failures are properly retained rather than erased. no benchmark rerun was needed for this audit. ## independent checks and boundaries on dev-01, reviewer worktree `~/Developer/Worktrees/review-788-r2`, Nix tools and `~/bin/slot`, `-j4`: - new cancellation regression: **fails**, two files versus one (`~/review788-cancellation.log`). - original device test plus pending precondition only: **fails**, zero pending versus one (`~/review788-device-red.log`). - scoped fault + pending precondition, then focused packages: **35 attachments, 94 fsio, 21 engine passed**, one engine performance test ignored, doc tests pass (`~/review788-focused.log`). the known failing cancellation test was explicitly skipped in this positive run. ```sh cargo test --locked -p calternal-attachments -j4 cancelled_reconciliation_keeps_the_duplicate_guard -- --nocapture cargo test --locked -p calternal-attachments -j4 a_changed_device_number -- --nocapture cargo test --locked -p calternal-attachments -p calternal-fsio -p calternal-engine -j4 -- --skip cancelled_reconciliation_keeps_the_duplicate_guard ``` reviewer test/precondition changes are uncommitted only in that isolated VM worktree; product source is unchanged. no full-workspace or native macOS rerun claimed. process-death ordering of unsynced atomic records is consistent with the approved scope; it is not a power-loss guarantee. normal publication syncs both rename parents, but retry repair still only resyncs the destination, so it does not independently demonstrate recovery from a failed source-parent fsync. pinning now covers scoped and legacy receipts, and aged SIGKILL cases pass; that proof remains conditional on registering the pin source before any runtime opens. **fix R2-P1 and R2-P2 before round 3. close or explicitly resolve the remaining plan acceptance gaps; do not label the three-size early signal as the full performance gate.**
kayg commented 2026-10-31 17:48:08 +00:00
Author
Owner
Copy link

Round 2 corrections pushed as ef9a08d9c5c891dadd9c2f307012ee4eed1a3cdc. Ready for FINAL independent review (round 3); no merge performed.

The duplicate guard is now owned by the shared held record through every repair/resolve await. Reconciliation copies the completion facts while holding placement, then removes the held record only after the engine confirms a durable result. Cancellation cannot drop the guard; the existing on-disk Ready record remains authoritative on restart. No new on-disk state, extra sync, or detached reconciliation task. One bounded completion snapshot is the only added transient cost (record cap 16 KiB); scan complexity is unchanged.

Finding / gate Status and evidence
R2-P1 exact reproduction Fixed. Original scenario ran red with 201, already=false, cancel 2.jpg, two files. Same test now gives 503 while index repair fails, one file, one Ready, zero events, then the original-key retry and restart converge to one file/event.
Cancellation at every reconciliation await Pass: eight labelled await sites across reconciliation, repair, engine resolution and retirement; cancellation before/after each, with both live retry and immediate restart (32 cases). Each asserts one saved copy, one added event and eventual record retirement. Shared guard is explicitly required while the result remains owed. This test ran red at key.resolve/false before the ownership fix. Test hooks compile out to the original awaits in non-test builds.
R2-P2 changed-device test Fixed scoped fault key; now requires initial failure, one pending Ready, zero committed events, and exactly one record mutation. Preconditions ran red against the old raw fault key; corrected full scenario passes.
Focused packages Exit 0: 37 attachments, 94 fsio, 21 engine; one engine perf-only test ignored; doc tests pass.
Full workspace through doc tests Exit 0: 1,287 passed across 91 log summaries, 20 ignored, zero failed. Count includes the isolated reproduction child (1,286 at the workspace level plus that child).
Separate 1,000-round SIGKILL loop Exit 0: 1,000 uploads settled exactly once; seed 75931004, 67.07 s; 353 before state / 534 scratch / 19 placed-and-owing / 94 finished.
Workspace all-target Clippy, warnings denied Exit 0.
Formatting Exit 0.
FINAL round 3 independent review Pending; no new review delegate started by this implementation sub-agent.
Existing acceptance gaps Native macOS proof, complete #771 cold/warm/split-storage/client/Home-size/performance matrix, and other open round-1 items remain open as detailed in the round-2 review. The old three-size benchmark remains an early signal, not the full gate.

All commands ran on dev-01, in ~/Developer/Worktrees/fix-759b, using Nix and ~/bin/slot; heavy Cargo commands used -j4. Exact commands:

cargo test --locked -p calternal-attachments -p calternal-fsio -p calternal-engine -j4 --no-fail-fast
CALTERNAL_KILL_LOOP_SEED=75931003 cargo test --workspace --locked -j4 --no-fail-fast
CALTERNAL_KILL_LOOP_N=1000 CALTERNAL_KILL_LOOP_SEED=75931004 cargo test --locked -p calternal --test attachments_kill_loop -j4 -- --nocapture
cargo clippy --workspace --all-targets --locked -j4 -- -D warnings
cargo fmt --all --check

Logs: ~/fix759b-r3-{red,device-red,await-red,focused,workspace,kill-loop,clippy,fmt}.log; final orchestration ~/fix759b-r3-gates.log and per-gate .exit files. The initial await-test compilation error (private test module) was corrected before the recorded assertion-red run. The regression's global pause hook runs in an isolated child so parallel tests cannot consume it. The cancellation matrix uses task-local hooks.

Gates tested the exact final source bytes as uncommitted changes atop ba86c6f6; SHA256 of all four changed files matched between dev-01 and control before committing/pushing ef9a08d9. The kill-loop's printed Git head therefore names the parent; it exercised the final changed source. Control worktree is clean. Required VM Nix shell files were not committed.

Scope remains process death, not power loss. The round-2 source-parent sync, pin registration/composition and other documented limitations are not claimed resolved by this correction. No production access, deployment or merge.

Round 2 corrections pushed as `ef9a08d9c5c891dadd9c2f307012ee4eed1a3cdc`. Ready for FINAL independent review (round 3); no merge performed. The duplicate guard is now owned by the shared held record through every repair/resolve await. Reconciliation copies the completion facts while holding placement, then removes the held record only after the engine confirms a durable result. Cancellation cannot drop the guard; the existing on-disk Ready record remains authoritative on restart. No new on-disk state, extra sync, or detached reconciliation task. One bounded completion snapshot is the only added transient cost (record cap 16 KiB); scan complexity is unchanged. | Finding / gate | Status and evidence | |---|---| | R2-P1 exact reproduction | Fixed. Original scenario ran red with **201, already=false, cancel 2.jpg, two files**. Same test now gives **503 while index repair fails, one file, one Ready, zero events**, then the original-key retry and restart converge to one file/event. | | Cancellation at every reconciliation await | Pass: eight labelled await sites across reconciliation, repair, engine resolution and retirement; cancellation before/after each, with both live retry and immediate restart (32 cases). Each asserts one saved copy, one added event and eventual record retirement. Shared guard is explicitly required while the result remains owed. This test ran red at `key.resolve/false` before the ownership fix. Test hooks compile out to the original awaits in non-test builds. | | R2-P2 changed-device test | Fixed scoped fault key; now requires initial failure, one pending Ready, zero committed events, and exactly one record mutation. Preconditions ran red against the old raw fault key; corrected full scenario passes. | | Focused packages | Exit 0: 37 attachments, 94 fsio, 21 engine; one engine perf-only test ignored; doc tests pass. | | Full workspace through doc tests | Exit 0: **1,287 passed across 91 log summaries, 20 ignored, zero failed**. Count includes the isolated reproduction child (1,286 at the workspace level plus that child). | | Separate 1,000-round SIGKILL loop | Exit 0: 1,000 uploads settled exactly once; seed **75931004**, 67.07 s; 353 before state / 534 scratch / **19 placed-and-owing** / 94 finished. | | Workspace all-target Clippy, warnings denied | Exit 0. | | Formatting | Exit 0. | | FINAL round 3 independent review | Pending; no new review delegate started by this implementation sub-agent. | | Existing acceptance gaps | Native macOS proof, complete #771 cold/warm/split-storage/client/Home-size/performance matrix, and other open round-1 items remain open as detailed in the round-2 review. The old three-size benchmark remains an early signal, not the full gate. | All commands ran on **dev-01**, in `~/Developer/Worktrees/fix-759b`, using Nix and `~/bin/slot`; heavy Cargo commands used `-j4`. Exact commands: ```sh cargo test --locked -p calternal-attachments -p calternal-fsio -p calternal-engine -j4 --no-fail-fast CALTERNAL_KILL_LOOP_SEED=75931003 cargo test --workspace --locked -j4 --no-fail-fast CALTERNAL_KILL_LOOP_N=1000 CALTERNAL_KILL_LOOP_SEED=75931004 cargo test --locked -p calternal --test attachments_kill_loop -j4 -- --nocapture cargo clippy --workspace --all-targets --locked -j4 -- -D warnings cargo fmt --all --check ``` Logs: `~/fix759b-r3-{red,device-red,await-red,focused,workspace,kill-loop,clippy,fmt}.log`; final orchestration `~/fix759b-r3-gates.log` and per-gate `.exit` files. The initial await-test compilation error (private test module) was corrected before the recorded assertion-red run. The regression's global pause hook runs in an isolated child so parallel tests cannot consume it. The cancellation matrix uses task-local hooks. Gates tested the exact final source bytes as uncommitted changes atop `ba86c6f6`; SHA256 of all four changed files matched between dev-01 and control before committing/pushing `ef9a08d9`. The kill-loop's printed Git head therefore names the parent; it exercised the final changed source. Control worktree is clean. Required VM Nix shell files were not committed. Scope remains process death, not power loss. The round-2 source-parent sync, pin registration/composition and other documented limitations are not claimed resolved by this correction. No production access, deployment or merge.
kayg commented 2026-10-31 18:00:03 +00:00
Author
Owner
Copy link

Independent review, round 3 (final)

verdict: small fix (P2-or-lower only) at ef9a08d9c5c891dadd9c2f307012ee4eed1a3cdc. the round-2 correctness fix is verified. no new P0/P1; no new P2/P3 found in this correction. overall acceptance still requires closing or explicitly resolving the previously documented acceptance gaps below. this is not approval to merge.

reviewed the first PR comment, round 2 (#issuecomment-71016), fix status (#issuecomment-71096), the four-file correction, and the surrounding placement, HTTP, engine-resolution, record and store paths. control host remained read-only; all compilation/tests ran on dev-01. no code pushed.

check status evidence / limit
R2-P1 cancellation loses duplicate guard verified fixed crates/attachments/src/recovery.rs:409–421 clones completion facts without removing the shared Kept. failed or cancelled repair leaves the map entry and Ready authority intact. exact cancelled-blocking-repair + index-EIO reproduction passes: refusal, one file, one Ready, zero events, then convergence.
cancellation at reconciliation await sites pass shipped matrix: eight labelled sites, before/after, live retry/immediate restart = 32 cases. tests assert the guard remains while the event is owed, one copy/event, and eventual retirement. the blocking-repair reproduction additionally cancels while the blocking job is actually suspended; these tests do not exhaust every scheduler interleaving inside engine internals.
crash + cancellation combined pass, independently added reviewer-only subprocess test drops the reconciliation future at each of those 16 before/after boundaries, then really SIGKILLs the process. restart and four concurrent original-key retries produce one file/event, zero records, no held entry. all 16 cases passed.
concurrent retries of the same unresolved key pass, independently added 16 live rounds, four simultaneous original-key uploads per round after failed publication flush. all replay successfully; one file/event, zero records, no held entry. placement serializes reconciliation and the map is rechecked after acquiring it; engine claims serialize/join commit work.
release only after repair + commit verified within process-death scope destination fsync and index repair at recovery.rs:284–288 precede awaited engine resolution; removal at :424 follows success. existing repeated sync/index-EIO regression passes. engine resolution runs owned work independently of a cancelled waiter and checks scoped/legacy receipts before committing. retry still does not resync the source parent: the existing round-2 limitation remains, so this is not proof of both-parent retry repair or power-loss durability.
no permanent guard leak after recoverable failure/cancellation pass original failure/cancellation regressions settle and retire. both added tests assert no held entry and no pending record after settlement, then successfully save different bytes with a fresh key. persistent storage damage intentionally remains a refusal, not silently released authority.
R2-P2 changed-device test verified fixed crates/attachments/src/tests/recovery.rs:201–236 injects the scoped key and requires initial failure, one Ready, zero events and exactly one mutated record. regression passes.
carried-forward acceptance gaps open, not new findings native macOS, full #771 performance/storage/client/Home-size matrix and the other unresolved round-1 P2/P3 items remain as listed in round 2. the correction does not resolve source-parent retry sync, pin registration/composition or those earlier gaps.

new findings

  • P0: none.
  • P1: none.
  • P2: none newly identified.
  • P3: none newly identified.

there is no new P0/P1 triggering kayg's round-3 stop-and-simplify rule. retain the shared-map ownership correction. before calling the complete PR ready, close or explicitly resolve the existing P2 acceptance items, especially native macOS and the complete #771 gate; the three-size early signal remains insufficient for that gate.

independent validation

isolated VM worktree: ~/Developer/Worktrees/review-788-r3, detached at the reviewed head. product code unchanged; reviewer-only additions are uncommitted in crates/attachments/src/tests/reconcile.rs, alongside required uncommitted Nix shell files. correction source and changed-device test SHA256 matched control.

export PATH=/nix/var/nix/profiles/default/bin:$PATH
export CARGO_TARGET_DIR=/home/claude/Developer/Worktrees/fix-759b/target
cd ~/Developer/Worktrees/review-788-r3
~/bin/slot nix develop -c bash -c 'cargo test --locked -p calternal-attachments -j4 -- --nocapture'

final run: 39 passed, zero failed, doc tests pass, exit 0. log: ~/review788-r3-focused2.log, exit file: ~/review788-r3-focused2.exit. this includes all 37 shipped tests plus the two reviewer tests containing the cases above.

the initial reviewer run required all 16 simultaneous uploads to succeed and correctly hit the four-upload admission limit (429). adjusted only reviewer concurrency to four, then reran successfully; initial log retained at ~/review788-r3-focused.log.

also checked implementation gate exit files (workspace/kill-loop/Clippy/fmt all 0) and the recorded 1,000-round SIGKILL summary: 1,000 settled once, 19 placed-and-owing. those broader gates were not independently rerun here. the independent tests above run against the exact reviewed product source. no production access, deployment, merge or push.

# Independent review, round 3 (final) **verdict: small fix (P2-or-lower only)** at `ef9a08d9c5c891dadd9c2f307012ee4eed1a3cdc`. the round-2 correctness fix is verified. **no new P0/P1; no new P2/P3 found in this correction.** overall acceptance still requires closing or explicitly resolving the previously documented acceptance gaps below. this is not approval to merge. reviewed the first PR comment, round 2 (#issuecomment-71016), fix status (#issuecomment-71096), the four-file correction, and the surrounding placement, HTTP, engine-resolution, record and store paths. control host remained read-only; all compilation/tests ran on dev-01. no code pushed. | check | status | evidence / limit | |---|---|---| | R2-P1 cancellation loses duplicate guard | verified fixed | `crates/attachments/src/recovery.rs:409–421` clones completion facts without removing the shared `Kept`. failed or cancelled repair leaves the map entry and Ready authority intact. exact cancelled-blocking-repair + index-EIO reproduction passes: refusal, one file, one Ready, zero events, then convergence. | | cancellation at reconciliation await sites | pass | shipped matrix: eight labelled sites, before/after, live retry/immediate restart = 32 cases. tests assert the guard remains while the event is owed, one copy/event, and eventual retirement. the blocking-repair reproduction additionally cancels while the blocking job is actually suspended; these tests do not exhaust every scheduler interleaving inside engine internals. | | crash + cancellation combined | pass, independently added | reviewer-only subprocess test drops the reconciliation future at each of those 16 before/after boundaries, then really SIGKILLs the process. restart and four concurrent original-key retries produce one file/event, zero records, no held entry. all 16 cases passed. | | concurrent retries of the same unresolved key | pass, independently added | 16 live rounds, four simultaneous original-key uploads per round after failed publication flush. all replay successfully; one file/event, zero records, no held entry. placement serializes reconciliation and the map is rechecked after acquiring it; engine claims serialize/join commit work. | | release only after repair + commit | verified within process-death scope | destination fsync and index repair at `recovery.rs:284–288` precede awaited engine resolution; removal at `:424` follows success. existing repeated sync/index-EIO regression passes. engine resolution runs owned work independently of a cancelled waiter and checks scoped/legacy receipts before committing. retry still does not resync the source parent: the existing round-2 limitation remains, so this is not proof of both-parent retry repair or power-loss durability. | | no permanent guard leak after recoverable failure/cancellation | pass | original failure/cancellation regressions settle and retire. both added tests assert no held entry and no pending record after settlement, then successfully save different bytes with a fresh key. persistent storage damage intentionally remains a refusal, not silently released authority. | | R2-P2 changed-device test | verified fixed | `crates/attachments/src/tests/recovery.rs:201–236` injects the scoped key and requires initial failure, one Ready, zero events and exactly one mutated record. regression passes. | | carried-forward acceptance gaps | open, not new findings | native macOS, full #771 performance/storage/client/Home-size matrix and the other unresolved round-1 P2/P3 items remain as listed in round 2. the correction does not resolve source-parent retry sync, pin registration/composition or those earlier gaps. | ## new findings - P0: none. - P1: none. - P2: none newly identified. - P3: none newly identified. there is no new P0/P1 triggering kayg's round-3 stop-and-simplify rule. retain the shared-map ownership correction. before calling the complete PR ready, close or explicitly resolve the existing P2 acceptance items, especially native macOS and the complete #771 gate; the three-size early signal remains insufficient for that gate. ## independent validation isolated VM worktree: `~/Developer/Worktrees/review-788-r3`, detached at the reviewed head. product code unchanged; reviewer-only additions are uncommitted in `crates/attachments/src/tests/reconcile.rs`, alongside required uncommitted Nix shell files. correction source and changed-device test SHA256 matched control. ```sh export PATH=/nix/var/nix/profiles/default/bin:$PATH export CARGO_TARGET_DIR=/home/claude/Developer/Worktrees/fix-759b/target cd ~/Developer/Worktrees/review-788-r3 ~/bin/slot nix develop -c bash -c 'cargo test --locked -p calternal-attachments -j4 -- --nocapture' ``` final run: **39 passed, zero failed**, doc tests pass, exit 0. log: `~/review788-r3-focused2.log`, exit file: `~/review788-r3-focused2.exit`. this includes all 37 shipped tests plus the two reviewer tests containing the cases above. the initial reviewer run required all 16 simultaneous uploads to succeed and correctly hit the four-upload admission limit (429). adjusted only reviewer concurrency to four, then reran successfully; initial log retained at `~/review788-r3-focused.log`. also checked implementation gate exit files (workspace/kill-loop/Clippy/fmt all 0) and the recorded 1,000-round SIGKILL summary: 1,000 settled once, 19 placed-and-owing. those broader gates were **not independently rerun** here. the independent tests above run against the exact reviewed product source. no production access, deployment, merge or push.
kayg commented 2026-10-31 21:10:08 +00:00
Author
Owner
Copy link

Acceptance status at 0d2d416e

Status: not ready for merge. No merge was performed. This records the native runs, current partial matrix, post-round-3 small fixes, and the remaining round-1/round-2 dispositions. The complete #771 gate is still open.

Gate table

Gate Result Evidence
Dev-01 workspace gates Pass Exact head 0d2d416e; cargo fmt --all --check, cargo clippy --workspace --all-targets --locked -j4 -- -D warnings, and CALTERNAL_KILL_LOOP_SEED=75931003 cargo test --workspace --locked -j4 --no-fail-fast, run through the VM slot/Nix shell. 1,289 passed, 20 ignored, 0 failed; exit 0. Log: ~/fix759b-0d2d-dev-gates.log.
Native macOS focused suites Pass On lume@calternal-macos-01-std, attachments 38, fsio 81, engine 21 passed / 1 ignored; storage_locks 5 passed. The runner held /tmp/mac-bench.lock in the same shell that installed the EXIT trap. Exit 0; the trap released it and the queued glass-frames-795 job acquired it afterward.
Native macOS kill loop Pass CALTERNAL_KILL_LOOP_N=1000 CALTERNAL_KILL_LOOP_SEED=75931005 cargo test --locked -p calternal --test attachments_kill_loop -j4 -- --nocapture: 1,000 rounds / 1,000 uploads, all settled exactly once; 279 died before state, 681 in scratch, 3 placed and owing, 37 finished. Exit 0.
Broader native #771 restart/storage matrix Open These runs do not establish the combined actual-server restart with a fork-only child retaining inherited descriptors, split Home/state attachment recovery, or every #771 recovery fixture. The requested native suites and kill loop passed, but this wider plan gate remains unproven.
Release artifact relay Pass for the measured product code Release binaries were built on dev-01, relayed through the control host, and SHA256 checked on both perf VMs. Head binary SHA256 c78fbed0aaccfbe3ba089ff6fcd31b0e2f32e4891d26fa04e6ad11789f66330e; Rust perf driver SHA256 e83e22618a1ce06bc4bc42ee16ed966eff0fe4ff922c42da1cc42ae4f8700a8b. The measured product code is 0f38de31; later branch commits change tests/fixtures and formatting. Baseline is release dev 133cf2c2.
Warm performance matrix In progress; partial result fails p95 limits Five alternating warm pairs are running under ~/bin/perf-lock on perf-01 and perf-02. Each VM has 53 JSON results at the latest status check: five upload sizes have all five valid baseline/head pairs in the first configuration, and 1 GiB is still underway. Perf-01 1k-split-c1: 4 KiB, 256 KiB, 4 MiB, 32 MiB pass the three thresholds; 128 MiB has response p95 116.98% and save-ack p95 116.20% of dev. Perf-02 10k-ssd-c1: 4 KiB, 256 KiB, 4 MiB, and 128 MiB fail save-ack p95 (119.02%, 128.52%, 156.34%, 118.03%); 32 MiB passes. Completed throughput ratios are at least 99.03%. These are paired medians, not a complete gate result.
Full #771 size/Home/storage/client matrix Open The runner enumerates 1k/10k/100k Homes, split and all-SSD layouts, 1/4 clients, and 4 KiB/256 KiB/4 MiB/32 MiB/128 MiB/1 GiB. Only the first configuration on each VM has partial results so far. The #771 target is throughput ≥95%, response p95 ≤105%, and save-ack p95 ≤105% for every workload. PR #787's perf/vm/upload-bench.sh is only a fixed three-size wrapper; the current matrix uses the Rust calternal-perf driver.
Cold-cache and system-cost evidence Open --cold-runs 0: sudo -n test -w /proc/sys/vm/drop_caches is unavailable on these VMs. The current runner records RSS and response/save samples, but not metadata bytes or syscall/flush counts. The cold pairs and these system-cost measurements are not claimed.

macOS behavior

The code uses File::sync_all / fsync; it makes no explicit F_FULLFSYNC call. The guarantee remains process death, not power-loss durability. Darwin F_SETLK owner-lock and old-flock startup probes passed in the native suite; their semantics are not assumed identical to Linux. On APFS, case-only exclusive rename names can alias the same entry, so the fixtures account for that. The native build emitted an unused state variable warning at crates/calternal/src/storage.rs:265; it did not fail the run.

Small carried-forward fixes after round 3

Commit 0f38de31 adds source-parent retry sync with a recovery regression, pin-source composition and late-registration rejection tests, and idle pending-temp cleanup. 6ff0490b registers the auth-test pin source before its first store use. The workspace gate above covers these changes.

Round-1 / round-2 P2 dispositions

  • P2-1 unreadable completion record: Keep pruning fail-closed and retain possible commit evidence; warning and per-user recovery degradation remain.
  • P2-2 power loss: Excluded by the approved process-death-only guarantee; no power-loss ordering is claimed.
  • P2-3 repeated same-name sources: Fixed for distinct inodes; the regression asserts separate recovered destinations and passes.
  • P2-4 unknown recovered entries blocking admin cleanup: Keep them visibly blocked rather than remove bytes without a recovery inventory; the cleanup workflow remains open.
  • P2-5 recovered-intent traversal: Fixed and tested for forged path components; other source variants still need broader coverage.
  • P2-6 public provenance xattr: Retain it as provenance; it grants no deletion authority under the approved scratch boundary.
  • P2-7 filesystem capability preflight: No startup probe; unsupported targets fail safely at the affected upload, so preflight remains open.
  • P2-8 labels, large xattrs, Darwin ACL inheritance: Existing checks refuse label mismatch; broad xattr and native ACL inheritance evidence remains open.
  • P2-9 registry final-close/reopen race and global syscall mutex: Defer the registry lifetime change until a deterministic last-close/reopen test can cover its lock ordering; current lock tests do not prove this interval.
  • P2-10 split Home/state attachment recovery: Keep open until a split-root actual-server recovery test covers attachment paths and pins.
  • P2-11 inherited-FD actual-server macOS restart: Keep open; the native storage-lock suite and kill loop do not combine the fork-child, restart, and recovery case.
  • P2-12 adoption mapping and complete first-PR gates: Keep open; the matrix is incomplete, and no broader adoption mapping is claimed.
  • P2-13 repeated recovery scans for stuck state: No backoff added; each retry still rechecks state, and repeated-scan load under persistent failure is unmeasured.
  • P2-14 invalid owner without liveness lock: Same-user edits inside app-controlled scratch are outside the approved preservation boundary; a live-invalid-owner test remains absent.

Round-1 P3 dispositions

  • P3-1 drop-order comment: Deferred as a comment-only discrepancy; inode validation is independent of pending metadata order.
  • P3-2 inheritance after payload sync: No power-loss ordering claim is made; the metadata sync ordering remains open.
  • P3-3 non-UTF-8 recovery names: Byte-preserving enumeration and tests remain open.
  • P3-4 pin registration/composition: Fixed with composed pin sources and rejection of registration after runtime open; tested on the engine path.
  • P3-5 production fault seam mutex: Mitigated by the inactive atomic fast path; workspace tests pass.
  • P3-6 idle pending-temp cleanup: Fixed in 0f38de31 and covered by the workspace suite.
  • P3-7 permanent Scratch cache/fds: No eviction added while cached entries can own active descriptors; a safe bound and eviction test remain open.
  • P3-8 kill-loop coverage/salt: Verified; the native 1,000-round run above passed with placed-and-owing coverage.
  • P3-9 old flock transition: Offline stop-before-upgrade/rollback is the chosen protocol; unsupported locks fail closed and the native old-flock startup probe passes. Live mixed-generation upgrade is not claimed.
  • P3-10 offline rollback: Deployment stops the old server before restart in either direction; existing deployment checks/docs cover it.
  • P3-11 shutdown waiting on blocking recovery: Deferred until a bounded join/cancellation design can preserve safe recovery completion.
  • P3-12 all Home walkers exclude private files: Serving/scratch guards exist, but an exhaustive walker and lock-descriptor audit remains open.

The open rows above are explicit limits, not acceptance passes. PR #788 should remain unmerged until the warm/cold matrix and the wider native/storage evidence are resolved.

Exact-head matrix continuation

The partial ratios above are from the pre-existing warm run using product binary 0f38de31; subsequent PR commits contain test/fixture and formatting changes. I prepared a fresh run at exact PR head 0d2d416e on both requested claude@ perf accounts. The dev release baseline SHA256 3627d4f1a055593f4387ab2c59c4de1cfc93a6b5a6bd199ae9ac35fd1f214328 matches the source on dev-01 and both perf VMs. The slot helper SHA256 is 4b2c6146cf0868a8b96b49a2f3344ca2b5117d6ab905736bb9efbcea287fb423 on dev-01 and both perf VMs. Exact head and driver hashes are listed above.

Both fresh matrix scripts pass bash -n and use the requested ~/bin/perf-lock and ~/bin/slot, with five alternating warm pairs. Transient timers are active for both VMs and fire after 7,200 seconds, after the existing three-hour capped jobs can release their locks. Each resulting service has RuntimeMaxSec=10800 and Restart=on-failure with a 20-second delay; the script skips valid completed pairs and reruns an interrupted pair. Exact-head matrix results are not yet available, so the full gate remains open.

# Acceptance status at `0d2d416e` **Status: not ready for merge. No merge was performed.** This records the native runs, current partial matrix, post-round-3 small fixes, and the remaining round-1/round-2 dispositions. The complete #771 gate is still open. ## Gate table | Gate | Result | Evidence | |---|---|---| | Dev-01 workspace gates | **Pass** | Exact head `0d2d416e`; `cargo fmt --all --check`, `cargo clippy --workspace --all-targets --locked -j4 -- -D warnings`, and `CALTERNAL_KILL_LOOP_SEED=75931003 cargo test --workspace --locked -j4 --no-fail-fast`, run through the VM slot/Nix shell. **1,289 passed, 20 ignored, 0 failed**; exit 0. Log: `~/fix759b-0d2d-dev-gates.log`. | | Native macOS focused suites | **Pass** | On `lume@calternal-macos-01-std`, attachments **38**, fsio **81**, engine **21 passed / 1 ignored**; `storage_locks` **5 passed**. The runner held `/tmp/mac-bench.lock` in the same shell that installed the EXIT trap. Exit 0; the trap released it and the queued `glass-frames-795` job acquired it afterward. | | Native macOS kill loop | **Pass** | `CALTERNAL_KILL_LOOP_N=1000 CALTERNAL_KILL_LOOP_SEED=75931005 cargo test --locked -p calternal --test attachments_kill_loop -j4 -- --nocapture`: **1,000 rounds / 1,000 uploads**, all settled exactly once; 279 died before state, 681 in scratch, 3 placed and owing, 37 finished. Exit 0. | | Broader native #771 restart/storage matrix | **Open** | These runs do not establish the combined actual-server restart with a fork-only child retaining inherited descriptors, split Home/state attachment recovery, or every #771 recovery fixture. The requested native suites and kill loop passed, but this wider plan gate remains unproven. | | Release artifact relay | **Pass for the measured product code** | Release binaries were built on dev-01, relayed through the control host, and SHA256 checked on both perf VMs. Head binary SHA256 `c78fbed0aaccfbe3ba089ff6fcd31b0e2f32e4891d26fa04e6ad11789f66330e`; Rust perf driver SHA256 `e83e22618a1ce06bc4bc42ee16ed966eff0fe4ff922c42da1cc42ae4f8700a8b`. The measured product code is `0f38de31`; later branch commits change tests/fixtures and formatting. Baseline is release dev `133cf2c2`. | | Warm performance matrix | **In progress; partial result fails p95 limits** | Five alternating warm pairs are running under `~/bin/perf-lock` on perf-01 and perf-02. Each VM has 53 JSON results at the latest status check: five upload sizes have all five valid baseline/head pairs in the first configuration, and 1 GiB is still underway. Perf-01 `1k-split-c1`: 4 KiB, 256 KiB, 4 MiB, 32 MiB pass the three thresholds; 128 MiB has response p95 **116.98%** and save-ack p95 **116.20%** of dev. Perf-02 `10k-ssd-c1`: 4 KiB, 256 KiB, 4 MiB, and 128 MiB fail save-ack p95 (**119.02%, 128.52%, 156.34%, 118.03%**); 32 MiB passes. Completed throughput ratios are at least 99.03%. These are paired medians, not a complete gate result. | | Full #771 size/Home/storage/client matrix | **Open** | The runner enumerates 1k/10k/100k Homes, split and all-SSD layouts, 1/4 clients, and 4 KiB/256 KiB/4 MiB/32 MiB/128 MiB/1 GiB. Only the first configuration on each VM has partial results so far. The #771 target is throughput ≥95%, response p95 ≤105%, and save-ack p95 ≤105% for every workload. PR #787's `perf/vm/upload-bench.sh` is only a fixed three-size wrapper; the current matrix uses the Rust `calternal-perf` driver. | | Cold-cache and system-cost evidence | **Open** | `--cold-runs 0`: `sudo -n test -w /proc/sys/vm/drop_caches` is unavailable on these VMs. The current runner records RSS and response/save samples, but not metadata bytes or syscall/flush counts. The cold pairs and these system-cost measurements are not claimed. | ### macOS behavior The code uses `File::sync_all` / `fsync`; it makes no explicit `F_FULLFSYNC` call. The guarantee remains process death, not power-loss durability. Darwin `F_SETLK` owner-lock and old-`flock` startup probes passed in the native suite; their semantics are not assumed identical to Linux. On APFS, case-only exclusive rename names can alias the same entry, so the fixtures account for that. The native build emitted an unused `state` variable warning at `crates/calternal/src/storage.rs:265`; it did not fail the run. ### Small carried-forward fixes after round 3 Commit `0f38de31` adds source-parent retry sync with a recovery regression, pin-source composition and late-registration rejection tests, and idle pending-temp cleanup. `6ff0490b` registers the auth-test pin source before its first store use. The workspace gate above covers these changes. ### Round-1 / round-2 P2 dispositions - **P2-1 unreadable completion record:** Keep pruning fail-closed and retain possible commit evidence; warning and per-user recovery degradation remain. - **P2-2 power loss:** Excluded by the approved process-death-only guarantee; no power-loss ordering is claimed. - **P2-3 repeated same-name sources:** Fixed for distinct inodes; the regression asserts separate recovered destinations and passes. - **P2-4 unknown recovered entries blocking admin cleanup:** Keep them visibly blocked rather than remove bytes without a recovery inventory; the cleanup workflow remains open. - **P2-5 recovered-intent traversal:** Fixed and tested for forged path components; other source variants still need broader coverage. - **P2-6 public provenance xattr:** Retain it as provenance; it grants no deletion authority under the approved scratch boundary. - **P2-7 filesystem capability preflight:** No startup probe; unsupported targets fail safely at the affected upload, so preflight remains open. - **P2-8 labels, large xattrs, Darwin ACL inheritance:** Existing checks refuse label mismatch; broad xattr and native ACL inheritance evidence remains open. - **P2-9 registry final-close/reopen race and global syscall mutex:** Defer the registry lifetime change until a deterministic last-close/reopen test can cover its lock ordering; current lock tests do not prove this interval. - **P2-10 split Home/state attachment recovery:** Keep open until a split-root actual-server recovery test covers attachment paths and pins. - **P2-11 inherited-FD actual-server macOS restart:** Keep open; the native storage-lock suite and kill loop do not combine the fork-child, restart, and recovery case. - **P2-12 adoption mapping and complete first-PR gates:** Keep open; the matrix is incomplete, and no broader adoption mapping is claimed. - **P2-13 repeated recovery scans for stuck state:** No backoff added; each retry still rechecks state, and repeated-scan load under persistent failure is unmeasured. - **P2-14 invalid owner without liveness lock:** Same-user edits inside app-controlled scratch are outside the approved preservation boundary; a live-invalid-owner test remains absent. ### Round-1 P3 dispositions - **P3-1 drop-order comment:** Deferred as a comment-only discrepancy; inode validation is independent of pending metadata order. - **P3-2 inheritance after payload sync:** No power-loss ordering claim is made; the metadata sync ordering remains open. - **P3-3 non-UTF-8 recovery names:** Byte-preserving enumeration and tests remain open. - **P3-4 pin registration/composition:** Fixed with composed pin sources and rejection of registration after runtime open; tested on the engine path. - **P3-5 production fault seam mutex:** Mitigated by the inactive atomic fast path; workspace tests pass. - **P3-6 idle pending-temp cleanup:** Fixed in `0f38de31` and covered by the workspace suite. - **P3-7 permanent Scratch cache/fds:** No eviction added while cached entries can own active descriptors; a safe bound and eviction test remain open. - **P3-8 kill-loop coverage/salt:** Verified; the native 1,000-round run above passed with placed-and-owing coverage. - **P3-9 old `flock` transition:** Offline stop-before-upgrade/rollback is the chosen protocol; unsupported locks fail closed and the native old-`flock` startup probe passes. Live mixed-generation upgrade is not claimed. - **P3-10 offline rollback:** Deployment stops the old server before restart in either direction; existing deployment checks/docs cover it. - **P3-11 shutdown waiting on blocking recovery:** Deferred until a bounded join/cancellation design can preserve safe recovery completion. - **P3-12 all Home walkers exclude private files:** Serving/scratch guards exist, but an exhaustive walker and lock-descriptor audit remains open. The open rows above are explicit limits, not acceptance passes. PR #788 should remain unmerged until the warm/cold matrix and the wider native/storage evidence are resolved. ### Exact-head matrix continuation The partial ratios above are from the pre-existing warm run using product binary `0f38de31`; subsequent PR commits contain test/fixture and formatting changes. I prepared a fresh run at exact PR head `0d2d416e` on both requested `claude@` perf accounts. The dev release baseline SHA256 `3627d4f1a055593f4387ab2c59c4de1cfc93a6b5a6bd199ae9ac35fd1f214328` matches the source on dev-01 and both perf VMs. The slot helper SHA256 is `4b2c6146cf0868a8b96b49a2f3344ca2b5117d6ab905736bb9efbcea287fb423` on dev-01 and both perf VMs. Exact head and driver hashes are listed above. Both fresh matrix scripts pass `bash -n` and use the requested `~/bin/perf-lock` and `~/bin/slot`, with five alternating warm pairs. Transient timers are active for both VMs and fire after 7,200 seconds, after the existing three-hour capped jobs can release their locks. Each resulting service has `RuntimeMaxSec=10800` and `Restart=on-failure` with a 20-second delay; the script skips valid completed pairs and reruns an interrupted pair. Exact-head matrix results are not yet available, so the full gate remains open.
kayg commented 2026-11-01 11:50:02 +00:00
Author
Owner
Copy link

759b matrix takeover results

Head 0d2d416e5ac13b39fb46f53e7a7af79f6d36b3b2; baseline 133cf2c2. Median of matched per-pair head/dev ratios. Gates: throughput >=95%, response p95 <=105%, overlapping save-ack p95 <=105%. Only full five-pair cells can pass acceptance. RSS is retained in raw JSON, with no additional RSS gate invented.

VM / Home-layout-clients upload valid matched pairs / 5 throughput % response p95 % save-ack p95 % verdict
perf-01 / 1k-split-c1 4k 5 146.88 68.42 101.71 PASS
perf-01 / 1k-split-c1 256k 5 144.95 72.59 96.27 PASS
perf-01 / 1k-split-c1 4m 5 118.08 89.54 106.88 FAIL
perf-01 / 1k-split-c1 32m 5 97.24 99.34 103.60 PASS
perf-01 / 1k-split-c1 128m 5 100.99 100.44 111.39 FAIL
perf-01 / 1k-split-c1 1g 5 100.61 95.00 105.43 FAIL
perf-01 / 1k-split-c4 4k 5 137.30 71.27 87.82 PASS
perf-01 / 1k-split-c4 256k 5 138.44 75.31 93.32 PASS
perf-01 / 1k-split-c4 4m 5 122.12 75.46 83.73 PASS
perf-01 / 1k-split-c4 32m 5 104.58 85.23 89.89 PASS
perf-01 / 1k-split-c4 128m 5 100.01 104.62 79.83 PASS
perf-01 / 1k-split-c4 1g 5 101.73 98.75 98.59 PASS
perf-01 / 1k-ssd-c1 4k 5 127.76 83.54 144.09 FAIL
perf-01 / 1k-ssd-c1 256k 5 122.31 84.18 140.70 FAIL
perf-01 / 1k-ssd-c1 4m 5 115.11 90.95 117.69 FAIL
perf-01 / 1k-ssd-c1 32m 5 102.36 97.43 88.31 PASS
perf-01 / 1k-ssd-c1 128m 5 97.70 104.55 129.87 FAIL
perf-01 / 1k-ssd-c1 1g 5 101.95 95.70 99.29 PASS
perf-01 / 1k-ssd-c4 4k 5 116.30 97.80 146.52 FAIL
perf-01 / 1k-ssd-c4 256k 5 114.18 91.44 199.91 FAIL
perf-01 / 1k-ssd-c4 4m 5 105.34 128.52 148.67 FAIL
perf-01 / 1k-ssd-c4 32m 5 103.79 88.37 95.53 PASS
perf-01 / 1k-ssd-c4 128m 5 101.73 70.22 73.90 PASS
perf-01 / 1k-ssd-c4 1g 3 76.55 139.07 126.37 partial, fails observed
perf-01 / 10k-split-c1 4k 0 — — — missing
perf-01 / 10k-split-c1 256k 0 — — — missing
perf-01 / 10k-split-c1 4m 0 — — — missing
perf-01 / 10k-split-c1 32m 0 — — — missing
perf-01 / 10k-split-c1 128m 0 — — — missing
perf-01 / 10k-split-c1 1g 0 — — — missing
perf-01 / 10k-split-c4 4k 0 — — — missing
perf-01 / 10k-split-c4 256k 0 — — — missing
perf-01 / 10k-split-c4 4m 0 — — — missing
perf-01 / 10k-split-c4 32m 0 — — — missing
perf-01 / 10k-split-c4 128m 0 — — — missing
perf-01 / 10k-split-c4 1g 0 — — — missing
perf-02 / 10k-ssd-c1 4k 5 132.30 75.83 136.04 FAIL
perf-02 / 10k-ssd-c1 256k 5 132.51 82.56 102.96 PASS
perf-02 / 10k-ssd-c1 4m 5 116.95 89.96 125.69 FAIL
perf-02 / 10k-ssd-c1 32m 5 104.26 96.99 103.59 PASS
perf-02 / 10k-ssd-c1 128m 5 101.71 93.36 84.50 PASS
perf-02 / 10k-ssd-c1 1g 0 — — — missing
perf-02 / 10k-ssd-c4 4k 5 115.84 94.26 196.90 FAIL
perf-02 / 10k-ssd-c4 256k 5 113.23 97.84 165.05 FAIL
perf-02 / 10k-ssd-c4 4m 5 104.48 99.56 114.89 FAIL
perf-02 / 10k-ssd-c4 32m 5 89.34 109.17 171.49 FAIL
perf-02 / 10k-ssd-c4 128m 5 105.42 104.28 108.54 FAIL
perf-02 / 10k-ssd-c4 1g 0 — — — missing
perf-02 / 100k-split-c1 4k 5 145.43 72.58 106.36 FAIL
perf-02 / 100k-split-c1 256k 5 143.26 71.42 99.99 PASS
perf-02 / 100k-split-c1 4m 5 124.31 79.25 91.11 PASS
perf-02 / 100k-split-c1 32m 5 104.76 84.80 87.04 PASS
perf-02 / 100k-split-c1 128m 5 100.95 96.35 94.88 PASS
perf-02 / 100k-split-c1 1g 1 91.43 108.81 124.36 partial, fails observed
perf-02 / 100k-split-c4 4k 0 — — — missing
perf-02 / 100k-split-c4 256k 0 — — — missing
perf-02 / 100k-split-c4 4m 0 — — — missing
perf-02 / 100k-split-c4 32m 0 — — — missing
perf-02 / 100k-split-c4 128m 0 — — — missing
perf-02 / 100k-split-c4 1g 0 — — — missing
perf-02 / 100k-ssd-c1 4k 0 — — — missing
perf-02 / 100k-ssd-c1 256k 0 — — — missing
perf-02 / 100k-ssd-c1 4m 0 — — — missing
perf-02 / 100k-ssd-c1 32m 0 — — — missing
perf-02 / 100k-ssd-c1 128m 0 — — — missing
perf-02 / 100k-ssd-c1 1g 0 — — — missing
perf-02 / 100k-ssd-c4 4k 0 — — — missing
perf-02 / 100k-ssd-c4 256k 0 — — — missing
perf-02 / 100k-ssd-c4 4m 0 — — — missing
perf-02 / 100k-ssd-c4 32m 0 — — — missing
perf-02 / 100k-ssd-c4 128m 0 — — — missing
perf-02 / 100k-ssd-c4 1g 0 — — — missing

Cell counts: {'PASS': 20, 'FAIL': 18, 'partial, fails observed': 2, 'missing': 32}.

Invalid result failures (not latency gates):

  • 20 × warm run 1: upload failed: 0 null
  • 1 × warm run 1: upload failed: 500 {"error":{"code":"internal","message":"Something went wrong on our side. Trying again might help.","request_id":"01a196c9-92a5-7698-b6d5-726044511d31","retryable":true}}

job disposition and limits

Both fix759b-exact-matrix-perf01.service and fix759b-exact-matrix-perf02.service were stopped using systemctl --user stop, including their service process trees. No pattern kills. perf-02 had only 3.3 GB available at takeover (below the brief's 15 GB floor); continuing measurements was not acceptable. perf-01 was still measuring useful warm data, but had >120 invocations left; each takes at least 60 seconds, plus fixture/indexing/setup, so it could not finish within the authorized approximately two-hour bound. No replacement matrix started.

Collected current matrix JSON: perf-01 236 without measurement failures; perf-02 152 without measurement failures and 21 failed. All completed/failed JSON, MD and small logs were retained. Two earlier perf-01 startup failures under pre-claude-path-invalid are separately retained, never counted. 20 fully paired cells pass; 18 fully paired cells fail; two partial cells already show failing medians; 32 cells have no matched pair. A partial cell is not an accepted five-pair result. A successful driver exit is not the paired relative performance gate.

The matrix was intended to prove #771's warm upload and overlapping note-save performance on 1k/10k/100k Homes, split versus all-SSD storage, 1/4 upload clients, six upload sizes, five alternating matched pairs, at fixed seed 75931004. It does not prove the full acceptance plan: cold-cache pairs (--cold-runs 0), metadata-byte/syscall/flush evidence, and the remaining broader native/recovery gaps remain missing. Earlier PR status is superseded by the retained table above. Most observed relative failures are save-ack p95; failed transport/HTTP runs are excluded rather than treated as passing latency samples.

Results remain on each VM in ~/perf-results/759b-matrix/perf0{1,2}/results and ~/perf-reports-kept/takeover-20261101/759b-perf0{1,2}/results; control collection: /home/kayg/perf-reports/takeover-20261101/perf0{1,2}/. Seeded Home/data/state/work cleanup is in progress separately; small reports are preserved. Both shared locks were released by these units, allowing #792 qualification to start. Verdict: incomplete and performance gates fail; not qualified, no merge.

Final cleanup verification

Both 759b units stopped; quota r4 had already exited. All takeover cleanup and reproduction processes have exited. Earlier r2 cleanup completed untouched. Final free: perf-01 root 51G, split Home 58G; perf-02 root 37G, split Home 97G. Shared locks are held by #792 qualification (159697 / 1325428), not the stopped jobs. Cleanup/fixture-copy I/O overlapped the first #792 samples; those samples need overlap exclusion. Retained observations do not establish why quota-4 cache was reset.

Perf-02 cleanup and fixture-copy overlap was approximately 17:13–17:22 IST on 1 November (11:43–11:52 UTC). The earlier r2 file walk also overlapped #792 startup until it completed. #792 first overlapping samples must not be treated as uncontended measurements. No #792 process was stopped.

# 759b matrix takeover results Head `0d2d416e5ac13b39fb46f53e7a7af79f6d36b3b2`; baseline `133cf2c2`. Median of matched per-pair head/dev ratios. Gates: throughput >=95%, response p95 <=105%, overlapping save-ack p95 <=105%. Only full five-pair cells can pass acceptance. RSS is retained in raw JSON, with no additional RSS gate invented. | VM / Home-layout-clients | upload | valid matched pairs / 5 | throughput % | response p95 % | save-ack p95 % | verdict | |---|---|---:|---:|---:|---:|---| | perf-01 / 1k-split-c1 | 4k | 5 | 146.88 | 68.42 | 101.71 | PASS | | perf-01 / 1k-split-c1 | 256k | 5 | 144.95 | 72.59 | 96.27 | PASS | | perf-01 / 1k-split-c1 | 4m | 5 | 118.08 | 89.54 | 106.88 | FAIL | | perf-01 / 1k-split-c1 | 32m | 5 | 97.24 | 99.34 | 103.60 | PASS | | perf-01 / 1k-split-c1 | 128m | 5 | 100.99 | 100.44 | 111.39 | FAIL | | perf-01 / 1k-split-c1 | 1g | 5 | 100.61 | 95.00 | 105.43 | FAIL | | perf-01 / 1k-split-c4 | 4k | 5 | 137.30 | 71.27 | 87.82 | PASS | | perf-01 / 1k-split-c4 | 256k | 5 | 138.44 | 75.31 | 93.32 | PASS | | perf-01 / 1k-split-c4 | 4m | 5 | 122.12 | 75.46 | 83.73 | PASS | | perf-01 / 1k-split-c4 | 32m | 5 | 104.58 | 85.23 | 89.89 | PASS | | perf-01 / 1k-split-c4 | 128m | 5 | 100.01 | 104.62 | 79.83 | PASS | | perf-01 / 1k-split-c4 | 1g | 5 | 101.73 | 98.75 | 98.59 | PASS | | perf-01 / 1k-ssd-c1 | 4k | 5 | 127.76 | 83.54 | 144.09 | FAIL | | perf-01 / 1k-ssd-c1 | 256k | 5 | 122.31 | 84.18 | 140.70 | FAIL | | perf-01 / 1k-ssd-c1 | 4m | 5 | 115.11 | 90.95 | 117.69 | FAIL | | perf-01 / 1k-ssd-c1 | 32m | 5 | 102.36 | 97.43 | 88.31 | PASS | | perf-01 / 1k-ssd-c1 | 128m | 5 | 97.70 | 104.55 | 129.87 | FAIL | | perf-01 / 1k-ssd-c1 | 1g | 5 | 101.95 | 95.70 | 99.29 | PASS | | perf-01 / 1k-ssd-c4 | 4k | 5 | 116.30 | 97.80 | 146.52 | FAIL | | perf-01 / 1k-ssd-c4 | 256k | 5 | 114.18 | 91.44 | 199.91 | FAIL | | perf-01 / 1k-ssd-c4 | 4m | 5 | 105.34 | 128.52 | 148.67 | FAIL | | perf-01 / 1k-ssd-c4 | 32m | 5 | 103.79 | 88.37 | 95.53 | PASS | | perf-01 / 1k-ssd-c4 | 128m | 5 | 101.73 | 70.22 | 73.90 | PASS | | perf-01 / 1k-ssd-c4 | 1g | 3 | 76.55 | 139.07 | 126.37 | partial, fails observed | | perf-01 / 10k-split-c1 | 4k | 0 | — | — | — | missing | | perf-01 / 10k-split-c1 | 256k | 0 | — | — | — | missing | | perf-01 / 10k-split-c1 | 4m | 0 | — | — | — | missing | | perf-01 / 10k-split-c1 | 32m | 0 | — | — | — | missing | | perf-01 / 10k-split-c1 | 128m | 0 | — | — | — | missing | | perf-01 / 10k-split-c1 | 1g | 0 | — | — | — | missing | | perf-01 / 10k-split-c4 | 4k | 0 | — | — | — | missing | | perf-01 / 10k-split-c4 | 256k | 0 | — | — | — | missing | | perf-01 / 10k-split-c4 | 4m | 0 | — | — | — | missing | | perf-01 / 10k-split-c4 | 32m | 0 | — | — | — | missing | | perf-01 / 10k-split-c4 | 128m | 0 | — | — | — | missing | | perf-01 / 10k-split-c4 | 1g | 0 | — | — | — | missing | | perf-02 / 10k-ssd-c1 | 4k | 5 | 132.30 | 75.83 | 136.04 | FAIL | | perf-02 / 10k-ssd-c1 | 256k | 5 | 132.51 | 82.56 | 102.96 | PASS | | perf-02 / 10k-ssd-c1 | 4m | 5 | 116.95 | 89.96 | 125.69 | FAIL | | perf-02 / 10k-ssd-c1 | 32m | 5 | 104.26 | 96.99 | 103.59 | PASS | | perf-02 / 10k-ssd-c1 | 128m | 5 | 101.71 | 93.36 | 84.50 | PASS | | perf-02 / 10k-ssd-c1 | 1g | 0 | — | — | — | missing | | perf-02 / 10k-ssd-c4 | 4k | 5 | 115.84 | 94.26 | 196.90 | FAIL | | perf-02 / 10k-ssd-c4 | 256k | 5 | 113.23 | 97.84 | 165.05 | FAIL | | perf-02 / 10k-ssd-c4 | 4m | 5 | 104.48 | 99.56 | 114.89 | FAIL | | perf-02 / 10k-ssd-c4 | 32m | 5 | 89.34 | 109.17 | 171.49 | FAIL | | perf-02 / 10k-ssd-c4 | 128m | 5 | 105.42 | 104.28 | 108.54 | FAIL | | perf-02 / 10k-ssd-c4 | 1g | 0 | — | — | — | missing | | perf-02 / 100k-split-c1 | 4k | 5 | 145.43 | 72.58 | 106.36 | FAIL | | perf-02 / 100k-split-c1 | 256k | 5 | 143.26 | 71.42 | 99.99 | PASS | | perf-02 / 100k-split-c1 | 4m | 5 | 124.31 | 79.25 | 91.11 | PASS | | perf-02 / 100k-split-c1 | 32m | 5 | 104.76 | 84.80 | 87.04 | PASS | | perf-02 / 100k-split-c1 | 128m | 5 | 100.95 | 96.35 | 94.88 | PASS | | perf-02 / 100k-split-c1 | 1g | 1 | 91.43 | 108.81 | 124.36 | partial, fails observed | | perf-02 / 100k-split-c4 | 4k | 0 | — | — | — | missing | | perf-02 / 100k-split-c4 | 256k | 0 | — | — | — | missing | | perf-02 / 100k-split-c4 | 4m | 0 | — | — | — | missing | | perf-02 / 100k-split-c4 | 32m | 0 | — | — | — | missing | | perf-02 / 100k-split-c4 | 128m | 0 | — | — | — | missing | | perf-02 / 100k-split-c4 | 1g | 0 | — | — | — | missing | | perf-02 / 100k-ssd-c1 | 4k | 0 | — | — | — | missing | | perf-02 / 100k-ssd-c1 | 256k | 0 | — | — | — | missing | | perf-02 / 100k-ssd-c1 | 4m | 0 | — | — | — | missing | | perf-02 / 100k-ssd-c1 | 32m | 0 | — | — | — | missing | | perf-02 / 100k-ssd-c1 | 128m | 0 | — | — | — | missing | | perf-02 / 100k-ssd-c1 | 1g | 0 | — | — | — | missing | | perf-02 / 100k-ssd-c4 | 4k | 0 | — | — | — | missing | | perf-02 / 100k-ssd-c4 | 256k | 0 | — | — | — | missing | | perf-02 / 100k-ssd-c4 | 4m | 0 | — | — | — | missing | | perf-02 / 100k-ssd-c4 | 32m | 0 | — | — | — | missing | | perf-02 / 100k-ssd-c4 | 128m | 0 | — | — | — | missing | | perf-02 / 100k-ssd-c4 | 1g | 0 | — | — | — | missing | Cell counts: {'PASS': 20, 'FAIL': 18, 'partial, fails observed': 2, 'missing': 32}. Invalid result failures (not latency gates): - 20 × warm run 1: upload failed: 0 null - 1 × warm run 1: upload failed: 500 {"error":{"code":"internal","message":"Something went wrong on our side. Trying again might help.","request_id":"01a196c9-92a5-7698-b6d5-726044511d31","retryable":true}} ## job disposition and limits Both `fix759b-exact-matrix-perf01.service` and `fix759b-exact-matrix-perf02.service` were stopped using `systemctl --user stop`, including their service process trees. No pattern kills. perf-02 had only 3.3 GB available at takeover (below the brief's 15 GB floor); continuing measurements was not acceptable. perf-01 was still measuring useful warm data, but had >120 invocations left; each takes at least 60 seconds, plus fixture/indexing/setup, so it could not finish within the authorized approximately two-hour bound. No replacement matrix started. Collected current matrix JSON: perf-01 236 without measurement failures; perf-02 152 without measurement failures and 21 failed. All completed/failed JSON, MD and small logs were retained. Two earlier perf-01 startup failures under `pre-claude-path-invalid` are separately retained, never counted. 20 fully paired cells pass; 18 fully paired cells fail; two partial cells already show failing medians; 32 cells have no matched pair. A partial cell is not an accepted five-pair result. A successful driver exit is not the paired relative performance gate. The matrix was intended to prove #771's warm upload and overlapping note-save performance on 1k/10k/100k Homes, split versus all-SSD storage, 1/4 upload clients, six upload sizes, five alternating matched pairs, at fixed seed 75931004. It does not prove the full acceptance plan: cold-cache pairs (`--cold-runs 0`), metadata-byte/syscall/flush evidence, and the remaining broader native/recovery gaps remain missing. Earlier PR status is superseded by the retained table above. Most observed relative failures are save-ack p95; failed transport/HTTP runs are excluded rather than treated as passing latency samples. Results remain on each VM in `~/perf-results/759b-matrix/perf0{1,2}/results` and `~/perf-reports-kept/takeover-20261101/759b-perf0{1,2}/results`; control collection: `/home/kayg/perf-reports/takeover-20261101/perf0{1,2}/`. Seeded Home/data/state/work cleanup is in progress separately; small reports are preserved. Both shared locks were released by these units, allowing #792 qualification to start. Verdict: incomplete and performance gates fail; not qualified, no merge. ### Final cleanup verification Both 759b units stopped; quota r4 had already exited. All takeover cleanup and reproduction processes have exited. Earlier r2 cleanup completed untouched. Final free: perf-01 root 51G, split Home 58G; perf-02 root 37G, split Home 97G. Shared locks are held by #792 qualification (159697 / 1325428), not the stopped jobs. Cleanup/fixture-copy I/O overlapped the first #792 samples; those samples need overlap exclusion. Retained observations do not establish why quota-4 cache was reset. Perf-02 cleanup and fixture-copy overlap was approximately **17:13–17:22 IST on 1 November** (11:43–11:52 UTC). The earlier r2 file walk also overlapped #792 startup until it completed. #792 first overlapping samples must not be treated as uncontended measurements. No #792 process was stopped.
kayg commented 2026-11-01 16:52:13 +00:00
Author
Owner
Copy link

performance diagnosis of the 18 complete failures

Scope: the frozen table in 72886, head 0d2d416e, dev baseline 133cf2c2. Recomputed all five matched ratios from retained JSON. All 18 fail save-ack p95; two also fail response p95, one also fails throughput. The other 34 partial/missing cells remain unqualified.

group cells likely cause evidence against the same dev cells
small uploads, all-SSD (11) 1k/c1 and 1k/c4: 4KiB, 256KiB, 4MiB; 10k/c1: 4KiB, 4MiB; 10k/c4: 4KiB, 256KiB, 4MiB Shared-device flush contention as faster uploads increase completion/metadata pressure. Inherited attachment-index connection churn is a measured hotspot; the branch-specific contribution remains unisolated. Save p95 114.89–199.91% of matched dev. 1k/c4/256KiB: median dev/head p95 22.18/42.98ms; all five paired ratios fail (152.9–275.5%). 1k/c4/4MiB also has response p95 128.52%.
larger uploads, all-SSD (2) 128MiB at 1k/c1 and 10k/c4 Storage/history interference is plausible; large-upload pacing and publication need separate attribution. Save p95 129.87%, 108.54%; throughput and response pass. 10k/c4 paired signs vary, and absolute median head p95 is lower despite the failing median paired ratio.
mixed large-upload failure (1) 10k/SSD/c4/32MiB Broader contention/outlier, not explained solely by small-file metadata work. Throughput 89.34%, response p95 109.17%, save p95 171.49%; save paired ratios range 66.5–556.7%. No isolated cause established.
near-limit split-storage saves (4) 1k/c1: 4MiB,128MiB,1GiB; 100k/c1: 4KiB Tail variance or smaller history/scheduling interference; insufficient evidence to assign the all-SSD cause. Save p95 105.43–111.39%; throughput/response pass. Paired signs vary; 1k/4MiB absolute median dev/head p95 is 21.83/21.62ms. These still fail the specified paired gate.

dev comparison: these are relative head/dev budgets (throughput ≥95%, response/save p95 ≤105%), not absolute budgets dev can itself miss. The retained all-SSD regressions cannot be dismissed as “dev also fails.” The underlying flush hotspot does exist on dev. There is no demonstrated single cause for all 18.

dev-01 diagnosis: ran 1k-ssd-c4/256KiB, seed 75931004, 60s/version, four clients and 60 overlapping saves, through ~/bin/slot and Nix; repeated with strace -f -tt -T -yy for sync/open/close/rename/locking. Reused dev 133cf2c2 and head 0f38de31 release binaries built on dev-01; head has equivalent release behavior to 0d2d416e (subsequent tests/constant visibility only). The workload reproduced, the regression did not: plain dev/head save p95 49.02/38.51ms, throughput 2.561/3.295MiB/s. This shared-host single pair cannot override the perf matrix.

The trace finds about four attachment-index syncs/upload on both versions (two WAL, one database, one directory). Fresh Index::open connections in already_uploaded/place close on the hot path, causing checkpoint/close flushes despite WAL/NORMAL. Warm history-WAL sync p95 reached 125.95/75.52ms; tracing inflated save p95 to 157.30/92.87ms. This establishes inherited flush amplification/history stalls, not causal proof of the branch regression. #788 removes pending-record syncs but adds private-operation/source-parent work and changes upload completion pressure.

recommendation: fix overlapping-save performance before accepting #788; retain the budgets. Start with attachment-index connection reuse/checkpoint scheduling in shared code, preferably land that fix on dev first and rebase #788; then isolate upload fairness and source-parent publication work. Preserve durability barriers. The mixed 32MiB cell and near-limit split cells need separate uncontended paired evidence. No evidence supports raising the 5% budget.

Committed diagnosis, exact commands and summaries. No perf-VM measurement, build, lock acquisition, merge or deployment; no diagnostic jobs remain running. Seeded diagnostic directories and raw traces were removed; small reports retained.

## performance diagnosis of the 18 complete failures Scope: the frozen table in [72886](https://git.kayg.org/kayg/calternal/pulls/788#issuecomment-72886), head `0d2d416e`, dev baseline `133cf2c2`. Recomputed all five matched ratios from retained JSON. All 18 fail save-ack p95; two also fail response p95, one also fails throughput. The other 34 partial/missing cells remain unqualified. | group | cells | likely cause | evidence against the same dev cells | |---|---|---|---| | small uploads, all-SSD (11) | 1k/c1 and 1k/c4: 4KiB, 256KiB, 4MiB; 10k/c1: 4KiB, 4MiB; 10k/c4: 4KiB, 256KiB, 4MiB | Shared-device flush contention as faster uploads increase completion/metadata pressure. Inherited attachment-index connection churn is a measured hotspot; the branch-specific contribution remains unisolated. | Save p95 114.89–199.91% of matched dev. 1k/c4/256KiB: median dev/head p95 22.18/42.98ms; all five paired ratios fail (152.9–275.5%). 1k/c4/4MiB also has response p95 128.52%. | | larger uploads, all-SSD (2) | 128MiB at 1k/c1 and 10k/c4 | Storage/history interference is plausible; large-upload pacing and publication need separate attribution. | Save p95 129.87%, 108.54%; throughput and response pass. 10k/c4 paired signs vary, and absolute median head p95 is lower despite the failing median paired ratio. | | mixed large-upload failure (1) | 10k/SSD/c4/32MiB | Broader contention/outlier, not explained solely by small-file metadata work. | Throughput 89.34%, response p95 109.17%, save p95 171.49%; save paired ratios range 66.5–556.7%. No isolated cause established. | | near-limit split-storage saves (4) | 1k/c1: 4MiB,128MiB,1GiB; 100k/c1: 4KiB | Tail variance or smaller history/scheduling interference; insufficient evidence to assign the all-SSD cause. | Save p95 105.43–111.39%; throughput/response pass. Paired signs vary; 1k/4MiB absolute median dev/head p95 is 21.83/21.62ms. These still fail the specified paired gate. | **dev comparison:** these are relative head/dev budgets (throughput ≥95%, response/save p95 ≤105%), not absolute budgets dev can itself miss. The retained all-SSD regressions cannot be dismissed as “dev also fails.” The underlying flush hotspot does exist on dev. There is no demonstrated single cause for all 18. **dev-01 diagnosis:** ran `1k-ssd-c4/256KiB`, seed 75931004, 60s/version, four clients and 60 overlapping saves, through `~/bin/slot` and Nix; repeated with `strace -f -tt -T -yy` for sync/open/close/rename/locking. Reused dev `133cf2c2` and head `0f38de31` release binaries built on dev-01; head has equivalent release behavior to `0d2d416e` (subsequent tests/constant visibility only). **The workload reproduced, the regression did not:** plain dev/head save p95 49.02/38.51ms, throughput 2.561/3.295MiB/s. This shared-host single pair cannot override the perf matrix. The trace finds about **four attachment-index syncs/upload on both versions** (two WAL, one database, one directory). Fresh `Index::open` connections in `already_uploaded`/`place` close on the hot path, causing checkpoint/close flushes despite WAL/NORMAL. Warm history-WAL sync p95 reached 125.95/75.52ms; tracing inflated save p95 to 157.30/92.87ms. This establishes inherited flush amplification/history stalls, **not causal proof of the branch regression**. #788 removes pending-record syncs but adds private-operation/source-parent work and changes upload completion pressure. **recommendation: fix overlapping-save performance before accepting #788; retain the budgets.** Start with attachment-index connection reuse/checkpoint scheduling in shared code, preferably land that fix on dev first and rebase #788; then isolate upload fairness and source-parent publication work. Preserve durability barriers. The mixed 32MiB cell and near-limit split cells need separate uncontended paired evidence. No evidence supports raising the 5% budget. [Committed diagnosis, exact commands and summaries](https://git.kayg.org/kayg/calternal/src/commit/8abf9472/research/788-diagnosis). No perf-VM measurement, build, lock acquisition, merge or deployment; no diagnostic jobs remain running. Seeded diagnostic directories and raw traces were removed; small reports retained.
kayg commented 2026-11-01 18:05:37 +00:00
Author
Owner
Copy link

Attachment-index flush fix and measurements are in PR #831.

On current dev, the traced upload run measured 4.017 attachment-index sync calls per uploaded attachment before the fix and 1.017 after. The Linux syscall test now asserts zero index syncs for a duplicate/no-change save and one durable WAL sync for one new attachment. Dev-01 save-ack p95 moved from 56.26 ms to 40.62 ms.

On perf-01, five alternating matched pairs for the small all-SSD 1k/c4/256 KiB cell gave median save-ack p50/p95 of 11.70/18.94 ms on current dev and 12.20/18.33 ms with the fix. The median paired save-ack p95 ratio was 98.8%, within the existing 105% limit; upload response p95 ratio was 68.0% and throughput ratio 148.9%. No budgets changed. This covers one cell; the complete 18-cell matrix still needs requalification after #788 is rebased.

Attachment-index flush fix and measurements are in [PR #831](https://git.kayg.org/kayg/calternal/pulls/831). On current dev, the traced upload run measured 4.017 attachment-index sync calls per uploaded attachment before the fix and 1.017 after. The Linux syscall test now asserts zero index syncs for a duplicate/no-change save and one durable WAL sync for one new attachment. Dev-01 save-ack p95 moved from 56.26 ms to 40.62 ms. On perf-01, five alternating matched pairs for the small all-SSD 1k/c4/256 KiB cell gave median save-ack p50/p95 of 11.70/18.94 ms on current dev and 12.20/18.33 ms with the fix. The median paired save-ack p95 ratio was 98.8%, within the existing 105% limit; upload response p95 ratio was 68.0% and throughput ratio 148.9%. No budgets changed. This covers one cell; the complete 18-cell matrix still needs requalification after #788 is rebased.
kayg commented 2026-11-07 20:32:11 +00:00
Author
Owner
Copy link

housekeeping #1009: rebased+failing, left open for review.

rebased private staging while preserving dev cached attachment index, request/action-key scope, legacy receipts and publication_receipts. reconciled partial move handling and retained current deploy guide plus offline upgrade notes. fixed boxed placement failures, request borrows and recovery Caller credentials; applied VM formatting. scoped Linux record-lock fixtures at module level so macOS imports stay valid. retained both legacy and action-scoped receipt pins; repaired three recovery fault injections to use dev’s scoped action keys.

head: 1ed91a0057a47ca2b125cdeef05a3f0b6cada2fe; based on dev cd7f84432febfc63b788fbb9b9f9950452982d74, pushed with a recorded exact force-with-lease.

frontend frozen install, check, lint, build and unit tests passed. bundle gate passed. first-paint brotli vs dev: JS 230777 B (+0 B), CSS 18386 B (+0 B).

Rust gate results (Mac VM):

exit command
0 cargo fmt -p calternal-attachments -p calternal-auth -p calternal -p calternal-engine -p calternal-fsio -p calternal-perf -p calternal-server -p calternal-store -- --check
0 cargo check -j4 --locked -p calternal --bins
101 cargo check -j4 --locked -p calternal-attachments -p calternal-auth -p calternal -p calternal-engine -p calternal-fsio -p calternal-perf -p calternal-server -p calternal-store --all-targets
101 cargo clippy -j4 --locked -p calternal-attachments -p calternal-auth -p calternal -p calternal-engine -p calternal-fsio -p calternal-perf -p calternal-server -p calternal-store --all-targets -- -D warnings
101 cargo test -j4 --locked -p calternal-attachments -p calternal-auth -p calternal -p calternal-engine -p calternal-fsio -p calternal-perf -p calternal-server -p calternal-store
0 cargo check -j4 --locked -p calternal-fsio -p calternal-attachments --all-targets
0 cargo clippy -j4 --locked -p calternal-fsio -p calternal-attachments --all-targets -- -D warnings
0 cargo test -j4 --locked -p calternal-fsio -p calternal-attachments --lib

server-wide all-target checks/tests are blocked on macOS by storage_startup.rs:147 borrowing Child immutably; clippy also reports unused state in storage.rs:265. both reproduce on dev. component fsio/attachment check, clippy and unit tests passed after repairing action-scoped receipt pins and recovery fault keys; the earlier three recovery failures are resolved.

all executed gates used the VM Nix environment, shared Mac lock and slot. exact commands, source heads, logs and dev baseline. frontend inputs are identical to the tested frontend tree when a Rust-only repair changed the head.

original-head cherry/patch inventory, pr-788.md; unmatched entries are missing-patch candidates, with semantic overlap/replacements noted above.

housekeeping #1009: rebased+failing, left open for review. rebased private staging while preserving dev cached attachment index, request/action-key scope, legacy receipts and publication_receipts. reconciled partial move handling and retained current deploy guide plus offline upgrade notes. fixed boxed placement failures, request borrows and recovery Caller credentials; applied VM formatting. scoped Linux record-lock fixtures at module level so macOS imports stay valid. retained both legacy and action-scoped receipt pins; repaired three recovery fault injections to use dev’s scoped action keys. head: `1ed91a0057a47ca2b125cdeef05a3f0b6cada2fe`; based on dev `cd7f84432febfc63b788fbb9b9f9950452982d74`, pushed with a recorded exact force-with-lease. frontend frozen install, check, lint, build and unit tests passed. bundle gate passed. first-paint brotli vs dev: JS 230777 B (+0 B), CSS 18386 B (+0 B). Rust gate results (Mac VM): | exit | command | |---|---| | 0 | `cargo fmt -p calternal-attachments -p calternal-auth -p calternal -p calternal-engine -p calternal-fsio -p calternal-perf -p calternal-server -p calternal-store -- --check` | | 0 | `cargo check -j4 --locked -p calternal --bins` | | 101 | `cargo check -j4 --locked -p calternal-attachments -p calternal-auth -p calternal -p calternal-engine -p calternal-fsio -p calternal-perf -p calternal-server -p calternal-store --all-targets` | | 101 | `cargo clippy -j4 --locked -p calternal-attachments -p calternal-auth -p calternal -p calternal-engine -p calternal-fsio -p calternal-perf -p calternal-server -p calternal-store --all-targets -- -D warnings` | | 101 | `cargo test -j4 --locked -p calternal-attachments -p calternal-auth -p calternal -p calternal-engine -p calternal-fsio -p calternal-perf -p calternal-server -p calternal-store` | | 0 | `cargo check -j4 --locked -p calternal-fsio -p calternal-attachments --all-targets` | | 0 | `cargo clippy -j4 --locked -p calternal-fsio -p calternal-attachments --all-targets -- -D warnings` | | 0 | `cargo test -j4 --locked -p calternal-fsio -p calternal-attachments --lib` | server-wide all-target checks/tests are blocked on macOS by `storage_startup.rs:147` borrowing Child immutably; clippy also reports unused `state` in `storage.rs:265`. both reproduce on dev. component fsio/attachment check, clippy and unit tests passed after repairing action-scoped receipt pins and recovery fault keys; the earlier three recovery failures are resolved. all executed gates used the VM Nix environment, shared Mac lock and slot. [exact commands, source heads, logs and dev baseline](https://git.kayg.org/attachments/e13cb717-09a7-47b8-928a-bad59c124807). frontend inputs are identical to the tested frontend tree when a Rust-only repair changed the head. [original-head cherry/patch inventory](https://git.kayg.org/attachments/e05a38dc-c028-4d77-8655-efa9d5c8ea50), `pr-788.md`; unmatched entries are missing-patch candidates, with semantic overlap/replacements noted above.
Some checks failed
CI / server (pull_request) Has been cancelled
Details
CI / rust (pull_request) Has been cancelled
Details
CI / web (pull_request) Has been cancelled
Details
This pull request has changes conflicting with the target branch.
  • crates/attachments/src/actions.rs
  • crates/attachments/src/lib.rs
  • crates/attachments/src/place.rs
  • crates/attachments/src/tests/mod.rs
  • crates/store/src/writer.rs
View command line instructions

Manual merge helper

Use this merge commit message when completing the merge manually.

Checkout

From your project repository, check out a new branch and test the changes.
git fetch -u origin fix/private-staging-759b-r2:fix/private-staging-759b-r2
git switch fix/private-staging-759b-r2

Merge

Merge the changes and update on Forgejo.

Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.

git switch dev
git merge --no-ff fix/private-staging-759b-r2
git switch fix/private-staging-759b-r2
git rebase dev
git switch dev
git merge --ff-only fix/private-staging-759b-r2
git switch fix/private-staging-759b-r2
git rebase dev
git switch dev
git merge --no-ff fix/private-staging-759b-r2
git switch dev
git merge --squash fix/private-staging-759b-r2
git switch dev
git merge --ff-only fix/private-staging-759b-r2
git switch dev
git merge fix/private-staging-759b-r2
git push origin dev
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
backend
Rust server

  
design
Mockups, UI and TASTE

  
docs
ADRs, CONTEXT, TASTE

  
idea
Parked idea, never worked until promoted

  
infra
Deploy, CI, VMs

  
perf
Performance and budgets

  
reliability
Data integrity, corruption traps

  
web
Web client

No labels
backend
design
docs
idea
infra
perf
reliability
web
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
kayg/calternal!788
No description provided.