fix(core): stream the resumable-upload resume instead of buffering the whole file - #279
Merged
Conversation
…e whole file Incident 2026-08-14: quitting mid-upload of an 88 GB file and relaunching made the startup reconcile's resume path (resume_persisted) re-read the ENTIRE file into one Vec via read_hash_encrypt - >10 GB of RSS growth in ~30 s at disk speed until the OS killed the app, repeating on every launch because the pending op survives the kill. The live upload path was already streaming and bounded; only the restart-resume (and the adopt re-hash) still buffered. - resume_persisted: single streaming pass - hash every byte, discard the acked prefix, push the tail in bounded wire chunks (~2 in flight), with the legacy-hash cross-check still running before the final chunk can finalize the object. MemGauge-instrumented; pacer charged only for pushed bytes. - rehash_local_plaintext: hash-only streaming read (its old callee's plaintext arm buffered the body despite the comment claiming otherwise). - reconcile now DROPS pending upload ops for paths the user has since excluded (the incident file's folder had just been excluded; recovery would have kept uploading it). - Instrumentation so the next diagnostics zip self-diagnoses: INFO logs for cycle start (tick source), orchestrator state transitions, reconcile entry/resume decisions with sizes+offsets; an RSS watchdog task with a trailing sample window; pending_ops.txt + memory.txt in the diagnostic bundle (session URLs never included); redaction policy updated. Regression tests: the crash-mid-resumable e2e now asserts the resume routes through the gauge-instrumented streaming loop and peaks under 3 wire chunks; new tests for the excluded-op drop, the pending-ops bundle summary (URL non-leak), and the RSS sampler. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A7q3CvJzL4zZmDA9CbXyQQ
… deny) Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A7q3CvJzL4zZmDA9CbXyQQ
Contributor
Coverage
Gate: passed - no coverage regression (epsilon 0.1 pp). |
- Backend offset semantics: interpret every InProgress answer against the streaming window (offset == file position of acc[0]). A backward offset (driven-s3's one-shot post-hydration rewind-to-0; sftp/localfs backward resync) RESTARTS the pass from the store's stated offset (budget: one restart); a forward offset within the window drains exactly the newly covered bytes; beyond the window restarts; an exact stall abandons. Previously every S3 resume with prior progress was misread as a stall and abandoned to a full re-upload. - ResumeAcc RAII guard ties the MemGauge charge to the buffer so error unwinds (`?` on resume_chunk / persist_payload) refund in-flight bytes. - Legacy rows (uploaded_blake3_hex, no resume_identity) are verified by a hash-only prepass BEFORE any byte is pushed, restoring the buffered code's check-then-push ordering for rows whose only gate is the hash. - Cheap EOF identity re-fstat aborts a mid-stream-modified file before the finalizing chunk (the md5-vs-store check remains the correctness backstop). - Excluded-path handling redesigned (angleB): reconcile now SKIPS only the resume for a now-excluded path and lets adopt-or-requeue run, so a finalized object is adopted into a file_state row (never an invisible untracked orphan) and an unfinalized one requeues into nothing. - push_chunks (fresh-session path) gets the same no-progress stall guard. - Orchestrator transition log emits the state NAME only (the Error variant's free-text details could leak partially-redacted paths into bundles). - memlog: shared SystemClock for sample timestamps, Option<u64> log sentinel, spawn_blocking for the syscall, and snapshot() now reports staleness (memory.txt labels sample=fresh|stale-or-unread). - Cross-reference comments tie PendingOpPayload to the bundle summary's field list. New tests: one-shot rewind resumes to completion (red on the stall-guard misread), mid-resume RPC error leaves the gauge balanced (red without the RAII guard), excluded path skips resume with a never-called shim + control, finalized orphan on an excluded path is adopted not leaked (red on the previous drop-the-op design). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A7q3CvJzL4zZmDA9CbXyQQ
…ounting Lifts the Rust line coverage back over the regression gate (the hardening round added abandon branches only reachable through fault shims). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A7q3CvJzL4zZmDA9CbXyQQ
The ultra-review verifier confirmed the flag list silently omitted it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01A7q3CvJzL4zZmDA9CbXyQQ
This was referenced Aug 14, 2026
pmaxhogan
added a commit
that referenced
this pull request
Aug 14, 2026
🤖 I have created a release *beep* *boop* --- ## [2.10.1](v2.10.0...v2.10.1) (2026-08-14) ### Bug Fixes * **core:** stream the resumable-upload resume instead of buffering the whole file ([#279](#279)) ([adc81fa](adc81fa)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Incident (2026-08-14)
Quitting Driven mid-upload of an 88.6 GB file (
dev-drives/dev.vhdx) and relaunching made the app consume >10 GB of RAM in ~30 s (at disk read speed) until Windows killed it - and it repeated on every relaunch. Diagnostics zip + the laptop'spending_opsrow confirmed the mechanism:size=88655003648,acked_offset=7436500992), passed the resume-identity gate, and calledread_hash_encrypt- which buffers the entire upload body into oneVec. The live upload path has streamed with bounded channels since DESIGN s11.4.3; only the restart-resume path (and the adopt re-hash) still used the legacy buffered read.delete_pending_op, so the op survives and every launch retries the same 88 GB read.Fix
resume_persistednow streams via a pass-based design: one sequential read that hashes every byte, discards the already-acked prefix, and pushes the tail in bounded wire chunks (~2 in flight,ResumeAccRAII-guarded MemGauge accounting that stays balanced even on?error unwinds). The pacer is charged only for pushed wire bytes.offsetalways equals the file position of the accumulator's first byte): driven-s3's one-shot post-hydration rewind-to-0 and sftp/localfs resyncs RESTART the pass from the store's stated offset (one-restart budget); in-window forward offsets drain exactly the newly covered bytes; an exact stall abandons. Without this, every S3 resume with prior progress degraded to a full re-upload.rehash_local_plaintextstreams (hash-only): its old callee's plaintext arm buffered the whole body despite the comment claiming otherwise.file_staterow (never left as an invisible untracked orphan on the remote), an unfinalized one requeues into nothing (the next scan excludes the path; the abandoned session is GC'd).push_chunks(fresh-session uploads) gets the same no-progress stall guard the resume path has.Instrumentation (make the next diagnostics zip self-diagnosing)
driven::app::memlog): samples every 15 s on the blocking pool, logs on >=128 MiB movement, keeps a trailing ~15 min window.pending_ops.txt(op shapes, sizes, offsets, ages, recovery-flag presence; session URLs are capability secrets and are never included) andmemory.txt(current + peak RSS, asample=fresh|stale-or-unreadhonesty flag, and the trailing sample window). Redaction policy text updated in the same change.Review
An ultra multi-agent review ran against the first cut; its confirmed findings (S3 rewind misread, MemGauge leak on error unwind, excluded-op orphan gap, push-before-validate on legacy rows, state-transition Debug leak, plus several consistency nits) are fixed in the follow-up commit. Refuted findings (e.g. mid-stream prefix modification "undetected" - the full-stream md5 vs the store's stored md5 catches exactly that) are documented in code comments.
Tests
crash_mid_upload_resumes_persisted_session_byte_for_byteasserts MemGaugepeak > 0(proves the resume routes through the instrumented streaming loop) andpeak <= 3 * WIRE_CHUNK(boundedness).resume_honours_a_one_shot_rewind_to_zero(S3 contract; red on the stall-guard misread),resume_error_midstream_keeps_the_mem_gauge_balanced(red without the RAII guard),reconcile_skips_resuming_a_now_excluded_paths_session(+ control proving the gate),reconcile_adopts_finalized_orphan_even_when_path_now_excluded(red on a drop-the-op design),pending_ops_summary_names_resumable_sessions_without_urls, memlog sampler tests.README checked - no changes needed (it does not enumerate bundle contents, and no feature claims changed).
🤖 Generated with Claude Code
https://claude.ai/code/session_01A7q3CvJzL4zZmDA9CbXyQQ