fix(tiering): cache negative policy lookups

RELEASE_NOTES_2026.06.2.md

@@ -47,6 +47,8 @@ Line Protocol and TLE imports are unchanged. CSV/Parquet imports no longer depen
 
 **S3 and Azure listing loops now check context cancellation.** `List`, `ListDirectories`, and `ListObjects` on both `S3Backend` and `AzureBlobBackend` previously did not check `ctx.Err()` between paginated API calls. On very large prefixes (many pages), a cancelled or timed-out context would not propagate until the next SDK call, leaving the loop running longer than necessary. Every pagination loop now checks context cancellation at the top of each iteration, returning immediately when the context is done.
 
+**Tiering policy lookups for databases without a custom policy no longer hit SQLite on every call (#345).** The tiering policy cache only stored found policies, despite a code comment claiming otherwise — so every lookup for a database using the global defaults (the common case) queried SQLite, twice per file per migration cycle. Not-found results are now cached alongside found ones; creating or deleting a policy invalidates the entry exactly as before, and a concurrent policy creation racing a lookup can no longer be shadowed by a stale not-found entry. Enterprise-only (tiering). One edge-case behavior change: a policy row written to SQLite outside the API (manual edit, restore-from-backup) was previously picked up by the next uncached lookup; with not-found results cached, it now requires a restart (or a `PUT` through the API) to become visible.
+
 **Tiered storage migration history now has periodic cleanup.** The `tier_migrations` metadata table previously grew without bound — every migration attempt (successful or failed) was recorded and never cleaned up. Each migration cycle now deletes records older than `[tiered_storage].migration_history_retention_days` (default: 90 days). OSS deployments are unaffected (tiering requires an enterprise license).
 
 **WAL reader now uses `io.ReadFull` for fixed-size header reads.** The WAL reader previously used `f.Read` to read fixed-size file and entry headers. `f.Read` may return fewer bytes than the buffer size without an error, which could cause partial header reads that cascade into misaligned subsequent reads — corrupting all entries after the partial read. Both header reads now use `io.ReadFull`, which guarantees the buffer is filled completely or returns an error.

internal/tiering/policy.go

@@ -118,12 +118,24 @@ func (s *PolicyStore) Get(ctx context.Context, database string) (*DatabasePolicy
 		return nil, err
 	}
 
-	// Cache the result (even if nil, we cache to avoid repeated DB lookups)
-	if policy != nil {
-		s.mu.Lock()
-		s.cache[database] = policy
-		s.mu.Unlock()
+	// Cache the result — including nil (not-found): most databases have no
+	// custom policy, and an uncached negative would otherwise hit SQLite on
+	// every lookup, twice per file per migration cycle (#345). A cached nil
+	// returns (nil, nil) on the next Get via the two-value map read above —
+	// the same not-found contract as an uncached miss. Set overwrites the
+	// entry and Delete removes it, so invalidation is unchanged.
+	//
+	// Double-check under the write lock: getFromDB ran outside the lock, so
+	// a concurrent Set may have inserted the policy and cached it after our
+	// DB read returned no row. Its fresher entry must not be overwritten —
+	// a stale cached nil would silently disable the policy until the next
+	// Set, with no self-healing re-query. Return the fresher entry too.
+	s.mu.Lock()
+	defer s.mu.Unlock()
+	if cached, raced := s.cache[database]; raced {
+		return cached, nil
 	}
+	s.cache[database] = policy
 
 	return policy, nil
 }
@@ -205,8 +217,23 @@ func (s *PolicyStore) List(ctx context.Context) ([]DatabasePolicy, error) {
 	s.mu.RLock()
 	defer s.mu.RUnlock()
 
-	policies := make([]DatabasePolicy, 0, len(s.cache))
+	// Size the slice from the non-nil count: the cache also holds nil
+	// not-found entries (#345), often the majority, and len(s.cache) would
+	// over-allocate. Both passes run under the same RLock.
+	var count int
+	for _, policy := range s.cache {
+		if policy != nil {
+			count++
+		}
+	}
+
+	policies := make([]DatabasePolicy, 0, count)
 	for _, policy := range s.cache {
+		// Skip cached negatives: nil marks "no custom policy" (#345) and
+		// must not be dereferenced or listed.
+		if policy == nil {
+			continue
+		}
 		policies = append(policies, *policy)
 	}
 	return policies, nil

internal/tiering/policy_test.go

@@ -264,3 +264,116 @@ func TestPolicyStore_Update(t *testing.T) {
 		t.Error("HotOnly = false, want true")
 	}
 }
+
+// TestPolicyStore_NegativeLookupCached verifies that a not-found lookup is
+// cached (#345). The proof is behavioral: after the first Get caches the
+// negative, a row inserted directly via SQL (bypassing Set, so the cache
+// never learns) must stay invisible to subsequent Gets.
+func TestPolicyStore_NegativeLookupCached(t *testing.T) {
+	store, cleanup := setupTestPolicyStore(t)
+	defer cleanup()
+
+	ctx := context.Background()
+
+	policy, err := store.Get(ctx, "nocustom")
+	if err != nil {
+		t.Fatalf("Get() error = %v", err)
+	}
+	if policy != nil {
+		t.Fatalf("Get() for nonexistent policy = %+v, want nil", policy)
+	}
+
+	// Insert behind the store's back. If the negative lookup was cached,
+	// the store must keep returning nil; if it re-queries SQLite, this row
+	// leaks through and the test fails.
+	_, err = store.db.Exec(
+		`INSERT INTO tiering_policies (database, hot_only, updated_at) VALUES (?, 1, CURRENT_TIMESTAMP)`,
+		"nocustom",
+	)
+	if err != nil {
+		t.Fatalf("direct insert failed: %v", err)
+	}
+
+	policy, err = store.Get(ctx, "nocustom")
+	if err != nil {
+		t.Fatalf("Get() after direct insert error = %v", err)
+	}
+	if policy != nil {
+		t.Errorf("Get() after direct insert = %+v, want nil (negative lookup should be served from cache, not SQLite)", policy)
+	}
+}
+
+// TestPolicyStore_SetOverridesCachedNegative verifies that Set invalidates a
+// previously cached not-found entry (#345).
+func TestPolicyStore_SetOverridesCachedNegative(t *testing.T) {
+	store, cleanup := setupTestPolicyStore(t)
+	defer cleanup()
+
+	ctx := context.Background()
+
+	policy, err := store.Get(ctx, "latecomer")
+	if err != nil {
+		t.Fatalf("Get() error = %v", err)
+	}
+	if policy != nil {
+		t.Fatalf("Get() for nonexistent policy = %+v, want nil", policy)
+	}
+
+	hotDays := 3
+	if err := store.Set(ctx, &DatabasePolicy{
+		Database:      "latecomer",
+		HotMaxAgeDays: &hotDays,
+	}); err != nil {
+		t.Fatalf("Set() error = %v", err)
+	}
+
+	policy, err = store.Get(ctx, "latecomer")
+	if err != nil {
+		t.Fatalf("Get() after Set error = %v", err)
+	}
+	if policy == nil {
+		t.Fatal("Get() after Set = nil, want policy (Set must overwrite the cached negative)")
+	}
+	if policy.HotMaxAgeDays == nil || *policy.HotMaxAgeDays != hotDays {
+		t.Errorf("Get() after Set HotMaxAgeDays = %v, want %d", policy.HotMaxAgeDays, hotDays)
+	}
+}
+
+// TestPolicyStore_ListSkipsCachedNegatives is the regression test for the
+// review blocker on #345: List() dereferences every cache value, and a
+// cached negative (nil) must not panic it or appear in the listing.
+func TestPolicyStore_ListSkipsCachedNegatives(t *testing.T) {
+	store, cleanup := setupTestPolicyStore(t)
+	defer cleanup()
+
+	ctx := context.Background()
+
+	// Cache a negative, then a real policy.
+	if policy, err := store.Get(ctx, "defaultsonly"); err != nil || policy != nil {
+		t.Fatalf("Get() = (%+v, %v), want (nil, nil)", policy, err)
+	}
+	hotDays := 5
+	if err := store.Set(ctx, &DatabasePolicy{Database: "custom", HotMaxAgeDays: &hotDays}); err != nil {
+		t.Fatalf("Set() error = %v", err)
+	}
+
+	policies, err := store.List(ctx)
+	if err != nil {
+		t.Fatalf("List() error = %v", err)
+	}
+	if len(policies) != 1 {
+		t.Fatalf("List() returned %d policies, want 1 (cached negative must be skipped)", len(policies))
+	}
+	if policies[0].Database != "custom" {
+		t.Errorf("List()[0].Database = %q, want %q", policies[0].Database, "custom")
+	}
+
+	// Derived lookups must treat a cached negative as "use defaults".
+	if store.IsHotOnly(ctx, "defaultsonly") {
+		t.Error("IsHotOnly() after cached negative = true, want false")
+	}
+	effective := store.GetEffective(ctx, "defaultsonly")
+	if effective == nil || effective.Source != "global" {
+		t.Errorf("GetEffective() after cached negative = %+v, want global defaults", effective)
+	}
+}