From f23bf2a6d9d83314f755938d6e2c460f5786af35 Mon Sep 17 00:00:00 2001 From: Ben Vincent Date: Thu, 2 Jul 2026 20:07:30 +1000 Subject: [PATCH] fix: serveFromStore does a guaranteed-miss S3 lookup on every cache hit (#82) Fixes #78 ## Why `serveFromStore` first called `store.Download` with the bare content hash as the S3 key, which never matches real object keys (`blobs/sha256/`). Every cached blob serve therefore paid an extra guaranteed-404 round-trip before retrying with the correct `BlobKey`. ## Changes - Remove the dead first `Download` attempt; go straight to the `BlobKey` lookup, then fall back to the index key. ## Validation - `make e2e` passes (proxy cache-hit paths exercised end-to-end). Reviewed-on: https://git.unkin.net/unkin/artifactapi/pulls/82 Co-authored-by: Ben Vincent Co-committed-by: Ben Vincent --- internal/proxy/engine.go | 11 +---------- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/internal/proxy/engine.go b/internal/proxy/engine.go index ba63e78..c7dbc72 100644 --- a/internal/proxy/engine.go +++ b/internal/proxy/engine.go @@ -250,17 +250,8 @@ func (e *Engine) fetchFromUpstream(ctx context.Context, remote models.Remote, pa func (e *Engine) serveFromStore(ctx context.Context, remote models.Remote, path string) (*FetchResult, error) { artifact, err := e.db.GetArtifact(ctx, remote.Name, path) if err == nil && artifact != nil { - reader, info, err := e.store.Download(ctx, artifact.ContentHash[len("sha256:"):]) - if err == nil { - _ = e.db.TouchArtifactAccess(ctx, remote.Name, path) - return &FetchResult{ - Reader: reader, - ContentType: info.ContentType, - Size: info.Size, - }, nil - } s3Key := storage.BlobKey(artifact.ContentHash[len("sha256:"):]) - reader, info, err = e.store.Download(ctx, s3Key) + reader, info, err := e.store.Download(ctx, s3Key) if err == nil { _ = e.db.TouchArtifactAccess(ctx, remote.Name, path) return &FetchResult{