Add cache cleanup

Author: Thomas Stromberg

README.md

@@ -31,6 +31,12 @@ val, found, err := cache.Get(ctx, "answer")
 
 // With smart persistence (local files for dev, Google Cloud Datastore for Cloud Run)
 cache, err := bdcache.New[string, User](ctx, bdcache.WithBestStore("myapp"))
+
+// With Cloud Datastore persistence and automatic cleanup
+cache, err := bdcache.New[string, User](ctx,
+    bdcache.WithCloudDatastore("myapp"),
+    bdcache.WithCleanup(24*time.Hour), // Cleanup entries older than 24h
+)
 ```
 
 ## Features
@@ -92,6 +98,41 @@ BenchmarkCache_Get_PersistDiskRead-16     73K ops/sec    13.8 µs/op    7921 B/o
 BenchmarkCache_Set_WithPersistence-16      9K ops/sec   112.3 µs/op    2383 B/op    36 allocs
 ```
 
+## Cloud Datastore TTL Setup
+
+When using Google Cloud Datastore persistence, configure native TTL policies for automatic expiration:
+
+### One-time Setup (per database)
+
+```bash
+# Enable TTL on the 'expiry' field for CacheEntry kind
+gcloud firestore fields ttls update expiry \
+  --collection-group=CacheEntry \
+  --enable-ttl \
+  --database=YOUR_CACHE_ID
+```
+
+**Important:**
+- Replace `YOUR_CACHE_ID` with your cache ID (passed to `WithCloudDatastore()`)
+- This is a one-time setup per database
+- Datastore automatically deletes expired entries within 24 hours
+- No indexing needed on the expiry field (prevents hotspots)
+
+### Best Practices
+
+1. **Use Native TTL**: Let Datastore handle expiration automatically
+2. **Add Cleanup Fallback**: Use `WithCleanup()` as a safety net:
+   ```go
+   cache, err := bdcache.New[string, User](ctx,
+       bdcache.WithCloudDatastore("myapp"),
+       bdcache.WithCleanup(24*time.Hour), // Safety net for orphaned data
+   )
+   ```
+3. **Set Cleanup MaxAge**: Should match your longest TTL value
+4. **Monitor Costs**: TTL deletions count toward entity delete operations
+
+If native TTL is properly configured, `WithCleanup()` will find no entries (fast no-op).
+
 ## License
 
 Apache 2.0

benchmarks/hitrate_comparison_test.go

@@ -1,6 +1,5 @@
 package benchmarks
 
