Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
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
27 changes: 18 additions & 9 deletions api/cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,10 @@ type CacheEntryCreateResp struct {
Multipart bool `json:"multipart"`
UploadInstructions []string `json:"upload_instructions"`
Message string `json:"message"`
// RetentionSeconds is how long the backing blob should be kept, matching the
// registry entry's TTL. Zero when the server does not send it (older server),
// in which case the store falls back to its own default.
RetentionSeconds int64 `json:"retention_seconds"`
}

// CacheEntryRetrieveReq is the request body for retrieving a cache entry.
Expand All @@ -79,15 +83,20 @@ type CacheEntryRetrieveReq struct {

// CacheEntryRetrieveResp describes the cache entry to download.
type CacheEntryRetrieveResp struct {
TargetPaths []string `json:"target_paths"`
CacheKey []CacheKeyPart `json:"cache_key"`
Blobs []CacheBlob `json:"blobs"`
ExpiresAt time.Time `json:"expires_at"`
Store string `json:"store"`
Fallback bool `json:"fallback"`
Multipart bool `json:"multipart"`
DownloadInstructions []string `json:"download_instructions"`
Message string `json:"message"`
TargetPaths []string `json:"target_paths"`
CacheKey []CacheKeyPart `json:"cache_key"`
Blobs []CacheBlob `json:"blobs"`
ExpiresAt time.Time `json:"expires_at"`
// RetentionSeconds is how far to push the backing blob's expiry on an
// exact-hit refresh, matching the registry entry's TTL. Zero when the server
// does not send it (older server), in which case the store falls back to its
// own default.
RetentionSeconds int64 `json:"retention_seconds"`
Store string `json:"store"`
Fallback bool `json:"fallback"`
Multipart bool `json:"multipart"`
DownloadInstructions []string `json:"download_instructions"`
Message string `json:"message"`
// Scopes are the resolved entry's own scope labels (nil when unscoped),
// e.g. {"branch": "main"}. Echo these back on CacheEntryExpireReq to
// invalidate this exact entry — its scope may no longer match what the
Expand Down
6 changes: 3 additions & 3 deletions internal/cache/restore.go
Original file line number Diff line number Diff line change
Expand Up @@ -537,7 +537,7 @@ func (c *client) downloadCache(ctx context.Context, retrieveResp api.CacheEntryR
}

// Extend the blob's retention now that it's confirmed good.
maybeRefreshRetention(ctx, blobStore, retrieveResp.Fallback, storeObjectName)
maybeRefreshRetention(ctx, blobStore, retrieveResp.Fallback, storeObjectName, time.Duration(retrieveResp.RetentionSeconds)*time.Second)

span.SetStatus(codes.Ok, "download completed")

Expand All @@ -549,12 +549,12 @@ func (c *client) downloadCache(ctx context.Context, retrieveResp api.CacheEntryR
// shorter key sequence, and refreshing it would reset the clock on a blob
// the caller didn't explicitly target (mirrors the backend's
// `unless entry_result.fallback_used?` TTL-bump guard).
func maybeRefreshRetention(ctx context.Context, blobStore store.Blob, fallback bool, key string) {
func maybeRefreshRetention(ctx context.Context, blobStore store.Blob, fallback bool, key string, retention time.Duration) {
refresher, ok := blobStore.(store.RetentionRefresher)
if !ok || fallback {
return
}
refresher.RefreshRetention(ctx, key)
refresher.RefreshRetention(ctx, key, retention)
}

// extractCache extracts files from a cache archive
Expand Down
21 changes: 13 additions & 8 deletions internal/cache/restore_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,26 +448,28 @@ func TestCleanPathWindowsUNCShareRoot(t *testing.T) {
// fakeRefreshingBlob is a minimal store.Blob that also implements
// store.RetentionRefresher, mirroring NscStore/S3Blob.
type fakeRefreshingBlob struct {
refreshCalls []string
refreshCalls []string
refreshRetentions []time.Duration
}

func (f *fakeRefreshingBlob) Upload(_ context.Context, _, _ string) (*store.TransferInfo, error) {
func (f *fakeRefreshingBlob) Upload(_ context.Context, _, _ string, _ time.Duration) (*store.TransferInfo, error) {
return nil, nil
}

func (f *fakeRefreshingBlob) Download(_ context.Context, _, _ string) (*store.TransferInfo, error) {
return nil, nil
}

func (f *fakeRefreshingBlob) RefreshRetention(_ context.Context, key string) {
func (f *fakeRefreshingBlob) RefreshRetention(_ context.Context, key string, retention time.Duration) {
f.refreshCalls = append(f.refreshCalls, key)
f.refreshRetentions = append(f.refreshRetentions, retention)
}

// fakeNonRefreshingBlob is a store.Blob with no RefreshRetention method,
// mirroring LocalFileBlob (no retention concept to refresh).
type fakeNonRefreshingBlob struct{}

func (f *fakeNonRefreshingBlob) Upload(_ context.Context, _, _ string) (*store.TransferInfo, error) {
func (f *fakeNonRefreshingBlob) Upload(_ context.Context, _, _ string, _ time.Duration) (*store.TransferInfo, error) {
return nil, nil
}

Expand All @@ -476,22 +478,25 @@ func (f *fakeNonRefreshingBlob) Download(_ context.Context, _, _ string) (*store
}

func TestMaybeRefreshRetention(t *testing.T) {
t.Run("refreshes on an exact match", func(t *testing.T) {
t.Run("refreshes on an exact match with the given retention", func(t *testing.T) {
blob := &fakeRefreshingBlob{}

maybeRefreshRetention(t.Context(), blob, false, "key")
maybeRefreshRetention(t.Context(), blob, false, "key", 48*time.Hour)

if len(blob.refreshCalls) != 1 {
t.Errorf("refresh calls = %d, want 1", len(blob.refreshCalls))
}
if len(blob.refreshRetentions) != 1 || blob.refreshRetentions[0] != 48*time.Hour {
t.Errorf("refresh retentions = %v, want [48h]", blob.refreshRetentions)
}
})

// 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) {
blob := &fakeRefreshingBlob{}

maybeRefreshRetention(t.Context(), blob, true, "key")
maybeRefreshRetention(t.Context(), blob, true, "key", 48*time.Hour)

if len(blob.refreshCalls) != 0 {
t.Errorf("refresh calls = %d, want 0 for a fallback match", len(blob.refreshCalls))
Expand All @@ -501,6 +506,6 @@ func TestMaybeRefreshRetention(t *testing.T) {
t.Run("does nothing for a store with no retention concept", func(t *testing.T) {
blob := &fakeNonRefreshingBlob{}

maybeRefreshRetention(t.Context(), blob, false, "key") // must not panic
maybeRefreshRetention(t.Context(), blob, false, "key", 48*time.Hour) // must not panic
})
}
2 changes: 1 addition & 1 deletion internal/cache/save.go
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ func (c *client) Save(ctx context.Context, cacheID string) (SaveResult, error) {
return result, fmt.Errorf("failed to create blob store: %w", err)
}

transferInfo, err := blobStore.Upload(ctx, archiveInfo.ArchivePath, storeObjectName)
transferInfo, err := blobStore.Upload(ctx, archiveInfo.ArchivePath, storeObjectName, time.Duration(createResp.RetentionSeconds)*time.Second)
if err != nil {
span.RecordError(err)
span.SetStatus(codes.Error, "failed to upload cache")
Expand Down
12 changes: 8 additions & 4 deletions internal/cache/store/blob.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"errors"
"fmt"
"strings"
"time"
)

// ErrBlobNotFound is returned by a Blob's Download when the requested object
Expand All @@ -13,8 +14,10 @@ var ErrBlobNotFound = errors.New("blob not found")

// Blob interface defines the operations for blob storage
type Blob interface {
// Upload uploads a file to blob storage
Upload(ctx context.Context, filePath, key string) (*TransferInfo, error)
// Upload uploads a file to blob storage. retention is how long the blob
// should be kept; a store that has no retention concept (or a zero/negative
// value, meaning the server didn't specify one) uses its own default.
Upload(ctx context.Context, filePath, key string, retention time.Duration) (*TransferInfo, error)

// Download downloads a file from blob storage
Download(ctx context.Context, key, destPath string) (*TransferInfo, error)
Expand All @@ -23,9 +26,10 @@ type Blob interface {
// RetentionRefresher is implemented by Blob stores that support extending a
// blob's effective retention/TTL on access (NscStore, S3Blob). Optional
// because not every store has a retention concept to refresh (LocalFileBlob
// does not implement it).
// does not implement it). retention is the minimum lifetime to guarantee from
// now; a zero/negative value falls back to the store's own default.
type RetentionRefresher interface {
RefreshRetention(ctx context.Context, key string)
RefreshRetention(ctx context.Context, key string, retention time.Duration)
}

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 @@ -127,7 +127,7 @@ func NewLocalFileBlob(ctx context.Context, fileURL string) (*LocalFileBlob, erro
// semantics for concurrent uploads to the same key.
//
// Returns TransferInfo with bytes transferred, transfer speed, and duration.
func (b *LocalFileBlob) Upload(ctx context.Context, srcPath, key string) (*TransferInfo, error) {
func (b *LocalFileBlob) Upload(ctx context.Context, srcPath, key string, _ time.Duration) (*TransferInfo, error) {
_, span := trace.Start(ctx, "LocalFileBlob.Upload")
defer span.End()

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

key := "test/cache/artifact.txt"

info, err := blob.Upload(ctx, srcFile, key)
info, err := blob.Upload(ctx, srcFile, key, 0)
if err != nil {
t.Fatalf("Upload: %v", err)
}
Expand Down Expand Up @@ -231,7 +231,7 @@ func TestLocalFileBlobDownload(t *testing.T) {
key := "test/cache/artifact.txt"

// Upload first
_, err = blob.Upload(ctx, srcFile, key)
_, err = blob.Upload(ctx, srcFile, key, 0)
if err != nil {
t.Fatalf("Upload: %v", err)
}
Expand Down Expand Up @@ -285,7 +285,7 @@ func TestLocalFileBlobUploadOverwrite(t *testing.T) {
if err := os.WriteFile(srcFile1, content1, 0o600); err != nil {
t.Fatalf("WriteFile: %v", err)
}
_, err = blob.Upload(ctx, srcFile1, key)
_, err = blob.Upload(ctx, srcFile1, key, 0)
if err != nil {
t.Fatalf("Upload: %v", err)
}
Expand All @@ -297,7 +297,7 @@ func TestLocalFileBlobUploadOverwrite(t *testing.T) {
t.Fatalf("WriteFile: %v", err)
}

info, err := blob.Upload(ctx, srcFile2, key)
info, err := blob.Upload(ctx, srcFile2, key, 0)
if err != nil {
t.Fatalf("Upload: %v", err)
}
Expand Down Expand Up @@ -363,7 +363,7 @@ func TestLocalFileBlobUploadInvalidKey(t *testing.T) {
t.Fatalf("WriteFile: %v", err)
}

_, err = blob.Upload(ctx, srcFile, "../../../etc/passwd")
_, err = blob.Upload(ctx, srcFile, "../../../etc/passwd", 0)
if err == nil {
t.Fatal("expected error, got nil")
}
Expand Down Expand Up @@ -518,12 +518,12 @@ func TestLocalFileBlobConcurrentUpload(t *testing.T) {
done := make(chan error, 2)

go func() {
_, err := blob.Upload(ctx, srcFile1, key)
_, err := blob.Upload(ctx, srcFile1, key, 0)
done <- err
}()

go func() {
_, err := blob.Upload(ctx, srcFile2, key)
_, err := blob.Upload(ctx, srcFile2, key, 0)
done <- err
}()

Expand Down
43 changes: 26 additions & 17 deletions internal/cache/store/nsc.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,15 +21,24 @@ import (
// nscScheme is the URL scheme that routes an agent-managed cache store to NSC.
const nscScheme = "nsc"

// nscDefaultExpiry is the artifact lifetime used both when uploading a cache
// entry (--expires_in) and when refreshing it on access (--ensure_minimum).
// Keep this aligned with CacheRegistry::Entry::DEFAULT_EXPIRES in the Buildkite
// backend.
// nscDefaultRetention is the fallback artifact lifetime used for --expires_in
// (upload) and --ensure_minimum (refresh on access) when the server does not
// supply a retention (older server). Normally the retention comes from the
// cache registry's configured TTL, sent per request; see nscRetentionArg.
// Cache entries are content-addressed and short-lived, so we cap storage growth
// rather than relying on NSC's no-expiry default. Every restore pushes the
// expiry back out to this duration from now, keeping hot caches alive while
// letting cold ones expire.
const nscDefaultExpiry = "72h"
// rather than relying on NSC's no-expiry default.
const nscDefaultRetention = "72h"

// nscRetentionArg formats a retention duration for nsc's --expires_in / --ensure_minimum
// flags, rounding up to whole hours. A zero or negative duration (the server did
// not specify one) falls back to nscDefaultRetention.
func nscRetentionArg(retention time.Duration) string {
if retention <= 0 {
return nscDefaultRetention
}
hours := int64((retention + time.Hour - 1) / time.Hour)
return fmt.Sprintf("%dh", hours)
}

// commandRunner executes an external command. It is a seam so tests can assert
// the arguments passed to the nsc CLI without invoking the real binary.
Expand Down Expand Up @@ -135,7 +144,7 @@ func validateKey(key string) error {
return nil
}

func (n *NscStore) Upload(ctx context.Context, filePath, key string) (*TransferInfo, error) {
func (n *NscStore) Upload(ctx context.Context, filePath, key string, retention time.Duration) (*TransferInfo, error) {
_, span := trace.Start(ctx, "NscStore.Upload")
defer span.End()

Expand All @@ -150,7 +159,7 @@ func (n *NscStore) Upload(ctx context.Context, filePath, key string) (*TransferI
start := time.Now()

// Execute nsc artifact upload command
result, err := n.run(ctx, "", n.artifactArgs("upload", filePath, key, "--expires_in", nscDefaultExpiry)...)
result, err := n.run(ctx, "", n.artifactArgs("upload", filePath, key, "--expires_in", nscRetentionArg(retention))...)
if err != nil {
return nil, fmt.Errorf("failed to execute nsc upload command: %w", err)
}
Expand Down Expand Up @@ -238,17 +247,17 @@ func (n *NscStore) Download(ctx context.Context, key, filePath string) (*Transfe
}, nil
}

// RefreshRetention pushes the artifact's expiry out to at least
// nscDefaultExpiry from now via `nsc artifact extend --ensure_minimum`. Using
// --ensure_minimum (rather than the additive --by) makes the refresh
// idempotent, so calling it on every restore keeps a hot cache alive without
// growing its expiry unbounded.
// RefreshRetention pushes the artifact's expiry out to at least retention from
// now (falling back to nscDefaultRetention when unset) via `nsc artifact extend
// --ensure_minimum`. Using --ensure_minimum (rather than the additive --by)
// makes the refresh idempotent, so calling it on every restore keeps a hot
// cache alive without growing its expiry unbounded.
//
// This is best-effort: any failure is logged and swallowed so a restore never
// fails because its TTL could not be refreshed. Whether to call this at all
// (e.g. skipping it on a fallback match) is the restore flow's decision, not
// this store's — see store.RetentionRefresher.
func (n *NscStore) RefreshRetention(ctx context.Context, key string) {
func (n *NscStore) RefreshRetention(ctx context.Context, key string, retention time.Duration) {
if !n.extendSupported(ctx) {
// `nsc artifact extend` is still being rolled out by Namespace. Update
// the nsc CLI as a contingency so the command becomes available.
Expand All @@ -260,7 +269,7 @@ func (n *NscStore) RefreshRetention(ctx context.Context, key string) {
}
}

result, err := n.run(ctx, "", n.artifactArgs("extend", key, "--ensure_minimum", nscDefaultExpiry)...)
result, err := n.run(ctx, "", n.artifactArgs("extend", key, "--ensure_minimum", nscRetentionArg(retention))...)
switch {
case err != nil:
slog.Warn("failed to refresh cache TTL, continuing (non-fatal)", "key", key, "error", err)
Expand Down
Loading