Race condition in pool_available metric causes negative values during network instability

[DavraYoung]

Problem Description

The pgrst_db_pool_available metric can drift into negative values during network instability or connection pool disruptions. We observed values as low as -233 with pgrst_db_pool_max: 80, which is clearly incorrect.

Root Cause

The race condition is in src/PostgREST/Metrics.hs:35-41:

(HasqlPoolObs (SQL.ConnectionObservation _ status)) -> case status of
  SQL.ReadyForUseConnectionStatus  -> do
    incGauge poolAvailable
  SQL.InUseConnectionStatus        -> do
    decGauge poolAvailable
  SQL.TerminatedConnectionStatus  _ -> do
    decGauge poolAvailable
  SQL.ConnectingConnectionStatus -> pure ()

The incGauge and decGauge operations from prometheus-client are not atomic. During network instability:

1. Multiple connections transition states simultaneously
2. decGauge operations can occur before corresponding incGauge operations
3. Connections that terminate before becoming "ready" decrement without ever incrementing
4. The gauge drifts negative over time

Impact

• Metrics are unreliable - monitoring/alerting based on pool_available produces false negatives
• No actual pool impact - the underlying pool works correctly; only the metric is corrupted
• Persists until restart - the counter never self-corrects; requires PostgREST restart

Environment

• PostgREST version: v12.2.12
• prometheus-client constraint: >= 1.1.1 && < 1.2.0
• hasql-pool constraint: >= 1.0.1 && < 1.1
• Trigger: Network instability during Google GCE incident

Suggested Fix

The gauge updates need to be atomic. Options include:

1. Use STM - Wrap the gauge in an TVar for atomic updates
2. Use atomic operations - If prometheus-client supports atomic inc/dec
3. Track absolute count - Calculate available from (total - in_use) instead of incrementing
4. Add mutex/lock - Protect gauge updates with a lock (less ideal for performance)

Workaround

Restart PostgREST to reset the counter to correct values.

[steve-chavez]

Another thing that likely is wrong is that we're decrementing the gauge for all SQL.TerminatedConnectionStatus states:

-- | Status of a connection.
data ConnectionStatus
  = -- | Connection is being established.
    --
    -- This is the initial status of every connection.
    ConnectingConnectionStatus
  | -- | Connection is established and not occupied.
    ReadyForUseConnectionStatus
  | -- | Is being used by some session.
    --
    -- After it's done the status will transition to 'ReadyForUseConnectionStatus' or 'TerminatedConnectionStatus'.
    InUseConnectionStatus
  | -- | Connection terminated.
    TerminatedConnectionStatus ConnectionTerminationReason
  deriving (Show, Eq)

-- | Explanation of why a connection was terminated.
data ConnectionTerminationReason
  = -- | The age timeout of the connection has passed.
    AgingConnectionTerminationReason
  | -- | The timeout of how long a connection may remain idle in the pool has passed.
    IdlenessConnectionTerminationReason
  | -- | Connectivity issues with the server.
    NetworkErrorConnectionTerminationReason (Maybe Text)
  | -- | User has invoked the 'Hasql.Pool.release' procedure.
    ReleaseConnectionTerminationReason
  deriving (Show, Eq)

But on ReleaseConnectionTerminationReason maybe should reset the gauge to 0 instead. This would need some experimentation (maybe by printing all the states on log-level=debug and checking).

[taimoorzaeem]

The incGauge and decGauge operations from prometheus-client are not atomic

Suggested Fix

1. ...
2. Use atomic operations - If prometheus-client supports atomic inc/dec

Hm, I looked at the code upstream and it looks like they are already using atomic operations provided by atomic-primops library:

withGauge :: MonadMonitor m => Gauge -> (Double -> Double) -> m ()
withGauge (MkGauge ioref) f =
    doIO $ Atomics.atomicModifyIORefCAS_ ioref f

-- | Increments a gauge metric by 1.
incGauge :: MonadMonitor m => Gauge -> m ()
incGauge g = withGauge g (+ 1)

-- | Decrements a gauge metric by 1.
decGauge :: MonadMonitor m => Gauge -> m ()
decGauge g = withGauge g (+ (-1))