Wait for the least busy replica when every replica's pool is full, instead of reading from the primary

The primary takes every write, so it should not also take the reads of stalled replicas.

Claude-Session: https://claude.ai/code/session_01DeXuhSjq5Gta5qPJgSceFe

db/db.go

@@ -18,7 +18,7 @@ const (
 
 // DB wraps a primary *sql.DB and optional read replicas. All standard query/exec methods
 // delegate to the primary. The ReadOnly() method returns a *sql.DB from the least busy healthy
-// replica, falling back to the primary if none is configured, healthy, or has a free connection.
+// replica, falling back to the primary if none is configured or healthy.
 type DB struct {
 	primary  *Host
 	replicas []*Host
@@ -113,8 +113,9 @@ func (d *DB) Stats() []*PoolStats {
 
 // ReadOnly returns a *sql.DB suitable for read-only queries: the healthy replica with the fewest
 // connections in use, round-robin among equally busy ones. A replica that stalls piles up
-// connections and stops getting new reads. If no replica is healthy or every replica's pool is
-// full (a read would wait for a connection), the primary is returned.
+// connections and stops getting new reads. If every replica's pool is full, the read waits for a
+// connection rather than moving to the primary, which takes every write. If no replica is healthy
+// or none is configured, the primary is returned.
 func (d *DB) ReadOnly() *sql.DB {
 	if len(d.replicas) == 0 {
 		return d.primary.DB
@@ -128,12 +129,9 @@ func (d *DB) ReadOnly() *sql.DB {
 		if !r.healthy.Load() {
 			continue
 		}
-		stats := r.DB.Stats()
-		if stats.MaxOpenConnections > 0 && stats.InUse >= stats.MaxOpenConnections {
-			continue
-		}
-		if best == nil || stats.InUse < bestInUse {
-			best, bestInUse = r, stats.InUse
+		inUse := r.DB.Stats().InUse
+		if best == nil || inUse < bestInUse {
+			best, bestInUse = r, inUse
 		}
 	}
 	if best == nil {

db/db_test.go

@@ -130,19 +130,26 @@ func TestDB_ReadOnly_RoundRobinWhenEquallyBusy(t *testing.T) {
 	require.Equal(t, 5, seen[r2])
 }
 
-func TestDB_ReadOnly_PrimaryWhenReplicaPoolsFull(t *testing.T) {
-	// A read on a full pool would wait for a connection; the primary serves it instead
+func TestDB_ReadOnly_ReplicaWhenReplicaPoolsFull(t *testing.T) {
+	// Full pools wait for a connection on the least busy replica; the primary, which takes every
+	// write, does not take the reads of stalled replicas
 	primary := openSQLite(t, "primary.db", 5)
-	replica := openSQLite(t, "replica.db", 1)
-	d := db.New(&db.Host{DB: primary}, []*db.Host{{DB: replica}})
+	r1 := openSQLite(t, "r1.db", 1)
+	r2 := openSQLite(t, "r2.db", 2)
+	d := db.New(&db.Host{DB: primary}, []*db.Host{{DB: r1}, {DB: r2}})
 	defer d.Close()
 	d.MarkReplicasHealthyForTest()
 
-	require.Same(t, replica, d.ReadOnly())
-	conn, err := replica.Conn(context.Background())
-	require.Nil(t, err)
-	defer conn.Close()
-	require.Same(t, primary, d.ReadOnly())
+	// Fill both pools, r1 with one connection, r2 with two
+	held := []*sql.DB{r1, r2, r2}
+	for _, replica := range held {
+		conn, err := replica.Conn(context.Background())
+		require.Nil(t, err)
+		defer conn.Close()
+	}
+	for i := 0; i < 10; i++ {
+		require.Same(t, r1, d.ReadOnly())
+	}
 }
 
 func TestDB_Stats_PingDuration(t *testing.T) {

docs/config.md

@@ -439,8 +439,7 @@ To offload read-heavy queries from the primary database, you can optionally conf
 using the `database-replica-urls` option. When configured, non-critical read-only queries (e.g. fetching messages, checking access permissions, etc)
 go to the replica with the fewest connections in use (round-robin among equally busy ones), while all writes and
 correctness-critical reads continue to go to the primary. A replica that slows down piles up connections and stops
-getting new reads until it catches up. If a replica becomes unhealthy, or every replica's connection pool is full, ntfy
-falls back to the primary.
+getting new reads until it catches up. If every replica becomes unhealthy, ntfy falls back to the primary.
 You can also set this via the environment variable `NTFY_DATABASE_REPLICA_URLS` (comma-separated) or the command line
 flag `--database-replica-urls`.
 

docs/releases.md

@@ -2358,7 +2358,7 @@ and the [ntfy Android app](https://github.com/binwiederhier/ntfy-android/release
 * Do not send TCP keep-alive probes on the client listeners (`listen-http`, `listen-https`): Go sends one every 15 seconds on an idle connection by default, which, without a proxy in front of ntfy, woke phones holding a stream three times as often as ntfy's own keepalive event ([#2152](https://github.com/binwiederhier/ntfy/pull/2152))
 * `ntfy cluster status` lists the node itself as the first row of the table (marked "this node", and "leader" if it leads) instead of a line above it ([#2153](https://github.com/binwiederhier/ntfy/pull/2153))
 * The cluster and admin socket options lost their `experimental-` prefix, e.g. `cluster-listen` instead of `experimental-cluster-listen` (and `NTFY_CLUSTER_LISTEN`); the old names still work ([#2154](https://github.com/binwiederhier/ntfy/pull/2154))
-* Send read-only queries to the least busy read replica instead of round-robin, so a replica that stalls stops getting new reads, and to the primary when every replica's pool is full; the new `ntfy_db_ping_seconds` metric shows how long each database takes to answer its health check ([#2156](https://github.com/binwiederhier/ntfy/pull/2156))
+* Send read-only queries to the least busy read replica instead of round-robin, so a replica that stalls stops getting new reads; the new `ntfy_db_ping_seconds` metric shows how long each database takes to answer its health check ([#2156](https://github.com/binwiederhier/ntfy/pull/2156))
 
 ### ntfy iOS app v1.8.0 (UNRELEASED)