PgConnectOptions::statement_cache_capacity(0) causes PG connection backends (in db server) to leak memory

[milankinen]

I have found these related issues/pull requests

#3850 somehow related since it fixes individual non-persistent query memory leak but it does not affect the behaviour set by this global connect options setup.

Description

Hello!

We've been using PgConnectOptions::statement_cache_capacity(0) to "globally" disable statement caching. Recently we found that our Postgresql connections started accumulate (or "leak") memory at the db server side over time for long lived connections. Debugging revealed that this memory usage was caused by connection adding prepared statement for every query.

I investigated that the root cause is that Postgres executor assigns statement id based on the query persistence so that persistent queries always get statement id whereas non-persistent ones do not (previously also non-persistent statements got id but that was fixed by #3850) . Statement with assigned ids are not cleared by pg protocol, hence they start accumulating to memory.

However, it seems that the prepared statement cache LRU eviction is disabled if statement cache capacity is set to 0. This asymmetry now causes that setting cache size to 0 causes persistent (the default case) statements to get a statement id but not be evicted from the backend.

I'm not 100% sure if this a bug or desired behaviour but at least from the user's perspective it is very surprising. A potential (and easy) fix would be to also check the statement cache enablement for statement name assignment. If that breaks some defined behaviour, could this at least be documented to the statement_cache_capacity rustdoc so that when using value 0, user must always use .persistent(false) queries to prevent memory leaking at the db server side?

I can reproduce this with at least 0.8.6 and 0.9.0.

BTW, thanks for the awesome library! Its abstractions and feature set are exactly how I would have implemented myself as well!

Reproduction steps

use std::str::FromStr;
use std::time::Duration;

use sqlx::ConnectOptions;
use sqlx::postgres::PgConnectOptions;

#[tokio::main]
async fn main() -> Result<(), sqlx::Error> {
    let options = PgConnectOptions::from_str("postgres://postgres:postgres@localhost:5432/postgres")?
        // Removing (defaults to 100) or setting to >= 1 fixes the leak
        .statement_cache_capacity(0);

    let mut conn = options.connect().await?;

    let pid: i32 = sqlx::query_scalar("SELECT pg_backend_pid()")
        .persistent(false)
        .fetch_one(&mut conn)
        .await?;
    println!("backend pid={pid}");

    let mut iterations: u64 = 0;
    loop {
        // Using explicit .persistent(false) fixes the leak
        sqlx::query("SELECT 1")
            //.persistent(false)
            .execute(&mut conn)
            .await?;
        iterations += 1;

        if iterations % 1000 == 0 {
            let live: i64 = sqlx::query_scalar("SELECT count(*) FROM pg_prepared_statements")
                .persistent(false)
                .fetch_one(&mut conn)
                .await?;
            let backend_mem: String =
                sqlx::query_scalar("SELECT pg_size_pretty(sum(total_bytes)) FROM pg_backend_memory_contexts")
                    .persistent(false)
                    .fetch_one(&mut conn)
                    .await?;
            println!(
                "iterations={iterations:>9}  live_prepared_statements={live}  backend_mem={backend_mem}"
            );
            tokio::time::sleep(Duration::from_millis(50)).await;
        }
    }
}

# Cargo.toml
[package]
name = "sqlx-leak-repro"
version = "0.0.0"
edition = "2021"
publish = false

[workspace]

[dependencies]
sqlx = { version = "0.9.0", features = ["runtime-tokio", "tls-native-tls", "postgres"] }
tokio = { version = "1", features = ["full"] }

Prints (in my laptop) this:

backend pid=9220
iterations=     1000  live_prepared_statements=1000  backend_mem=7423 kB
iterations=     2000  live_prepared_statements=2000  backend_mem=13 MB
iterations=     3000  live_prepared_statements=3000  backend_mem=19 MB
iterations=     4000  live_prepared_statements=4000  backend_mem=25 MB
iterations=     5000  live_prepared_statements=5000  backend_mem=32 MB
iterations=     6000  live_prepared_statements=6000  backend_mem=37 MB
iterations=     7000  live_prepared_statements=7000  backend_mem=43 MB
iterations=     8000  live_prepared_statements=8000  backend_mem=49 MB
iterations=     9000  live_prepared_statements=9000  backend_mem=55 MB
iterations=    10000  live_prepared_statements=10000  backend_mem=62 MB
iterations=    11000  live_prepared_statements=11000  backend_mem=68 MB
...

SQLx version

0.9.0

Enabled SQLx features

postgres

Database server and version

Postgres

Operating system

macOS, Linux

Rust version

1.96.0

[abonander]

Yeah, if persistent should probably be if persistent && conn.inner.cache_statement.is_enabled() here:

sqlx/sqlx-postgres/src/connection/executor.rs

Line 31 in 4893f83

let id = if persistent {