diff --git a/db/db.go b/db/db.go index 66169e6d..dd5600d7 100644 --- a/db/db.go +++ b/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 { diff --git a/db/db_test.go b/db/db_test.go index de2a9983..ae56d0ec 100644 --- a/db/db_test.go +++ b/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) { diff --git a/docs/config.md b/docs/config.md index f9d7f223..d4215f20 100644 --- a/docs/config.md +++ b/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`. diff --git a/docs/releases.md b/docs/releases.md index 33df0e2c..a669df0e 100644 --- a/docs/releases.md +++ b/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)