Repository navigation
storage/gcp, storage/aws: fix double BucketPrefix on idempotent-write recovery - #1208
Merged
mhutchinson merged 2 commits intoOct 6, 2026
Merged
mhutchinson merged 2 commits into
mhutchinson merged 2 commits into
Conversation
setObject prefixed objName with bucketPrefix before the write, then on a precondition failure called getObject with that already-prefixed name. getObject applies bucketPrefix again, so the recovery read looked for <prefix>/<prefix>/<name> and always failed whenever BucketPrefix is non-empty, defeating the idempotent-write branch the precondition check exists to reach. Keep the caller's name unprefixed and use a separate prefixedName for the write, so the recovery getObject call gets the name it expects. Adds a regression test that drives gcsStorage against an httptest fake GCS endpoint with a non-empty BucketPrefix and asserts the recovery read targets the single-prefixed object and setObject returns nil.
…nt recovery Same defect as the GCP fix in the previous commit: setObjectIfNoneMatch prefixed objName with bucketPrefix before the conditional write, then on a precondition failure called getObject with that already-prefixed name. getObject applies bucketPrefix again, so the recovery read looked for <prefix>/<prefix>/<name> and always failed whenever BucketPrefix is non-empty, defeating the idempotent-write branch. Keep the caller's name unprefixed and use a separate prefixedName for the write, so the recovery getObject call gets the name it expects. Adds a regression test that drives s3Storage against an httptest fake S3 endpoint with a non-empty BucketPrefix and asserts both the write and the recovery read target the single-prefixed key.
mhutchinson
force-pushed
the
storage-bucketprefix-recovery
branch
from
October 6, 2026 15:41
c908a6f to
7264afe
Compare
mhutchinson
approved these changes
Oct 6, 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
2 participants
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.
What
Fixes the idempotent-write recovery path in the GCP and AWS drivers when
BucketPrefixis set.gcsStorage.setObjectands3Storage.setObjectIfNoneMatchoverwriteobjNamewith the prefixed name before the conditional write. When that write fails its precondition because the object already exists, they callgetObjectwith the already-prefixed name to compare contents.getObjectappliesbucketPrefixitself, so the read goes to<prefix>/<prefix>/<name>, fails with not-found, and the caller gets an error instead of success.The fix keeps
objNameunprefixed, uses a separateprefixedNamefor the write and for log, error and trace output, and passes the unprefixed name togetObject. One commit per driver. Nothing changes whenBucketPrefixis empty.Why
Tiles and entry bundles are written with a "does not exist" precondition, and a write that finds a bit-for-bit identical object already there is meant to succeed, so that retried writes are idempotent. With a non-empty
BucketPrefixthat branch can never succeed: every such write returnsfailed to fetch existing content ... object doesn't exist(GCS) orNoSuchKey(S3), even though the stored data matches.Testing
TestSetObjectBucketPrefixIdempotentRecovery(GCP) drivesgcsStoragewith a non-empty prefix against anhttptestfake of the GCS JSON API that answers the conditional upload with 412 and serves identical bytes only at<prefix>/<name>. It checkssetObjectreturns nil and the recovery read was for the single-prefixed object.TestSetObjectIfNoneMatchBucketPrefixIdempotentRecovery(AWS) does the same fors3Storageagainst anhttptestfake S3 endpoint, and also checks the conditionalPUTkey.Each test fails without the corresponding driver change, with the recovery read going to
<prefix>/<prefix>/<name>.go test ./storage/gcp/... ./storage/aws/...,go vet,gofmt,golangci-lintclean.