Skip to content

fix(core): finish a push after a key change, keep waits at hand-over - #2255

Merged
aicayzer merged 12 commits into
mainfrom
fix/099-push-rebuild
Oct 21, 2026
Merged

aicayzer merged 12 commits into
mainfrom
fix/099-push-rebuild

Conversation

@aicayzer

@aicayzer aicayzer commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Summary

After a folder's key is replaced or narrowed, the server's read view for that key changes. The first marfa folders push after the change stopped with copy_expired (read_view_changed) once its first write was read back, leaving every write behind it waiting until the person ran another push. A push, and a send-only run, now hydrates the copy again and drains once more in the same run, so the waiting writes go out, each once.

Separately, a folder watch forgot a server's Retry-After wait when it handed its folder to a command and took it back, so it sent again at once and was refused again. The wait is now kept in the folder's store by every drain of the folder, the watch's or a command's, and a watch that takes the folder back waits out the most recent one.

Changes

  • Folder::sync and Folder::send_only drain through a new drain_rebuilding: where a drain ends copy_expired with the reason read_view_changed, it hydrates and drains once more. Every drain of a push goes through it, the drain after the pull included. Only one rebuild per run: a later change ends the run copy_expired. A failed hydration ends the run with its own error and exit code. Waiting writes stay queued in both cases, and idempotency keys are kept, so nothing is sent twice.
  • A drain that ends in an error now keeps what it settled before the error. The core drain returns that partial report with the error (drain::Cut), and the folder drain remembers the placement refusals in it and holds the refused files before it returns. The rebuild adds the partial report to the report of the drain after it, so the push's report and exit code include every verdict from both drains.
  • settle_placements is split: remember_refusals reads only the report, and runs on the partial report too; giving way, which reads the server, runs only after a drain that ended well.
  • SyncReport and SendReport carry rebuilt, the hydration taken, and folders push reports it as rebuilt in JSON and as one line of text.
  • A folder watch already recovered from a changed read view through its follower, which hydrates on copy_expired. That path is unchanged and now has a regression test.
  • Each folder drain records the server's wait in the store (folder_send_again_at, wall-clock milliseconds), replaces it with a later answer, and clears it when a later drain has writes taken, or leaves writes undelivered, with no wait. A watch starts with the kept wait, capped at the 300-second most a drain keeps, so a wall clock set back cannot hold it.
  • A verdict committed before the read it owes is now reported when that read ends the drain, as a changed read view does. Before, an accepted write or a refused write whose read-back was refused 409 read_view_changed was left out of the push's report, and a refused placement was not remembered, so the next push sent it again. The same holds for a write refused before it was sent whose read meets the change.
  • A refused placement is remembered even when the copy can no longer be read: the record reads the settings version the store holds, rather than going through the hydrated-copy guard that refused it with hydration_incomplete and replaced the drain's own copy_expired.
  • A verdict an earlier drain told is dropped from the report when a later drain of the same run withdraws its write (a placement that gave way), so the report names only writes still in the queue.
  • Where the drain after the pull rebuilds the copy, the push now pulls once more, so the directory shows what the new view holds in the same run, and drains again where that pull queued a placement. One rebuild per run still holds: a change at that last drain ends the run copy_expired.
  • Contract: folders/view-changed-push-pulls-again (new), folders/view-changed-push-rebuilds, folders/view-changed-push-once, folders/view-changed-rebuild-fails, folders/view-changed-watch-rebuilds and folders/watch-rate-limit-handover in conformance/spec/folders.md.
  • Device tests in folders.test.ts (a scripted server that refuses the read-back with 409 read_view_changed, after a refused create, and at the placement read after the pull) and folders-handover.test.ts (a wait named by the watch's own drain, and a later one named to a command's push). scriptFolderWrites takes a reading hook for the read door, and EdgeDoor one for the read of an edge by id.
  • core/README.md describes both behaviors.

