Repository navigation
fix(storage): never commit a partial blob write - #612
Open
dylanjeffers wants to merge 1 commit into
Open
dylanjeffers wants to merge 1 commit into
dylanjeffers wants to merge 1 commit into
Conversation
gocloud's Writer.Close commits whatever was written; only cancelling the writer's context aborts. replicateToMyBucket and relocateBetweenBuckets closed the writer after a failed io.Copy, so a pull that died partway stored a truncated blob under the full CID. Route every bucket copy through copyToBucket, which cancels before closing on error. Qm CIDs can't be hash-checked, and repair, sniff and the serve fallback pull them unvalidated, so a short copy then spreads to every node. The chunked reader now fails when the byte count differs from the size the peer reported (Content-Range or Content-Length), and legacy pulls with no reported size ask the peer's blob info first. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rickyrombo
approved these changes
Oct 26, 2026
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
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.
Summary
A reported track (hoodinternet "1986") plays for 11 seconds and stops. Its Qm blob is 447,449 bytes on all 12 nodes holding it. The file's own LAME Info header says the encode was 7,333,590 bytes, the stored copy ends mid-frame, and it does not hash to its CID. Nothing ever caught it, because mediorum can't validate Qm CIDs and pulls them unchecked. This PR closes the two ways that can still happen.
1. Failed copies were committed. gocloud's
Writer.Closecommits whatever has been written, on every driver; only cancelling the writer's context aborts (blob.go: "To abort a write, cancel ctx").replicateToMyBucketandrelocateBetweenBucketscalled_ = w.Close()after a failedio.Copy, so a pull that died partway stored a truncated blob under the full CID. InrelocateBetweenBucketsthat partial copy can later be taken as the real one. The newcopyToBucketcancels before closing on error. The image-variant cache uses it too.2. Short legacy pulls were accepted. Repair, sniff and the serve fallback use
pullFileFromHost, which never checks content. For a Qm CID the byte count is the only check available.chunkedBlobReadernow fails witherrPulledBlobSizeMismatchwhen the bytes received differ from the size the peer reported, fromContent-RangeorContent-Length. A legacy pull from a peer that streams with no length asks/internal/blobs/infofor the size first, and is refused if that fails.Not in scope: a peer that already holds a short copy reports the short size, so this can't repair existing truncations (76833's full file no longer exists anywhere). It stops new ones and stops a partial pull from spreading.
Tests
partial_write_test.go: a failed copy leaves nothing under the key (memblob and fileblob); a truncated legacy pull errors and stores nothing; a complete legacy pull succeeds; a legacy pull of unknown size is refused; a 206 body longer than itsContent-Rangeerrors.cancel()fails the two write tests, and removing the size check fails the two size tests.go test ./pkg/mediorum/... -count=1against a local Postgres: all packages pass.go vetandgofmtare clean on the changed files.🤖 Generated with Claude Code