Skip to content

refactor(storage): remove the unused tus replication path - #529

Open
rickyrombo wants to merge 1 commit into
mainfrom
mjp-remove-tus-replication
Open

refactor(storage): remove the unused tus replication path#529
rickyrombo wants to merge 1 commit into
mainfrom
mjp-remove-tus-replication

Conversation

@rickyrombo

Copy link
Copy Markdown
Contributor

Removes the tus-based blob replication path. It was never reachable: replicateToHost's only reference was a commented-out line behind a // TODO: Replicate with TUSD, so no node has ever sent a tus replication upload and the receiving branch has never run in production.

Why remove rather than finish it

Tusd notifies completion on a channel after the response is sent, so the receiver cannot tell the sender that it rejected the blob — full disk, cid mismatch, failed bucket write all just log and return. A sender that treated the stream's success as replication success would record a mirror for a blob the peer never stored, and findMissedReplications would not re-queue it, because the mirror count already meets target. Silent under-replication is worse than a visible failure.

Its checks are also ordered the wrong way round. The pull path answers already_present or "no space" before any payload moves; the tus path stages the whole file to /tmp/tusd-uploads first and decides afterward. There is no expiration or GC configured either, so an aborted transfer would leave a partial file behind indefinitely.

Two existing paths already cover this — /internal/blobs/pull and the multipart /internal/blobs push — and both report their outcome synchronously.

Scope

  • Removes replicateToHost, tusAuthTransport, and the isReplication branches in validateTusUploadBeforeCreate, handleTusdUploadCreated, and handleTusdUploadComplete.
  • Tus itself stays — it is the resumable path for user uploads, which is what it is for. The tusgo client stays too; pkg/sdk uses it.
  • No behavior change for any deployed node, since nothing ever sent these requests.

Follow-ups

Part of a three-PR sequence from a review of replication:

  1. this PR
  2. fix the peer HTTP timeout so it bounds stalls rather than duration — a 3-hour mix cannot transfer inside the current 3 minute whole-request timeout, so long content is currently unreplicable and burns bandwidth retrying
  3. make pull the default by dropping the BlobStorageStreaming gate in replicateStoredFileToHost, leaving multipart push as the version-skew fallback

Still open after those: chunked ranged pull plus an async handoff, so a single large transfer cannot occupy a replication worker for tens of minutes. If large-blob replication ever wants resumability again, the right shape is PreFinishResponseCallback — it runs before the response, so the sender gets a truthful status — not the post-finish channel this PR removes.

Testing

go build ./... clean. go test ./pkg/mediorum/server/... shows the same three failures as unmodified main (TestPollDelistStatuses, TestRepair, TestUploadFile) and no new ones.

🤖 Generated with Claude Code

replicateToHost was never called -- the only reference was a commented
line behind a TODO -- so no node has ever sent a tus replication upload,
and the receiving branch it fed has never run in production.

Leaving it in place was the liability. It is a third way to write blobs
into the bucket, and structurally the weakest of the three: tusd notifies
completion on a channel after the response is sent, so the receiver
cannot report a rejection -- a full disk, a cid mismatch, a failed bucket
write -- to the sender. A sender that trusted the stream's success would
have recorded a mirror for a blob the peer never stored, and the sweep
would not re-queue it because the mirror count already met target. Its
checks also run after the payload is staged on disk, where the pull and
multipart paths decide before any bytes move.

Only the replication branch goes. Tus stays for user uploads, which is
what it is for, and the tusgo client remains in the sdk.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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