Testing

  • The new device tests were seen failing on the binary built without the fix: the push and send-only push ended copy_expired with exit 4, the rebuild-failure test got copy_expired instead of server, and the hand-over tests measured gaps of about 2 seconds where 8 and 4 seconds were named. All pass with the fix, and passed three runs in a row.
  • Review fixes, each seen failing first:
    • A refused create before the view changed (Alpha refused 400 validation_error, Beta created, Beta's read-back refused 409 read_view_changed). Without the fix the push exited 0 and its report had no refusal for Alpha's create, while the queue held it refused. With the fix the report has Alpha's validation_error refusal, the exit is 0 (a refusal is a verdict, not a write left waiting), nothing is undelivered, and each create was sent once.
    • The drain after the pull meets 409 read_view_changed on the read of the first placement it sent. Without the fix the push ended copy_expired with exit 4. With the fix it rebuilds once, places both files with one POST /edges each, exits 0, and the next push sends nothing and does not rebuild.
    • A unit test for the kept wait: a later drain that leaves writes undelivered with no retry_after_seconds now clears the stored wait. It kept the older wait before the fix.
  • The watch regression test passes on both binaries, which is why the watch path is unchanged.
  • cargo fmt, cargo clippy -p marfa-core -p marfa-cli --all-targets -- -D warnings, and cargo test -p marfa-core -p marfa-cli all pass.
  • Device suites folders, folders-handover, folders-send-only, folders-watch-every and read-view pass against the built binary (170 tests).
  • The contract checks in conformance/src/utils/ (spec-form, spec-statements, spec-citations) and ci/core-contract.test.ts pass, tsc --noEmit passes for conformance, and Prettier is clean.
  • Round 2 review fixes, each seen failing first:
    • Unit tests in drain.rs: an accepted write and a refused write whose read-back is refused read_view_changed, and a write refused unsent whose read meets the change. Before the fix each reported answered 0 and no verdict; each has a witness run where the read is answered.
    • Device test: a placement refused 403 edge_permission_denied whose owed read is refused read_view_changed at the drain after the pull. On a binary without the verdict fix, the next push sent the placement again (two POST /edges). With the verdict fix but not the settings-version fix, the push ended hydration_incomplete. With both, the push exits 0, rebuilds, reports the refusal, and the next push sends no placement.
    • Unit test in folder/sync.rs: a verdict an earlier drain told for a write a later drain withdrew leaves the report. It stayed with the filter removed.
    • Device test: at the drain after the pull, the view changes so that the key reads a new note, Three, and no longer reads Two. Without the extra pull, Three.md was not written. With it, Three.md is written and placed in the same push, Two.md keeps its bytes, and the next push reports it lost and sends no create.
  • Not tested: a real server with a real key replacement (only a scripted server), the live suites (read-view-live, edge-move-live), and the Swift and Node bindings' folder tests. The bindings compile and get the same rebuild through the core, but they do not report rebuilt yet. A key replacement that also changes the key the refusal record names clears that record by design, so a refused placement is tried again under the new key; the scripted server keeps the key, so that case is not exercised here.

Notes for reviewers

  • On the extra pull: a file whose row the new key cannot read is not removed by it. A bound file pins its row, a hydration keeps a pin whose row the key cannot read (device/pin-gone-kept), and the pull leaves a row the copy lost for the scan to report (folders/lost-row-unchanged-held). So the visible effect of the extra pull is writing and placing what the new view holds in the same push; the unreadable item's file stays, as before.

  • Review depth: full, it rebuilds the folder's copy with writes waiting.

  • The rebuild is limited to read_view_changed. Other copy_expired reasons (instance_changed, a credential ended, an invalid view) still end the run, because rebuilding against another instance or without a credential is not safe to do unasked.

  • Known limit: where the rebuild or the drain after it fails, the run ends with that error, and the verdicts the first drain settled are not in any report. They are in the queue, the placement refusals among them are remembered, and the refused files are held.

  • Docs for the release pass: core/README.md now describes the rebuild and the kept wait, and folders push --json has a new rebuilt field.

Checklist

  • The title is a scoped Conventional Commit.
  • New behavior has a test that was seen failing first, or no test was needed and the reason is above.
  • Docs, comments and the contract still say what is true after this change.

@aicayzer aicayzer changed the title fix(core): rebuild the copy after a key change and keep waits across hand-over fix(core): finish a push after a key change, keep waits at hand-over Oct 21, 2026

@aicayzer aicayzer left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review: ready to merge

Summary. After a key change, a push now rebuilds the folder's copy once and finishes in the same run, sending each waiting write once and reporting every verdict; a watch keeps the server's wait across a hand-over. Reviewed in three rounds on the strongest model; safe to merge.

First draft: 3/5. 2 blocking issues and 1 smaller one in round 1; 1 blocking issue and 2 smaller ones in round 2; 1 blocking issue in round 3. All fixed.

Found.

  • Round 1, blocking: a drain cut short by the view change lost every verdict it had settled, so a refused create went unreported (reproduced: exit 0, no refusal). Fixed with drain::Cut in b513a5a01.
  • Round 1, blocking: the drain after the pull did not rebuild, against the new statement. Fixed in a9c4dbd45.
  • Round 1: a later drain with no Retry-After kept an older wait. Fixed in 11fdee7f8.
  • Round 2, blocking: a write whose own read-back met the change was sent once and never reported; a refused placement in that position was sent again by the next push. Fixed in 98a8ff24d and 7eac4cd28, which also stops the placement refusal record failing hydration_incomplete on an expired copy.
  • Round 2: a withdrawn verdict from the cut drain stayed in the report. Fixed in 235fa3950.
  • Round 2, session decision: after a rebuild at the drain after the pull, the push pulls once more, so the folder holds what the new view holds when the run ends. 4cf32efbb, statement folders/view-changed-push-pulls-again.
  • Round 3, blocking: the drain after that second pull ran even when it placed nothing. Fixed in bf5e21a8d.

Checked. Writes keep their idempotency keys across the rebuild; the rebuild runs under the command's writer role with no second writer; at most one rebuild per run, and a change after it ends the run copy_expired; the stored wait is capped and cannot hold a folder for good; absorb never drops a verdict whose write is still queued.

Not checked. A real server with a real key replacement, the live suites, and the Swift and Node bindings, which take the rebuild through the core and do not report rebuilt yet.

@aicayzer
aicayzer marked this pull request as ready for review October 21, 2026 10:15
@aicayzer
aicayzer enabled auto-merge (squash) October 21, 2026 10:15
@aicayzer
aicayzer merged commit aea27c7 into main Oct 21, 2026
84 of 87 checks passed
@aicayzer
aicayzer deleted the fix/099-push-rebuild branch October 21, 2026 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant