Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion internal/cache/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -440,7 +440,7 @@ func (c *client) downloadCache(ctx context.Context, retrieveResp api.CacheEntryR
archiveFile = filepath.Join(tmpDir, storeObjectName)

// Download archive
transferInfo, err = blobStore.Download(ctx, storeObjectName, archiveFile)
transferInfo, err = blobStore.Download(ctx, storeObjectName, archiveFile, retrieveResp.Fallback)
if err != nil {
// Clean up temporary directory on failure
_ = os.RemoveAll(tmpDir)
Expand Down
5 changes: 3 additions & 2 deletions internal/cache/store/blob.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,9 @@ type Blob interface {
// Upload uploads a file to blob storage
Upload(ctx context.Context, filePath, key string) (*TransferInfo, error)

// Download downloads a file from blob storage
Download(ctx context.Context, key, destPath string) (*TransferInfo, error)
// Download downloads a file from blob storage. fallback reports whether the
// restore that resolved this key was a fallback match or an exact one.
Download(ctx context.Context, key, destPath string, fallback bool) (*TransferInfo, error)
}

func NewBlobStore(ctx context.Context, store, bucketURL string) (Blob, error) {
Expand Down
2 changes: 1 addition & 1 deletion internal/cache/store/file.go
Original file line number Diff line number Diff line change
Expand Up @@ -287,7 +287,7 @@ func (b *LocalFileBlob) Upload(ctx context.Context, srcPath, key string) (*Trans
//
// Returns TransferInfo with bytes transferred, transfer speed, and duration.
// Returns an error if the cache key doesn't exist or file operations fail.
func (b *LocalFileBlob) Download(ctx context.Context, key, destPath string) (*TransferInfo, error) {
func (b *LocalFileBlob) Download(ctx context.Context, key, destPath string, fallback bool) (*TransferInfo, error) {
_, span := trace.Start(ctx, "LocalFileBlob.Download")
defer span.End()

Expand Down
6 changes: 3 additions & 3 deletions internal/cache/store/file_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ func TestLocalFileBlobDownload(t *testing.T) {

// Then download
destFile := filepath.Join(destDir, "downloaded.txt")
info, err := blob.Download(ctx, key, destFile)
info, err := blob.Download(ctx, key, destFile, false)
if err != nil {
t.Fatalf("Download: %v", err)
}
Expand Down Expand Up @@ -333,7 +333,7 @@ func TestLocalFileBlobDownloadNonExistent(t *testing.T) {
}

destFile := filepath.Join(destDir, "nonexistent.txt")
_, err = blob.Download(ctx, "nonexistent/key", destFile)
_, err = blob.Download(ctx, "nonexistent/key", destFile, false)
if err == nil {
t.Fatal("expected error, got nil")
}
Expand Down Expand Up @@ -389,7 +389,7 @@ func TestLocalFileBlobDownloadInvalidKey(t *testing.T) {
}

destFile := filepath.Join(destDir, "test.txt")
_, err = blob.Download(ctx, "cache//invalid", destFile)
_, err = blob.Download(ctx, "cache//invalid", destFile, false)
if err == nil {
t.Fatal("expected error, got nil")
}
Expand Down
14 changes: 8 additions & 6 deletions internal/cache/store/nsc.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,7 @@ func (n *NscStore) Upload(ctx context.Context, filePath, key string) (*TransferI
}, nil
}

func (n *NscStore) Download(ctx context.Context, key, filePath string) (*TransferInfo, error) {
func (n *NscStore) Download(ctx context.Context, key, filePath string, fallback bool) (*TransferInfo, error) {
_, span := trace.Start(ctx, "NscStore.Download")
defer span.End()

Expand Down Expand Up @@ -230,11 +230,13 @@ func (n *NscStore) Download(ctx context.Context, key, filePath string) (*Transfe
attribute.String("nsc_key", key),
)

// Refresh the artifact's TTL on access so hot caches stay alive, mirroring
// the S3 store's self-CopyObject refresh. Unlike CopyObject this is cheap,
// so we refresh on every restore rather than gating it behind a minimum
// interval.
n.refreshExpiry(ctx, key)
// Refresh TTL on exact-match access to keep hot caches alive; cheap enough
// to run every time, unlike S3's gated CopyObject refresh. Skipped on
// fallback (mirrors the backend's TTL-bump guard) since the blob wasn't
// explicitly targeted.
if !fallback {
n.refreshExpiry(ctx, key)
}

return &TransferInfo{
BytesTransferred: bytesTransferred,
Expand Down
39 changes: 30 additions & 9 deletions internal/cache/store/nsc_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ func TestNscStore_RefreshesTTLOnDownload(t *testing.T) {
var calls [][]string
store := &NscStore{namespace: "my-namespace", run: recordingRunner(&calls, nil)}

if _, err := store.Download(ctx, "key", dest); err != nil {
if _, err := store.Download(ctx, "key", dest, false); err != nil {
t.Fatalf("Download: %v", err)
}

Expand All @@ -328,6 +328,27 @@ func TestNscStore_RefreshesTTLOnDownload(t *testing.T) {
}
}

// TestNscStore_SkipsRefreshOnFallbackDownload mirrors the backend's
// `unless entry_result.fallback_used?` TTL-bump guard: a fallback match must
// not refresh the blob's retention.
func TestNscStore_SkipsRefreshOnFallbackDownload(t *testing.T) {
ctx := t.Context()
dest := filepath.Join(t.TempDir(), "out.txt")

var calls [][]string
store := &NscStore{namespace: "my-namespace", run: recordingRunner(&calls, nil)}

if _, err := store.Download(ctx, "key", dest, true); err != nil {
t.Fatalf("Download: %v", err)
}

for _, c := range calls {
if isCommand(c, "nsc", "artifact", "extend") {
t.Errorf("unexpected extend command on fallback download: %v", c)
}
}
}

func TestNscStore_UpdatesCLIWhenExtendUnsupported(t *testing.T) {
ctx := t.Context()
dest := filepath.Join(t.TempDir(), "out.txt")
Expand All @@ -342,7 +363,7 @@ func TestNscStore_UpdatesCLIWhenExtendUnsupported(t *testing.T) {
}
store := &NscStore{namespace: "ns", run: recordingRunner(&calls, respond)}

if _, err := store.Download(ctx, "key", dest); err != nil {
if _, err := store.Download(ctx, "key", dest, false); err != nil {
t.Fatalf("Download: %v", err)
}

Expand Down Expand Up @@ -383,7 +404,7 @@ func TestNscStore_UpdatesCLIWhenExtendHelpLacksEnsureMinimum(t *testing.T) {
}
store := &NscStore{namespace: "ns", run: recordingRunner(&calls, respond)}

if _, err := store.Download(ctx, "key", dest); err != nil {
if _, err := store.Download(ctx, "key", dest, false); err != nil {
t.Fatalf("Download: %v", err)
}

Expand Down Expand Up @@ -417,7 +438,7 @@ func TestNscStore_DownloadSucceedsWhenRefreshFails(t *testing.T) {
var calls [][]string
store := &NscStore{namespace: "ns", run: recordingRunner(&calls, respond)}

if _, err := store.Download(ctx, "key", dest); err != nil {
if _, err := store.Download(ctx, "key", dest, false); err != nil {
t.Fatalf("Download should succeed despite a failed TTL refresh: %v", err)
}
}
Expand All @@ -441,7 +462,7 @@ func TestNscStore_ValidationShortCircuits(t *testing.T) {
if _, err := store.Upload(ctx, "invalid;path", "valid-key"); err == nil {
t.Error("Upload with unsafe path: expected error, got nil")
}
if _, err := store.Download(ctx, "invalid key with spaces", "dest.txt"); err == nil {
if _, err := store.Download(ctx, "invalid key with spaces", "dest.txt", false); err == nil {
t.Error("Download with unsafe key: expected error, got nil")
}
if ran {
Expand Down Expand Up @@ -513,7 +534,7 @@ func TestNscStore_Integration(t *testing.T) {

// Test download
downloadFile := filepath.Join(tmpDir, "test-download.txt")
transferInfo, err = store.Download(ctx, key, downloadFile)
transferInfo, err = store.Download(ctx, key, downloadFile, false)
if err != nil {
t.Fatalf("Download should succeed: %v", err)
}
Expand Down Expand Up @@ -546,7 +567,7 @@ func TestNscStore_DownloadNotFound(t *testing.T) {
store := &NscStore{namespace: "ns", run: func(context.Context, string, ...string) (*CommandResult, error) {
return &CommandResult{ExitCode: 1, Stderr: "Error: artifact not found"}, nil
}}
_, err := store.Download(ctx, "valid-key", dest)
_, err := store.Download(ctx, "valid-key", dest, false)
if !errors.Is(err, ErrBlobNotFound) {
t.Fatalf("Download err = %v, want ErrBlobNotFound", err)
}
Expand All @@ -559,7 +580,7 @@ func TestNscStore_DownloadNotFound(t *testing.T) {
Stderr: "Failed: the artifact has expired at 2026-07-14T02:06:24Z (request id: cbdorlqepas5e10ldvfvdpog40).",
}, nil
}}
_, err := store.Download(ctx, "valid-key", dest)
_, err := store.Download(ctx, "valid-key", dest, false)
if !errors.Is(err, ErrBlobNotFound) {
t.Fatalf("Download err = %v, want ErrBlobNotFound", err)
}
Expand All @@ -569,7 +590,7 @@ func TestNscStore_DownloadNotFound(t *testing.T) {
store := &NscStore{namespace: "ns", run: func(context.Context, string, ...string) (*CommandResult, error) {
return &CommandResult{ExitCode: 1, Stderr: "connection refused"}, nil
}}
_, err := store.Download(ctx, "valid-key", dest)
_, err := store.Download(ctx, "valid-key", dest, false)
if err == nil {
t.Fatal("Download: expected error, got nil")
}
Expand Down
71 changes: 45 additions & 26 deletions internal/cache/store/s3.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,12 @@ type objectDownloader interface {
Download(ctx context.Context, w io.WriterAt, input *s3.GetObjectInput, options ...func(*manager.Downloader)) (int64, error) //nolint:staticcheck // SA1019: pending migration to transfermanager
}

// objectCopier is the subset of *s3.Client used for the TTL-refresh self-copy,
// declared so the refresh decision can be tested with a fake.
type objectCopier interface {
CopyObject(ctx context.Context, input *s3.CopyObjectInput, opts ...func(*s3.Options)) (*s3.CopyObjectOutput, error)
}

// isPreconditionFailed returns true when an error is an S3 412 PreconditionFailed.
// This happens when:
//
Expand Down Expand Up @@ -172,7 +178,7 @@ func downloadWithRetry(ctx context.Context, r *roko.Retrier, d objectDownloader,

// S3Blob implements the Blob interface using AWS S3
type S3Blob struct {
client *s3.Client
client objectCopier
uploader *manager.Uploader //nolint:staticcheck // SA1019: pending migration to transfermanager
downloader *manager.Downloader //nolint:staticcheck // SA1019: pending migration to transfermanager
bucketName string
Expand Down Expand Up @@ -387,7 +393,7 @@ func (b *S3Blob) Upload(ctx context.Context, filePath, key string) (*TransferInf
const restoreRefreshMinInterval = 12 * time.Hour

// Download downloads a file from S3 using parallel range requests for large files
func (b *S3Blob) Download(ctx context.Context, key, destPath string) (*TransferInfo, error) {
func (b *S3Blob) Download(ctx context.Context, key, destPath string, fallback bool) (*TransferInfo, error) {
ctx, span := trace.Start(ctx, "S3Blob.Download")
defer span.End()

Expand Down Expand Up @@ -458,42 +464,55 @@ func (b *S3Blob) Download(ctx context.Context, key, destPath string) (*TransferI
attribute.Int("concurrency", b.downloadConcurrency),
)

// Extend an object's effective TTL by performing CopyObject on itself to
// refresh its LastModified timestamp.
//
// The CopySourceIfUnmodifiedSince precondition aborts the operation with an
// HTTP status code 412 if the object's LastModified timestamp falls within
// restoreRefreshMinInterval.
//
// This refresh is best-effort only: any failure (incl. objects exceeding
// S3's 5GB CopyObject limit) must not cause the overall restore operation to fail.
copySource := fmt.Sprintf("%s/%s", b.bucketName, fullKey)
_, err = b.client.CopyObject(ctx, &s3.CopyObjectInput{
Bucket: aws.String(b.bucketName),
refreshObjectExpiry(ctx, b.client, b.bucketName, fullKey, fallback)

return &TransferInfo{
BytesTransferred: bytesWritten,
TransferSpeed: averageSpeed,
RequestID: "", // Download doesn't return a single request ID for parallel downloads
Duration: duration,
PartCount: actualPartCount,
Concurrency: b.downloadConcurrency,
}, nil
}

// refreshObjectExpiry extends an object's effective TTL by performing
// CopyObject on itself to refresh its LastModified timestamp.
//
// Skipped on a fallback match — mirrors the backend's
// `unless entry_result.fallback_used?` TTL-bump guard: a fallback hit means
// the entry was saved under a shorter key sequence, and refreshing it would
// reset the clock on a blob the caller didn't explicitly target.
//
// The CopySourceIfUnmodifiedSince precondition aborts the operation with an
// HTTP status code 412 if the object's LastModified timestamp falls within
// restoreRefreshMinInterval.
//
// This refresh is best-effort only: any failure (incl. objects exceeding
// S3's 5GB CopyObject limit) must not cause the overall restore operation to fail.
func refreshObjectExpiry(ctx context.Context, copier objectCopier, bucket, fullKey string, fallback bool) {
if fallback {
return
}

copySource := fmt.Sprintf("%s/%s", bucket, fullKey)
_, err := copier.CopyObject(ctx, &s3.CopyObjectInput{
Bucket: aws.String(bucket),
Key: aws.String(fullKey),
CopySource: aws.String(copySource),
MetadataDirective: "REPLACE",
CopySourceIfUnmodifiedSince: aws.Time(time.Now().Add(-restoreRefreshMinInterval)),
})
switch {
case err == nil:
slog.Debug("refreshed object expiration", "key", fullKey, "bucket", b.bucketName)
slog.Debug("refreshed object expiration", "key", fullKey, "bucket", bucket)
case isPreconditionFailed(err):
slog.Debug("skipping cache TTL refresh, blob modified recently",
"key", fullKey, "bucket", b.bucketName)
"key", fullKey, "bucket", bucket)
default:
slog.Warn("failed to refresh object expiration, continuing (non-fatal)",
"key", fullKey, "bucket", b.bucketName, "error", err)
"key", fullKey, "bucket", bucket, "error", err)
}

return &TransferInfo{
BytesTransferred: bytesWritten,
TransferSpeed: averageSpeed,
RequestID: "", // Download doesn't return a single request ID for parallel downloads
Duration: duration,
PartCount: actualPartCount,
Concurrency: b.downloadConcurrency,
}, nil
}

// getFullKey combines the prefix with the key
Expand Down
62 changes: 62 additions & 0 deletions internal/cache/store/s3_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"testing"
"time"

"github.com/aws/aws-sdk-go-v2/aws"
awshttp "github.com/aws/aws-sdk-go-v2/aws/transport/http"
"github.com/aws/aws-sdk-go-v2/feature/s3/manager"
"github.com/aws/aws-sdk-go-v2/service/s3"
Expand Down Expand Up @@ -582,3 +583,64 @@ func TestIsNotFound(t *testing.T) {
})
}
}

// fakeCopier is a test double for objectCopier, recording the input of every
// CopyObject call and returning the configured result.
type fakeCopier struct {
calls []*s3.CopyObjectInput
err error
}

func (f *fakeCopier) CopyObject(_ context.Context, input *s3.CopyObjectInput, _ ...func(*s3.Options)) (*s3.CopyObjectOutput, error) {
f.calls = append(f.calls, input)
if f.err != nil {
return nil, f.err
}
return &s3.CopyObjectOutput{}, nil
}

func TestRefreshObjectExpiry(t *testing.T) {
t.Run("self-copies the object to refresh LastModified", func(t *testing.T) {
copier := &fakeCopier{}

refreshObjectExpiry(t.Context(), copier, "my-bucket", "prefix/key", false)

if len(copier.calls) != 1 {
t.Fatalf("CopyObject calls = %d, want 1", len(copier.calls))
}
got := copier.calls[0]
if aws.ToString(got.Bucket) != "my-bucket" || aws.ToString(got.Key) != "prefix/key" {
t.Errorf("CopyObjectInput bucket/key = %q/%q, want %q/%q", aws.ToString(got.Bucket), aws.ToString(got.Key), "my-bucket", "prefix/key")
}
if aws.ToString(got.CopySource) != "my-bucket/prefix/key" {
t.Errorf("CopySource = %q, want %q", aws.ToString(got.CopySource), "my-bucket/prefix/key")
}
if got.MetadataDirective != types.MetadataDirectiveReplace {
t.Errorf("MetadataDirective = %v, want REPLACE", got.MetadataDirective)
}
})

// Mirrors the backend's `unless entry_result.fallback_used?` TTL-bump
// guard: a fallback match must not refresh the blob's retention.
t.Run("skips the refresh on a fallback match", func(t *testing.T) {
copier := &fakeCopier{}

refreshObjectExpiry(t.Context(), copier, "my-bucket", "key", true)

if len(copier.calls) != 0 {
t.Errorf("CopyObject calls = %d, want 0 for a fallback match", len(copier.calls))
}
})

t.Run("swallows a precondition-failed error (recently refreshed)", func(t *testing.T) {
copier := &fakeCopier{err: responseErrorWithStatus(http.StatusPreconditionFailed)}

refreshObjectExpiry(t.Context(), copier, "my-bucket", "key", false) // must not panic
})

t.Run("swallows any other error (best-effort)", func(t *testing.T) {
copier := &fakeCopier{err: errors.New("boom")}

refreshObjectExpiry(t.Context(), copier, "my-bucket", "key", false) // must not panic
})
}
Loading