From a675ac96b9b401c02a9fb6de33a815fec3b94726 Mon Sep 17 00:00:00 2001 From: Janko Thyson Date: Thu, 11 Jun 2026 09:24:52 +0200 Subject: [PATCH] fix(storage): treat dedupe-candidate cache miss as no candidates, not an error (#4122) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit GetAllDedupeReposCandidates propagated zerr.ErrCacheMiss from the cache's GetAllBlobs verbatim. But a cache miss is the normal case for a not-yet-cached blob — the first push of a new blob, or a cross-repo mount check during a push — and semantically means "no dedupe/mount candidates", not a failure. Propagating it caused canMount (used by the CheckBlob and CreateBlobUpload handlers) to surface the error, which the route handlers log as an "unexpected error". With remote (e.g. S3) storage a cache is always present (dedupe:false does not disable it), so this logs an error-level line for every fresh blob digest on every push — significant log spam during bulk pushes and cross-repo mounts, with no functional impact (the push still succeeds via a normal upload). Handle ErrCacheMiss the same way as the existing nil-cache branch above: return no candidates and no error. Signed-off-by: Janko Thyson Co-authored-by: Claude Opus 4.8 (1M context) --- pkg/storage/imagestore/imagestore.go | 10 ++++++++++ pkg/storage/storage_test.go | 12 ++++++++++++ 2 files changed, 22 insertions(+) diff --git a/pkg/storage/imagestore/imagestore.go b/pkg/storage/imagestore/imagestore.go index 4538e40e..1819b363 100644 --- a/pkg/storage/imagestore/imagestore.go +++ b/pkg/storage/imagestore/imagestore.go @@ -1388,6 +1388,16 @@ func (is *ImageStore) GetAllDedupeReposCandidates(digest godigest.Digest) ([]str blobsPaths, err := is.cache.GetAllBlobs(digest) if err != nil { + // A cache miss means the digest is not present in the cache yet, so there + // are simply no dedupe/mount candidates for it — that is the normal case + // for a not-yet-cached blob, not a failure. Treat it like the nil-cache + // case above and return no candidates rather than propagating the error + // (which callers log as an "unexpected error", spamming the logs on every + // fresh blob during pushes/cross-repo mount checks). + if errors.Is(err, zerr.ErrCacheMiss) { + return nil, nil //nolint:nilnil + } + return nil, err } diff --git a/pkg/storage/storage_test.go b/pkg/storage/storage_test.go index e9c891d2..76316678 100644 --- a/pkg/storage/storage_test.go +++ b/pkg/storage/storage_test.go @@ -644,6 +644,18 @@ func TestGetAllDedupeReposCandidates(t *testing.T) { slices.Sort(repos) So(repoNames, ShouldResemble, repos) }) + + Convey("A digest with no cached blob returns no candidates and no error", t, func(c C) { + // A cache miss is the normal case for a not-yet-cached blob (e.g. the + // first push of a new blob, or a cross-repo mount check during a push). + // It must surface as "no dedupe/mount candidates", NOT as an error the + // caller logs as "unexpected error". + uncachedDigest := godigest.FromBytes([]byte("blob that was never pushed to any repo")) + + repos, err := imgStore.GetAllDedupeReposCandidates(uncachedDigest) + So(err, ShouldBeNil) + So(repos, ShouldBeEmpty) + }) }) } }