Let metrics reads run concurrently with writes instead of queueing

The metrics store capped its SQLite pool at one connection, so every UI
history read queued behind every buffered-write commit, and behind the
WAL checkpoints those commits pick up at the 4000-page threshold. On
write-heavy installs (many Docker agents with churning containers) that
serialization presented as sustained 120-260ms COMMIT warnings and an
unresponsive UI even with idle CPU and fast disks. Writes were never the
risk: flush, rollup, retention, and maintenance already funnel through
the single background worker goroutine, and the WriteBatchSync poller
path serializes on the WAL write lock via busy_timeout.

Raising the pool exposed a second bug: auto_vacuum(INCREMENTAL) in the
per-connection DSN pragmas replays as a database-header write whenever
the pool opens a new connection, which blocks connection creation behind
the active writer for up to the full 30s busy_timeout. auto_vacuum is a
persistent database property that migrateAutoVacuum already establishes
once at startup, so the per-connection copy is dropped.

Refs #1601

Contract-Neutral: behavioral fix: metrics store read concurrency and per-connection auto_vacuum pragma removal, no public contract delta (#1601)

pkg/metrics/issue1601_read_concurrency_test.go

@@ -0,0 +1,57 @@
+package metrics
+
+import (
+	"testing"
+	"time"
+)
+
+// A connection pool of one queued every UI history read behind metric-flush
+// commits, freezing charts whenever a commit picked up a WAL checkpoint
+// (#1601). Reads must proceed on their own connections while a write
+// transaction holds the WAL write lock.
+func TestQueriesDoNotQueueBehindWriteTransaction(t *testing.T) {
+	store, err := NewStore(DefaultConfig(t.TempDir()))
+	if err != nil {
+		t.Fatalf("NewStore: %v", err)
+	}
+	defer store.Close()
+
+	// Let startup maintenance (checkpoint/vacuum) finish so its locks don't
+	// entangle with the write transaction opened below.
+	if err := store.WaitForMaintenance(10 * time.Second); err != nil {
+		t.Fatalf("WaitForMaintenance: %v", err)
+	}
+
+	if got := store.db.DB.Stats().MaxOpenConnections; got < 2 {
+		t.Fatalf("MaxOpenConnections = %d, want at least 2 so reads do not serialize behind writes", got)
+	}
+
+	// Hold the WAL write lock on one connection.
+	tx, err := store.db.Begin()
+	if err != nil {
+		t.Fatalf("Begin: %v", err)
+	}
+	defer func() { _ = tx.Rollback() }()
+	if _, err := tx.Exec(
+		`INSERT INTO metrics (resource_type, resource_id, metric_type, value, timestamp, tier)
+		 VALUES ('node', 'n1', 'cpu', 1.0, ?, 'raw')`,
+		time.Now().Unix(),
+	); err != nil {
+		t.Fatalf("write inside transaction: %v", err)
+	}
+
+	done := make(chan error, 1)
+	go func() {
+		_, err := store.Query("node", "n1", "cpu", time.Now().Add(-time.Hour), time.Now(), 60)
+		done <- err
+	}()
+
+	select {
+	case err := <-done:
+		if err != nil {
+			t.Fatalf("Query during open write transaction: %v", err)
+		}
+	case <-time.After(5 * time.Second):
+		t.Fatal("Query blocked behind the open write transaction")
+	}
+}

pkg/metrics/store.go

@@ -258,7 +258,11 @@ func NewStore(config StoreConfig) (*Store, error) {
 			"busy_timeout(30000)",
 			"journal_mode(WAL)",
 			"synchronous(NORMAL)",
-			"auto_vacuum(INCREMENTAL)",
+			// auto_vacuum is deliberately NOT set here. It is a persistent
+			// database property that migrateAutoVacuum establishes once at
+			// startup; as a per-connection pragma it replays as a header
+			// write on every new pool connection, which blocks connection
+			// creation behind the active writer (#1601).
 			// Checkpoint less aggressively so high-cardinality installs don't
 			// keep rewriting tiny WAL segments back into the main DB file.
 			"wal_autocheckpoint(4000)",
@@ -269,9 +273,15 @@ func NewStore(config StoreConfig) (*Store, error) {
 		return nil, fmt.Errorf("failed to open metrics database: %w", err)
 	}
 
-	// Configure connection pool (SQLite works best with single writer)
-	rawDB.SetMaxOpenConns(1)
-	rawDB.SetMaxIdleConns(1)
+	// Configure connection pool. Writes are already funnelled: the flush,
+	// rollup, retention, and maintenance paths run on the single background
+	// worker goroutine, and the WriteBatchSync poller path serializes on the
+	// WAL write lock via busy_timeout. What a pool of one actually did was
+	// queue every UI history read behind those commits, which froze charts
+	// whenever a commit picked up a WAL checkpoint (#1601). WAL mode reads
+	// snapshot-isolated on their own connections, so give readers room.
+	rawDB.SetMaxOpenConns(4)
+	rawDB.SetMaxIdleConns(4)
 	rawDB.SetConnMaxLifetime(0)
 
 	db := pdb.Wrap(rawDB, "metrics")