Skip to content

Default TUS uploads back to a single remainder PATCH (chunking stays an opt-in knob)#114

Merged
morepriyam merged 1 commit into
mainfrom
fix/tus-single-patch-default
Jul 22, 2026
Merged

Default TUS uploads back to a single remainder PATCH (chunking stays an opt-in knob)#114
morepriyam merged 1 commit into
mainfrom
fix/tus-single-patch-default

Conversation

@morepriyam

Copy link
Copy Markdown
Collaborator

Relates to #89 — this deliberately reverses its direction. The always-chunked default from #108 is not feasible on device; this returns the app to the pre-#108 single-PATCH behavior while we wait for the server-side fix (mieweb/opensource-server#395).

Why the 32 MiB chunk default had to go

A real-world 250 MB merged upload (20 clips) crawled with ~13% progress jumps — which is literally the chunk size: 250 MB ÷ 32 MiB ≈ 8 chunks. Per chunk, the app paid four costs the old single-PATCH path never did:

  1. A full temp copy per chunkprepareChunkSource read 32 MiB + wrote 32 MiB to flash for every chunk of a multi-chunk file (~500 MB of extra serial disk I/O for that video). Pre-Send TUS uploads as bounded 32 MiB chunks #108, an offset-0 upload pointed the native task directly at the source file — zero copies.
  2. A fresh background-URLSession task per chunk — out-of-process nsurlsessiond startup/IPC, once per chunk instead of once per file.
  3. The edge's spool dead time × N — the proxy holds each request body until complete ([Bug]: Uploads are very slow through the agent reverse proxy (ModSecurity scanning request bodies) opensource-server#395); that end-of-body stall was paid per chunk instead of per file.
  4. The killer: screen lock froze the loop. The JS loop must run between chunks, and JS is suspended the moment the app backgrounds. A single PATCH survives lock as one native background transfer; chunked, the in-flight chunk finishes and then everything stalls until the next foreground or a BG resume wake (15-min OS floor). That's the "took forever" experience.

And the durability the chunks were buying is not needed as a permanent client workaround: the maintainer has confirmed the root cause (ModSecurity's connector unconditionally reads the whole body before proxy_pass) and a scoped modsecurity off fix is queued behind mieweb/opensource-server#407. Verified today the edge still buffers (killed a PATCH at ~30 MB → HEAD returns Upload-Offset: 0), so until that lands an interrupted upload resumes from 0 — an accepted, temporary trade-off.

What this PR does

  • chunkSizeBytes is unset by default → each PATCH carries the whole remainder (offset → EOF). A fresh upload is ONE PATCH: one native background transfer, streamed straight from the source file, no staging copy, survives lock/backgrounding. This is standard TUS practice — tus-js-client defaults chunkSize to Infinity; bounded chunks are only for body-buffering middleboxes.
  • Chunking stays fully intact as an explicit opt-in knob, with the 32 MiB benchmark table preserved in its doc (single PATCH 20.4 MB/s ≈ 32 MiB chunks 20.5 MB/s; 16 MiB 16.3; 8 MiB 10.1) so nobody tunes it downward again.
  • In-flight progress restored (removed by Send TUS uploads as bounded 32 MiB chunks #108): native task ticks are forwarded as offset + bytesSentThisAttempt (clamped to total), and each 204's server-acknowledged Upload-Offset re-anchors the bar to durable truth. After a failure, the re-HEAD's offset is reported as-is — the bar may honestly step back to the last durable byte.
  • Resume staging is now memory-bounded: the remainder temp copy streams through 8 MiB FileHandle reads into the temp file instead of one whole-remainder readBytes allocation — so returning to remainder-sized PATCHes does NOT reintroduce the OOM half of upload: OOM on large files — whole-file sha256 + whole-remainder resume copy #92 (the pre-Send TUS uploads as bounded 32 MiB chunks #108 code materialized the entire remainder in memory on every resume).
  • Offset discipline is unchanged: server offset is the sole source of truth, re-HEAD before any retry, advance only on the 204's Upload-Offset, 404-on-persisted-URL recreate, redirect rejection + origin pinning all preserved.

When mieweb/opensource-server#395's fix ships

Nothing more to do here — the edge streams bodies, so a killed single PATCH keeps every byte the server received and resume works from wherever it died. Fastest and durable, no client change needed.

Verification

  • npx tsc --noEmit clean
  • npx eslint src/features/upload/ clean
  • npx jest — 11 suites, 110 tests pass; tus-client suite now 22 tests including three new ones: remainder-by-default (a 100 MB file is exactly one PATCH), in-flight tick forwarding + 204 re-anchoring, and resumed-progress clamping (offset + sent never exceeds total)

…ing as a knob

Reverts the 32 MiB always-chunked default from #108 to the standard TUS
shape: one PATCH from the current offset to EOF. In practice the chunk
loop was what made big uploads crawl on device — a 32 MiB temp copy per
chunk, a fresh background-URLSession task per chunk, the edge's spool
dead time paid per chunk, and the JS loop freezing between chunks the
moment the screen locks (progress then creeps at <=32 MiB per rare BG
wake). A single PATCH is one native background transfer that streams
straight from the source file and survives lock/backgrounding.

chunkSizeBytes stays as an explicit opt-in escape hatch for deployments
behind a body-buffering edge, with the 32 MiB benchmark numbers kept in
its doc. Durability through the buffering proxy is deferred to the
server-side fix (mieweb/opensource-server#395).

Also:
- restore smooth in-flight progress: native task ticks are forwarded as
  offset + bytesSentThisAttempt (clamped), re-anchored on each 204's
  server-acknowledged Upload-Offset, and honestly stepped back to the
  re-HEAD offset after a failure
- resume staging is now memory-bounded: the remainder temp copy streams
  through 8 MiB FileHandle reads instead of one whole-remainder
  readBytes allocation, so remainder-sized PATCHes do not reintroduce
  the OOM half of #92

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@morepriyam
morepriyam merged commit 2c88930 into main Jul 22, 2026
4 checks passed
morepriyam added a commit that referenced this pull request Jul 22, 2026
Default TUS uploads back to a single remainder PATCH (chunking stays an opt-in knob)
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