Stop writing fileblob's .attrs sidecar - #328
Open
montehurd wants to merge 2 commits into
Open
Conversation
fileblob stores blob metadata in an ".attrs" file per object and rewrites it with os.Create, truncating in place outside the atomic rename that protects the blob. A read overlapping a write decodes a partial file and fails with "opening reader: EOF", served as a 502. One writer against four readers on a single key failed 408 of 2000 reads. cacheMetadataBlob is most exposed to it, rewriting a key on every refresh while readers are served from it. Nothing in the proxy reads what the sidecar holds. gocloud.dev/blob is imported only by internal/storage, Store sets no ContentType, and Attributes is used only for Size, which comes from os.Stat. A missing sidecar already defaults cleanly, so "metadata=skip" removes the hazard rather than locking around it, and saves a write per store.
metadata=skip stops fileblob rewriting sidecars but does not delete ones already on disk, so a sidecar left partial by an interrupted write now fails every read of its key for good. Before, a later store repaired it by rewriting. Store therefore removes the sidecar for the key it writes. Removal is atomic where the rewrite was not, so a concurrent reader gets the whole old file or nothing. Delete already removes sidecars, so the two paths drain a cache between them. Deriving that path is necessary because fileblob's key escaping is unexported. It is the identity for a plain key and parts from one only for keys that are not valid local paths, which is what filepath.Localize rejects. That also keeps the removal inside the cache directory: without it a key holding ".." resolves outside. The clearing test runs one key per storage path the proxy builds, seeded through a bucket that still writes sidecars so the path under test is fileblob's own.
This was referenced Sep 8, 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
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.
fileblobkeeps per-object metadata in an.attrssidecar and rewrites it withos.Create, truncating in place outside the atomic rename that protects the blob. A read overlapping a write to one key decodes a partial file and fails withopening reader: EOF, served as a 502. One writer against four readers on a single key failed 408 of 2000 reads.Nothing in the proxy reads what the sidecar holds (
gocloud.dev/blobis imported only byinternal/storage,Storesets noContentType, and the onlyAttributesfield read isSize), andgetAttrsalready defaults a missing one, sometadata=skipremoves the hazard instead of locking around it. That is the first commit, one line.The second is separable.
metadata=skipstops rewriting sidecars without deleting them, so one already left partial by an interrupted write would be stuck for good, where a later store used to repair it.Storetherefore clears the sidecar for the key it writes. Deriving that path usesfilepath.Localize, which also keeps the removal inside the cache directory. Drop this commit in favour of documenting afind <cache> -name '*.attrs' -deletesweep if that is preferred.Each commit carries the tests that prove it, and each fails without its own change. Green on ubuntu, macOS and Windows.