-
 import (
 	"context"
 	"fmt"

cache.go

@@ -50,6 +50,20 @@ func New[K comparable, V any](ctx context.Context, options ...Option) (*Cache[K,
 			}
 		}
 
+		// Run background cleanup if configured
+		if cache.persist != nil && opts.CleanupEnabled {
+			go func() {
+				deleted, err := cache.persist.Cleanup(ctx, opts.CleanupMaxAge)
+				if err != nil {
+					slog.Warn("error during cache cleanup", "error", err)
+					return
+				}
+				if deleted > 0 {
+					slog.Info("cache cleanup complete", "deleted", deleted)
+				}
+			}()
+		}
+
 		// Warm up cache from persistence if configured
 		if cache.persist != nil && opts.WarmupLimit > 0 {
 			go cache.warmup(ctx)
@@ -85,22 +99,22 @@ func (c *Cache[K, V]) warmup(ctx context.Context) {
 
 // Get retrieves a value from the cache.
 // It first checks the memory cache, then falls back to persistence if available.
-func (c *Cache[K, V]) Get(ctx context.Context, key K) (value V, found bool, err error) {
+func (c *Cache[K, V]) Get(ctx context.Context, key K) (V, bool, error) {
 	// Check memory first
 	if val, ok := c.memory.get(key); ok {
 		return val, true, nil
 	}
 
+	var zero V
+
 	// If no persistence, return miss
 	if c.persist == nil {
-		var zero V
 		return zero, false, nil
 	}
 
 	// Validate key before accessing persistence (security: prevent path traversal)
 	if err := c.persist.ValidateKey(key); err != nil {
 		slog.Warn("invalid key for persistence", "error", err, "key", key)
-		var zero V
 		return zero, false, nil
 	}
 
@@ -109,12 +123,10 @@ func (c *Cache[K, V]) Get(ctx context.Context, key K) (value V, found bool, err
 	if err != nil {
 		// Log error but don't fail - graceful degradation
 		slog.Warn("persistence load failed", "error", err, "key", key)
-		var zero V
 		return zero, false, nil
 	}
 
 	if !found {
-		var zero V
 		return zero, false, nil
 	}
 

cache_test.go

@@ -751,6 +751,10 @@ func (e *errorPersist[K, V]) ValidateKey(key K) error {
 	return nil // Allow all keys for test
 }
 
+func (e *errorPersist[K, V]) Cleanup(ctx context.Context, maxAge time.Duration) (int, error) {
+	return 0, context.DeadlineExceeded
+}
+
 func BenchmarkCache_Set_WithPersistence(b *testing.B) {
 	ctx := context.Background()
 	cacheID := "bench-persist-" + time.Now().Format("20060102150405")

options.go

@@ -7,11 +7,13 @@ import (
 
 // Options configures a Cache instance.
 type Options struct {
-	CacheID      string
-	MemorySize   int
-	DefaultTTL   time.Duration
-	WarmupLimit  int
-	UseDatastore bool
+	CacheID        string
+	MemorySize     int
+	DefaultTTL     time.Duration
+	WarmupLimit    int
+	UseDatastore   bool
+	CleanupEnabled bool
+	CleanupMaxAge  time.Duration
 }
 
 // Option is a functional option for configuring a Cache.
@@ -55,11 +57,7 @@ func WithCloudDatastore(cacheID string) Option {
 func WithBestStore(cacheID string) Option {
 	return func(o *Options) {
 		o.CacheID = cacheID
-		if os.Getenv("K_SERVICE") != "" {
-			o.UseDatastore = true
-		} else {
-			o.UseDatastore = false
-		}
+		o.UseDatastore = os.Getenv("K_SERVICE") != ""
 	}
 }
 
@@ -71,13 +69,20 @@ func WithWarmup(n int) Option {
 	}
 }
 
+// WithCleanup enables background cleanup of expired entries at startup.
+// maxAge should be set to your maximum TTL value - entries older than this are deleted.
+// This is a safety net for expired data and works alongside native Datastore TTL policies.
+// If native TTL is properly configured, this cleanup will be fast (no-op).
+func WithCleanup(maxAge time.Duration) Option {
+	return func(o *Options) {
+		o.CleanupEnabled = true
+		o.CleanupMaxAge = maxAge
+	}
+}
+
 // defaultOptions returns the default configuration (memory-only).
 func defaultOptions() *Options {
 	return &Options{
-		MemorySize:   10000,
-		DefaultTTL:   0,
-		CacheID:      "",
-		UseDatastore: false,
-		WarmupLimit:  0,
+		MemorySize: 10000,
 	}
 }

persist.go

@@ -31,6 +31,11 @@ type PersistenceLayer[K comparable, V any] interface {
 	// Equivalent to LoadRecent(ctx, 0).
 	LoadAll(ctx context.Context) (<-chan Entry[K, V], <-chan error)
 
+	// Cleanup removes expired entries from persistent storage.
+	// maxAge specifies how old entries must be before deletion.
+	// Returns the number of entries deleted and any error.
+	Cleanup(ctx context.Context, maxAge time.Duration) (int, error)
+
 	// Close releases any resources held by the persistence layer.
 	Close() error
 }

persist_datastore.go

@@ -82,18 +82,9 @@ func (d *datastorePersist[K, V]) Load(ctx context.Context, key K) (V, time.Time,
 		return zero, time.Time{}, false, fmt.Errorf("datastore get: %w", err)
 	}
 
-	// Check expiration
+	// Check expiration - return miss but don't delete
+	// Cleanup is handled by native Datastore TTL or periodic Cleanup() calls
 	if !entry.Expiry.IsZero() && time.Now().After(entry.Expiry) {
-		// Delete expired entry asynchronously with timeout
-		// Note: Using context.Background() intentionally - cleanup should continue even if parent context is cancelled
-		go func(_ context.Context) {
-			//nolint:contextcheck // Using Background intentionally for independent cleanup goroutine
-			cleanupCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
-			defer cancel()
-			if err := d.client.Delete(cleanupCtx, dsKey); err != nil {
-				slog.Debug("failed to delete expired entry", "error", err)
-			}
-		}(ctx)
 		return zero, time.Time{}, false, nil
 	}
 
@@ -187,17 +178,8 @@ func (d *datastorePersist[K, V]) LoadRecent(ctx context.Context, limit int) (<-c
 			default:
 			}
 
-			// Clean up expired entries asynchronously with timeout
+			// Skip expired entries - cleanup is handled by native TTL or periodic Cleanup() calls
 			if !entry.Expiry.IsZero() && now.After(entry.Expiry) {
-				// Note: Using context.Background() intentionally - cleanup should continue even if parent context is cancelled
-				go func(_ context.Context, key *datastore.Key) {
-					//nolint:contextcheck // Using Background intentionally for independent cleanup goroutine
-					cleanupCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
-					defer cancel()
-					if err := d.client.Delete(cleanupCtx, key); err != nil {
-						slog.Debug("failed to delete expired entry", "error", err)
-					}
-				}(ctx, dsKey)
 				expired++
 				continue
 			}
@@ -250,6 +232,37 @@ func (d *datastorePersist[K, V]) LoadAll(ctx context.Context) (<-chan Entry[K, V
 	return d.LoadRecent(ctx, 0)
 }
 
+// Cleanup removes expired entries from Datastore.
+// maxAge specifies how old entries must be (based on expiry field) before deletion.
+// If native Datastore TTL is properly configured, this will find no entries.
+func (d *datastorePersist[K, V]) Cleanup(ctx context.Context, maxAge time.Duration) (int, error) {
+	cutoff := time.Now().Add(-maxAge)
+
+	// Query for entries with expiry before cutoff
+	// This finds entries that should have expired based on maxAge
+	query := datastore.NewQuery(d.kind).
+		Filter("expiry >", time.Time{}).
+		Filter("expiry <", cutoff).
+		KeysOnly()
+
+	keys, err := d.client.GetAll(ctx, query, nil)
+	if err != nil {
+		return 0, fmt.Errorf("query expired entries: %w", err)
+	}
+
+	if len(keys) == 0 {
+		return 0, nil
+	}
+
+	// Batch delete expired entries
+	if err := d.client.DeleteMulti(ctx, keys); err != nil {
+		return 0, fmt.Errorf("delete expired entries: %w", err)
+	}
+
+	slog.Info("cleaned up expired entries", "count", len(keys), "kind", d.kind)
+	return len(keys), nil
+}
+
 // Close releases Datastore client resources.
 func (d *datastorePersist[K, V]) Close() error {
 	return d.client.Close()

persist_file.go

@@ -342,6 +342,72 @@ func (f *filePersist[K, V]) LoadAll(ctx context.Context) (<-chan Entry[K, V], <-
 	return f.LoadRecent(ctx, 0)
 }
 
+// Cleanup removes expired entries from file storage.
+// Walks through all cache files and deletes those with expired timestamps.
+func (f *filePersist[K, V]) Cleanup(ctx context.Context, maxAge time.Duration) (int, error) {
+	cutoff := time.Now().Add(-maxAge)
+	deleted := 0
+
+	entries, err := os.ReadDir(f.dir)
+	if err != nil {
+		if os.IsNotExist(err) {
+			return 0, nil
+		}
+		return 0, fmt.Errorf("read cache directory: %w", err)
+	}
+
+	for _, entry := range entries {
+		// Check context cancellation
+		select {
+		case <-ctx.Done():
+			return deleted, ctx.Err()
+		default:
+		}
+
+		if entry.IsDir() {
+			continue
+		}
+
+		filename := filepath.Join(f.dir, entry.Name())
+
+		// Read and check expiry
+		file, err := os.Open(filename)
+		if err != nil {
+			if os.IsNotExist(err) {
+				continue
+			}
+			slog.Debug("failed to open file for cleanup", "file", filename, "error", err)
+			continue
+		}
+
+		var entry Entry[K, V]
+		decoder := gob.NewDecoder(file)
+		err = decoder.Decode(&entry)
+		if closeErr := file.Close(); closeErr != nil {
+			slog.Debug("failed to close file during cleanup", "file", filename, "error", closeErr)
+		}
+
+		if err != nil {
+			slog.Debug("failed to decode file for cleanup", "file", filename, "error", err)
+			continue
+		}
+
+		// Delete if expired
+		if !entry.Expiry.IsZero() && entry.Expiry.Before(cutoff) {
+			if err := os.Remove(filename); err != nil && !os.IsNotExist(err) {
+				slog.Debug("failed to remove expired file", "file", filename, "error", err)
+				continue
+			}
+			deleted++
+		}
+	}
+
+	if deleted > 0 {
+		slog.Info("cleaned up expired file entries", "count", deleted, "dir", f.dir)
+	}
+	return deleted, nil
+}
+
 // Close cleans up resources.
 func (*filePersist[K, V]) Close() error {
 	// No resources to clean up for file-based persistence