From c119ba54302aeb87664d2f7aa221f7308e853cf8 Mon Sep 17 00:00:00 2001 From: Manuel Vidaurre Date: Mon, 31 Aug 2026 18:31:01 -0600 Subject: [PATCH 1/8] feat: per-user/per-database override for client_idle_timeout client_idle_timeout was previously global-only, disconnecting long-idle LISTEN/NOTIFY subscribers with no way to exempt them. Add an Option override on User and Database (users.toml / pgdog.toml), resolved with the same user -> database -> general precedence used elsewhere (idle_timeout, statement_timeout). 0 at any level exempts the client from the timeout entirely. Fixes #1462 Co-Authored-By: Claude Sonnet 5 Claude-Session: https://claude.ai/code/session_01NoNS7QBkAU2apgWGLFuMMp --- pgdog-config/src/core.rs | 110 ++++++++++++++++++ pgdog-config/src/database.rs | 6 + pgdog-config/src/users.rs | 6 + .../src/backend/pool/connection/mirror/mod.rs | 5 +- pgdog/src/config/general.rs | 1 - pgdog/src/config/mod.rs | 2 - pgdog/src/frontend/client/mod.rs | 8 +- pgdog/src/frontend/client/test/mod.rs | 25 ++++ pgdog/src/frontend/client/timeouts.rs | 43 ++++++- 9 files changed, 192 insertions(+), 14 deletions(-) delete mode 100644 pgdog/src/config/general.rs diff --git a/pgdog-config/src/core.rs b/pgdog-config/src/core.rs index e0124ad4f..d4db186b2 100644 --- a/pgdog-config/src/core.rs +++ b/pgdog-config/src/core.rs @@ -4,6 +4,7 @@ use serde::{Deserialize, Serialize}; use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet}; use std::fs::read_to_string; use std::path::{Path, PathBuf}; +use std::time::Duration; use tracing::{error, info, warn}; use crate::sharding::ShardedSchema; @@ -171,6 +172,34 @@ impl ConfigAndUsers { pub fn pub_sub_enabled(&self) -> bool { self.config.general.pub_sub_channel_size > 0 } + + /// Resolve `client_idle_timeout` for a connecting user/database pair. + /// + /// Precedence is user, then database, then general, matching + /// [`crate::pool::PoolConfig::resolve`]. `0` at any level means the + /// client is exempt from the timeout. + pub fn client_idle_timeout(&self, user: &str, database: &str) -> Duration { + let millis = self + .users + .users + .iter() + .find(|u| u.name == user && u.has_database(database)) + .and_then(|u| u.client_idle_timeout) + .or_else(|| { + self.config + .databases + .iter() + .find(|d| d.name == database) + .and_then(|d| d.client_idle_timeout) + }) + .unwrap_or(self.config.general.client_idle_timeout); + + if millis == 0 { + Duration::MAX + } else { + Duration::from_millis(millis) + } + } } impl Default for ConfigAndUsers { @@ -784,6 +813,87 @@ mod tests { assert!(!single.has_database("missing")); } + #[test] + fn client_idle_timeout_falls_back_to_general() { + let config = ConfigAndUsers { + config: Config { + general: General { + client_idle_timeout: 60_000, + ..Default::default() + }, + ..Default::default() + }, + ..Default::default() + }; + + assert_eq!( + config.client_idle_timeout("alice", "production"), + Duration::from_millis(60_000) + ); + } + + #[test] + fn client_idle_timeout_database_overrides_general() { + let config = ConfigAndUsers { + config: Config { + general: General { + client_idle_timeout: 60_000, + ..Default::default() + }, + databases: vec![Database { + name: "production".into(), + client_idle_timeout: Some(0), + ..Default::default() + }], + ..Default::default() + }, + ..Default::default() + }; + + assert_eq!( + config.client_idle_timeout("alice", "production"), + Duration::MAX + ); + } + + #[test] + fn client_idle_timeout_user_overrides_database() { + let config = ConfigAndUsers { + config: Config { + general: General { + client_idle_timeout: 60_000, + ..Default::default() + }, + databases: vec![Database { + name: "production".into(), + client_idle_timeout: Some(30_000), + ..Default::default() + }], + ..Default::default() + }, + users: Users { + users: vec![User { + name: "alice".into(), + database: "production".into(), + client_idle_timeout: Some(0), + ..Default::default() + }], + ..Default::default() + }, + ..Default::default() + }; + + assert_eq!( + config.client_idle_timeout("alice", "production"), + Duration::MAX + ); + // A different user on the same database isn't exempt. + assert_eq!( + config.client_idle_timeout("bob", "production"), + Duration::from_millis(30_000) + ); + } + #[test] fn test_basic() { let pgdog_source = r#" diff --git a/pgdog-config/src/database.rs b/pgdog-config/src/database.rs index ac316de31..a1b7d3999 100644 --- a/pgdog-config/src/database.rs +++ b/pgdog-config/src/database.rs @@ -191,6 +191,12 @@ pub struct Database { /// /// pub idle_timeout: Option, + /// Overrides the `client_idle_timeout` setting. Client connections to this database that haven't sent any queries for this long will be disconnected. + /// + /// **Note:** Set to `0` to exempt clients of this database from the client idle timeout entirely, e.g. for `LISTEN`/`NOTIFY` subscribers that are expected to stay quiet for long periods. + /// + /// + pub client_idle_timeout: Option, /// Sets the `default_transaction_read_only` connection parameter to `on` on all server connections to this database. Clients can still override it with `SET`. /// /// diff --git a/pgdog-config/src/users.rs b/pgdog-config/src/users.rs index d79bb224b..2b5098ce8 100644 --- a/pgdog-config/src/users.rs +++ b/pgdog-config/src/users.rs @@ -371,6 +371,12 @@ pub struct User { /// /// pub idle_timeout: Option, + /// Overrides [`client_idle_timeout`](https://docs.pgdog.dev/configuration/pgdog.toml/general/#client_idle_timeout) for this user. Client connections that haven't sent any queries for this long will be disconnected. + /// + /// **Note:** Set to `0` to exempt this user from the client idle timeout entirely, e.g. for `LISTEN`/`NOTIFY` subscribers that are expected to stay quiet for long periods. + /// + /// + pub client_idle_timeout: Option, /// Sets `default_transaction_read_only` to `on` for all connections. pub read_only: Option, /// Schema owner with elevated DDL privileges. diff --git a/pgdog/src/backend/pool/connection/mirror/mod.rs b/pgdog/src/backend/pool/connection/mirror/mod.rs index 6a884eefb..4b52d3f2a 100644 --- a/pgdog/src/backend/pool/connection/mirror/mod.rs +++ b/pgdog/src/backend/pool/connection/mirror/mod.rs @@ -19,7 +19,7 @@ use crate::net::{FrontendPid, Parameter, Parameters, Stream}; use crate::tasks; use super::Error; -use crate::util::safe_sleep; +use crate::util::{safe_sleep, user_database_from_params}; pub(crate) mod buffer_with_delay; pub(crate) mod handler; @@ -52,12 +52,13 @@ impl Mirror { fn new(params: &Parameters, config: &ConfigAndUsers) -> Self { let mut prepared_statements = PreparedStatements::new(); prepared_statements.set_level(config.prepared_statements()); + let (user, database) = user_database_from_params(params); Self { id: FrontendPid::new(), prepared_statements, params: params.clone(), - timeouts: Timeouts::from_config(&config.config.general), + timeouts: Timeouts::from_config(config, user, database), stream: Stream::dev_null(), transaction: None, } diff --git a/pgdog/src/config/general.rs b/pgdog/src/config/general.rs deleted file mode 100644 index 6f70ab821..000000000 --- a/pgdog/src/config/general.rs +++ /dev/null @@ -1 +0,0 @@ -pub(crate) use pgdog_config::general::General; diff --git a/pgdog/src/config/mod.rs b/pgdog/src/config/mod.rs index 0c07f5529..14f8f2b8f 100644 --- a/pgdog/src/config/mod.rs +++ b/pgdog/src/config/mod.rs @@ -5,7 +5,6 @@ pub(crate) mod convert; pub(crate) mod core; pub(crate) mod database; pub(crate) mod error; -pub(crate) mod general; pub(crate) mod memory; pub(crate) mod networking; pub(crate) mod overrides; @@ -18,7 +17,6 @@ pub(crate) mod users; pub(crate) use core::{Config, ConfigAndUsers}; pub(crate) use database::{Database, Role}; pub(crate) use error::Error; -pub(crate) use general::General; pub(crate) use memory::*; pub(crate) use networking::{MultiTenant, TlsVerifyMode}; pub(crate) use overrides::Overrides; diff --git a/pgdog/src/frontend/client/mod.rs b/pgdog/src/frontend/client/mod.rs index 4f59aba9f..1acd0a830 100644 --- a/pgdog/src/frontend/client/mod.rs +++ b/pgdog/src/frontend/client/mod.rs @@ -420,7 +420,7 @@ impl Client { params: params.clone(), prepared_statements: PreparedStatements::new(), transaction: None, - timeouts: Timeouts::from_config(&config.config.general), + timeouts: Timeouts::from_config(&config, user, database), client_request: ClientRequest::default(), stream_buffer: MessageBuffer::new( config.config.memory.message_buffer, @@ -448,6 +448,7 @@ impl Client { let key = BackendKeyData::new_frontend(ProtocolVersion::V3_0, id); let mut prepared_statements = PreparedStatements::new(); prepared_statements.level = config().config.general.prepared_statements; + let (user, database) = user_database_from_params(¶ms); Self { stream, @@ -458,7 +459,7 @@ impl Client { prepared_statements, admin: false, transaction: None, - timeouts: Timeouts::from_config(&config().config.general), + timeouts: Timeouts::from_config(&config(), user, database), client_request: ClientRequest::default(), stream_buffer: MessageBuffer::new( 4096, @@ -642,7 +643,8 @@ impl Client { let config = config::config(); // Configure prepared statements cache. self.prepared_statements.level = config.prepared_statements(); - self.timeouts = Timeouts::from_config(&config.config.general); + let (user, database) = user_database_from_params(&self.params); + self.timeouts = Timeouts::from_config(&config, user, database); self.query_log_stdout = config.config.general.query_log_stdout; self.query_size_limit = config.config.general.query_size_limit; self.stream_buffer diff --git a/pgdog/src/frontend/client/test/mod.rs b/pgdog/src/frontend/client/test/mod.rs index 5b9906827..c5cd58945 100644 --- a/pgdog/src/frontend/client/test/mod.rs +++ b/pgdog/src/frontend/client/test/mod.rs @@ -290,6 +290,31 @@ async fn test_client_idle_timeout() { ); } +#[tokio::test] +async fn test_client_idle_timeout_user_override() { + let (conn, mut client, _inner) = new_client!(false); + + let mut config = (*config()).clone(); + // General timeout is short, but this user is exempt. + config.config.general.client_idle_timeout = 25; + config.users.add_or_replace(pgdog_config::User { + name: "pgdog".into(), + database: "pgdog".into(), + client_idle_timeout: Some(0), + ..Default::default() + }); + set(config).unwrap(); + + assert!( + timeout(Duration::from_millis(50), client.buffer(State::Idle)) + .await + .is_err(), + "user override should exempt this client from the idle timeout" + ); + + drop(conn); +} + #[tokio::test] async fn test_parse_describe_flush_bind_execute_close_sync() { let (mut conn, mut client, _) = new_client!(false); diff --git a/pgdog/src/frontend/client/timeouts.rs b/pgdog/src/frontend/client/timeouts.rs index 8a3d6fcf0..66fd0386e 100644 --- a/pgdog/src/frontend/client/timeouts.rs +++ b/pgdog/src/frontend/client/timeouts.rs @@ -1,6 +1,6 @@ use std::time::Duration; -use crate::{config::General, frontend::ClientRequest, state::State}; +use crate::{config::ConfigAndUsers, frontend::ClientRequest, state::State}; #[derive(Debug, Clone, Copy)] pub(crate) struct Timeouts { @@ -20,11 +20,11 @@ impl Default for Timeouts { } impl Timeouts { - pub(crate) fn from_config(general: &General) -> Self { + pub(crate) fn from_config(config: &ConfigAndUsers, user: &str, database: &str) -> Self { Self { - query_timeout: general.query_timeout(), - client_idle_timeout: general.client_idle_timeout(), - idle_in_transaction_timeout: general.client_idle_in_transaction_timeout(), + query_timeout: config.config.general.query_timeout(), + client_idle_timeout: config.client_idle_timeout(user, database), + idle_in_transaction_timeout: config.config.general.client_idle_in_transaction_timeout(), } } @@ -74,7 +74,7 @@ mod test { #[test] fn test_idle_in_transaction_timeout() { let config = config(); // Will be default. - let timeout = Timeouts::from_config(&config.config.general); + let timeout = Timeouts::from_config(&config, "postgres", "postgres"); let actual = timeout.client_idle_timeout(&State::IdleInTransaction, &ClientRequest::default()); @@ -87,4 +87,35 @@ mod test { ); assert_eq!(actual, Duration::MAX); } + + #[test] + fn from_config_uses_per_user_client_idle_timeout_override() { + use pgdog_config::{Config, ConfigAndUsers, General, User, Users}; + + let config = ConfigAndUsers { + config: Config { + general: General { + client_idle_timeout: 60_000, + ..Default::default() + }, + ..Default::default() + }, + users: Users { + users: vec![User { + name: "listener".into(), + database: "pgdog".into(), + client_idle_timeout: Some(0), + ..Default::default() + }], + ..Default::default() + }, + ..Default::default() + }; + + let timeouts = Timeouts::from_config(&config, "listener", "pgdog"); + assert_eq!(timeouts.client_idle_timeout, Duration::MAX); + + let timeouts = Timeouts::from_config(&config, "other", "pgdog"); + assert_eq!(timeouts.client_idle_timeout, Duration::from_millis(60_000)); + } } From 649c39ca9c17078d41dc6285df9f43d30d153f26 Mon Sep 17 00:00:00 2001 From: Manuel Vidaurre Date: Mon, 31 Aug 2026 20:19:07 -0600 Subject: [PATCH 2/8] fix: harden client idle timeout overrides --- .schema/pgdog.schema.json | 9 +++++ .schema/users.schema.json | 9 +++++ pgdog-config/src/core.rs | 66 +++++++++++++++++++++++++------- pgdog/src/frontend/client/mod.rs | 27 ++++++++++--- 4 files changed, 92 insertions(+), 19 deletions(-) diff --git a/.schema/pgdog.schema.json b/.schema/pgdog.schema.json index 06452dd87..eea39233a 100644 --- a/.schema/pgdog.schema.json +++ b/.schema/pgdog.schema.json @@ -404,6 +404,15 @@ "description": "Database settings configure which databases PgDog is managing. This is a TOML list of hosts, ports, and other settings like database roles (primary or replica).\n\n", "type": "object", "properties": { + "client_idle_timeout": { + "description": "Overrides the `client_idle_timeout` setting. Client connections to this database that haven't sent any queries for this long will be disconnected.\n\n**Note:** Set to `0` to exempt clients of this database from the client idle timeout entirely, e.g. for `LISTEN`/`NOTIFY` subscribers that are expected to stay quiet for long periods.\n\n", + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 0 + }, "database_name": { "description": "Name of the PostgreSQL database on the server PgDog will connect to. If not set, this defaults to `name`.\n\n", "type": [ diff --git a/.schema/users.schema.json b/.schema/users.schema.json index 3afbf8796..18b02dc8a 100644 --- a/.schema/users.schema.json +++ b/.schema/users.schema.json @@ -107,6 +107,15 @@ "type": "boolean", "default": false }, + "client_idle_timeout": { + "description": "Overrides [`client_idle_timeout`](https://docs.pgdog.dev/configuration/pgdog.toml/general/#client_idle_timeout) for this user. Client connections that haven't sent any queries for this long will be disconnected.\n\n**Note:** Set to `0` to exempt this user from the client idle timeout entirely, e.g. for `LISTEN`/`NOTIFY` subscribers that are expected to stay quiet for long periods.\n\n", + "type": [ + "integer", + "null" + ], + "format": "uint64", + "minimum": 0 + }, "cross_shard_disabled": { "description": "Disable cross-shard queries for this user.", "type": [ diff --git a/pgdog-config/src/core.rs b/pgdog-config/src/core.rs index d4db186b2..71c7cd4c8 100644 --- a/pgdog-config/src/core.rs +++ b/pgdog-config/src/core.rs @@ -183,14 +183,20 @@ impl ConfigAndUsers { .users .users .iter() + .rev() + // Pool construction inserts users in configuration order, so the + // last matching entry is the effective one for this pair. .find(|u| u.name == user && u.has_database(database)) .and_then(|u| u.client_idle_timeout) .or_else(|| { + // A logical database can have several entries (shards and + // replicas). Use the first configured override rather than + // requiring it to be on the first physical server entry. self.config .databases .iter() - .find(|d| d.name == database) - .and_then(|d| d.client_idle_timeout) + .filter(|d| d.name == database) + .find_map(|d| d.client_idle_timeout) }) .unwrap_or(self.config.general.client_idle_timeout); @@ -496,6 +502,7 @@ impl Config { struct Check { pooler_mode: Option, + client_idle_timeout: Option, role: Role, role_warned: bool, parser_warned: bool, @@ -516,6 +523,19 @@ impl Config { database.name, database.shard, database.role, ); } + if let Some(client_idle_timeout) = database.client_idle_timeout { + if existing + .client_idle_timeout + .is_some_and(|existing| existing != client_idle_timeout) + { + warn!( + "database \"{}\" (shard={}, role={}) has a conflicting \"client_idle_timeout\" setting, using the first configured value", + database.name, database.shard, database.role, + ); + } else if existing.client_idle_timeout.is_none() { + existing.client_idle_timeout = Some(client_idle_timeout); + } + } let auto = existing.role == Role::Auto || database.role == Role::Auto; if auto && existing.role != database.role && !existing.role_warned { warn!( @@ -552,6 +572,7 @@ impl Config { database.name.clone(), Check { pooler_mode: database.pooler_mode, + client_idle_timeout: database.client_idle_timeout, role: database.role, role_warned: false, parser_warned: false, @@ -840,11 +861,17 @@ mod tests { client_idle_timeout: 60_000, ..Default::default() }, - databases: vec![Database { - name: "production".into(), - client_idle_timeout: Some(0), - ..Default::default() - }], + databases: vec![ + Database { + name: "production".into(), + ..Default::default() + }, + Database { + name: "production".into(), + client_idle_timeout: Some(0), + ..Default::default() + }, + ], ..Default::default() }, ..Default::default() @@ -872,12 +899,20 @@ mod tests { ..Default::default() }, users: Users { - users: vec![User { - name: "alice".into(), - database: "production".into(), - client_idle_timeout: Some(0), - ..Default::default() - }], + users: vec![ + User { + name: "alice".into(), + all_databases: true, + client_idle_timeout: Some(45_000), + ..Default::default() + }, + User { + name: "alice".into(), + database: "production".into(), + client_idle_timeout: Some(0), + ..Default::default() + }, + ], ..Default::default() }, ..Default::default() @@ -887,6 +922,11 @@ mod tests { config.client_idle_timeout("alice", "production"), Duration::MAX ); + // The earlier wildcard entry still applies where it is the effective user. + assert_eq!( + config.client_idle_timeout("alice", "other"), + Duration::from_millis(45_000) + ); // A different user on the same database isn't exempt. assert_eq!( config.client_idle_timeout("bob", "production"), diff --git a/pgdog/src/frontend/client/mod.rs b/pgdog/src/frontend/client/mod.rs index 1acd0a830..63c4f63aa 100644 --- a/pgdog/src/frontend/client/mod.rs +++ b/pgdog/src/frontend/client/mod.rs @@ -4,7 +4,7 @@ //! use std::net::SocketAddr; -use std::sync::Arc; +use std::sync::{Arc, Weak}; use std::time::{Duration, Instant}; use pgdog_config::users::PasswordKind; @@ -77,6 +77,10 @@ pub(crate) struct Client { // These change based on client state, e.g. if client is running query, // the `query_timeout` is active, and if the client is idle, the `client_idle_timeout` is. timeouts: Timeouts, + // Configuration snapshot used to resolve `timeouts`. Keeping a weak handle + // lets us avoid scanning all users and databases on every request while + // still refreshing timeouts after a configuration reload. + timeouts_config: Weak, // Stateful buffer containing the current whole client request. // This can be a query or just a `Parse` and `Flush`, but in either case, the client // will expect a response immediately and we need to handle it. @@ -421,6 +425,7 @@ impl Client { prepared_statements: PreparedStatements::new(), transaction: None, timeouts: Timeouts::from_config(&config, user, database), + timeouts_config: Arc::downgrade(&config), client_request: ClientRequest::default(), stream_buffer: MessageBuffer::new( config.config.memory.message_buffer, @@ -444,10 +449,11 @@ impl Client { params.insert("database", "pgdog"); } + let config = config(); let id = FrontendPid::new(); let key = BackendKeyData::new_frontend(ProtocolVersion::V3_0, id); let mut prepared_statements = PreparedStatements::new(); - prepared_statements.level = config().config.general.prepared_statements; + prepared_statements.level = config.config.general.prepared_statements; let (user, database) = user_database_from_params(¶ms); Self { @@ -459,11 +465,12 @@ impl Client { prepared_statements, admin: false, transaction: None, - timeouts: Timeouts::from_config(&config(), user, database), + timeouts: Timeouts::from_config(&config, user, database), + timeouts_config: Arc::downgrade(&config), client_request: ClientRequest::default(), stream_buffer: MessageBuffer::new( 4096, - config().config.general.frontend_query_size_limit_block(), + config.config.general.frontend_query_size_limit_block(), ), sticky: Sticky::from_params(¶ms), params, @@ -643,8 +650,15 @@ impl Client { let config = config::config(); // Configure prepared statements cache. self.prepared_statements.level = config.prepared_statements(); - let (user, database) = user_database_from_params(&self.params); - self.timeouts = Timeouts::from_config(&config, user, database); + let timeouts_current = self + .timeouts_config + .upgrade() + .is_some_and(|previous| Arc::ptr_eq(&previous, &config)); + if !timeouts_current { + let (user, database) = user_database_from_params(&self.params); + self.timeouts = Timeouts::from_config(&config, user, database); + self.timeouts_config = Arc::downgrade(&config); + } self.query_log_stdout = config.config.general.query_log_stdout; self.query_size_limit = config.config.general.query_size_limit; self.stream_buffer @@ -748,6 +762,7 @@ impl MemoryUsage for Client { + std::mem::size_of::() * 5 + self.prepared_statements.memory_used() + std::mem::size_of::() + + std::mem::size_of::>() + self.stream_buffer.capacity() + self.client_request.memory_usage() } From 91c3357d3a5d6eca6014f007d1125676cfd88517 Mon Sep 17 00:00:00 2001 From: Manuel Vidaurre Date: Mon, 31 Aug 2026 20:23:23 -0600 Subject: [PATCH 3/8] test: avoid backend pool in idle timeout tests --- pgdog/src/frontend/client/test/mod.rs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/pgdog/src/frontend/client/test/mod.rs b/pgdog/src/frontend/client/test/mod.rs index c5cd58945..c212a88a9 100644 --- a/pgdog/src/frontend/client/test/mod.rs +++ b/pgdog/src/frontend/client/test/mod.rs @@ -265,7 +265,8 @@ async fn test_abrupt_disconnect() { #[tokio::test] async fn test_client_idle_timeout() { - let (mut conn, mut client, _inner) = new_client!(false); + crate::logger(); + let (mut conn, mut client) = parallel_test_client().await; let mut config = (*config()).clone(); config.config.general.client_idle_timeout = 25; @@ -292,7 +293,8 @@ async fn test_client_idle_timeout() { #[tokio::test] async fn test_client_idle_timeout_user_override() { - let (conn, mut client, _inner) = new_client!(false); + crate::logger(); + let (conn, mut client) = parallel_test_client().await; let mut config = (*config()).clone(); // General timeout is short, but this user is exempt. @@ -300,6 +302,7 @@ async fn test_client_idle_timeout_user_override() { config.users.add_or_replace(pgdog_config::User { name: "pgdog".into(), database: "pgdog".into(), + password: Some("pgdog".into()), client_idle_timeout: Some(0), ..Default::default() }); From f25ed2b7cca26a57b2b929eff2d615686e8b2069 Mon Sep 17 00:00:00 2001 From: Manuel Vidaurre Date: Mon, 31 Aug 2026 22:05:33 -0600 Subject: [PATCH 4/8] fix: remove dead General::client_idle_timeout() with stale semantics The per-user/per-database override work moved timeout resolution to ConfigAndUsers::client_idle_timeout(user, database), where 0 means "disabled". This method was no longer called anywhere but still treated 0 as "time out immediately", the opposite of the new rule. Co-Authored-By: Claude Sonnet 5 --- pgdog-config/src/general.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/pgdog-config/src/general.rs b/pgdog-config/src/general.rs index 8e2e7c426..97cd70a9c 100644 --- a/pgdog-config/src/general.rs +++ b/pgdog-config/src/general.rs @@ -1232,10 +1232,6 @@ impl General { Duration::from_millis(self.prepared_statements_ttl_jitter.min(ttl)) } - pub fn client_idle_timeout(&self) -> Duration { - Duration::from_millis(self.client_idle_timeout) - } - pub fn connect_attempt_delay(&self) -> Duration { Duration::from_millis(self.connect_attempt_delay) } From e048d8b7f198492ec6cf2013195480612a0bc80d Mon Sep 17 00:00:00 2001 From: Manuel Vidaurre Date: Mon, 31 Aug 2026 22:32:38 -0600 Subject: [PATCH 5/8] docs: document 0=disabled semantics for general.client_idle_timeout The per-user/per-database override work made 0 mean "disabled" at every level, including general.client_idle_timeout (previously 0 meant "time out immediately" via the now-removed General::client_idle_timeout() method). Document this explicitly, matching the notes already added to the database/user overrides. Co-Authored-By: Claude Sonnet 5 --- .schema/pgdog.schema.json | 2 +- pgdog-config/src/general.rs | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/.schema/pgdog.schema.json b/.schema/pgdog.schema.json index eea39233a..8f6cbc938 100644 --- a/.schema/pgdog.schema.json +++ b/.schema/pgdog.schema.json @@ -730,7 +730,7 @@ "minimum": 0 }, "client_idle_timeout": { - "description": "Close client connections that have been idle, i.e., haven't sent any queries, for this amount of time.\n\n", + "description": "Close client connections that have been idle, i.e., haven't sent any queries, for this amount of time.\n\n**Note:** Set to `0` to disable the client idle timeout entirely. Can be overridden per-user or per-database.\n\n", "type": "integer", "format": "uint64", "default": 9223372036854775807, diff --git a/pgdog-config/src/general.rs b/pgdog-config/src/general.rs index 97cd70a9c..6b0dd03d2 100644 --- a/pgdog-config/src/general.rs +++ b/pgdog-config/src/general.rs @@ -517,6 +517,8 @@ pub struct General { /// Close client connections that have been idle, i.e., haven't sent any queries, for this amount of time. /// + /// **Note:** Set to `0` to disable the client idle timeout entirely. Can be overridden per-user or per-database. + /// /// #[serde(default = "General::default_client_idle_timeout")] pub client_idle_timeout: u64, From 63d73580984bd0448b2f94fe7a0ead733bf1c420 Mon Sep 17 00:00:00 2001 From: Manuel Vidaurre Date: Tue, 1 Sep 2026 06:11:29 -0600 Subject: [PATCH 6/8] fix: harden client_idle_timeout resolution from deep review - User-level resolution now takes the last matching entry that actually configures the setting, so bare entries appended by passthrough auth (or broader later entries without the setting) no longer erase an earlier override. Regression test included. - The virtual admin database is exempt from user/database overrides, matching pool construction which never associates users with it. Regression test included. - Extracted Config::database_client_idle_timeout() so first-wins resolution for duplicate [[databases]] entries lives in one place, and rewrote the Config::check merge as a match without shadowed bindings. - Replaced Weak::upgrade + Arc::ptr_eq on the per-request hot path with a plain pointer compare (our own Weak pins the allocation, so no ABA). Co-Authored-By: Claude Sonnet 5 --- pgdog-config/src/core.rs | 151 +++++++++++++++++++++----- pgdog/src/frontend/client/mod.rs | 8 +- pgdog/src/frontend/client/test/mod.rs | 5 +- 3 files changed, 127 insertions(+), 37 deletions(-) diff --git a/pgdog-config/src/core.rs b/pgdog-config/src/core.rs index 71c7cd4c8..b252819ea 100644 --- a/pgdog-config/src/core.rs +++ b/pgdog-config/src/core.rs @@ -179,26 +179,25 @@ impl ConfigAndUsers { /// [`crate::pool::PoolConfig::resolve`]. `0` at any level means the /// client is exempt from the timeout. pub fn client_idle_timeout(&self, user: &str, database: &str) -> Duration { - let millis = self - .users - .users - .iter() - .rev() - // Pool construction inserts users in configuration order, so the - // last matching entry is the effective one for this pair. - .find(|u| u.name == user && u.has_database(database)) - .and_then(|u| u.client_idle_timeout) - .or_else(|| { - // A logical database can have several entries (shards and - // replicas). Use the first configured override rather than - // requiring it to be on the first physical server entry. - self.config - .databases - .iter() - .filter(|d| d.name == database) - .find_map(|d| d.client_idle_timeout) - }) - .unwrap_or(self.config.general.client_idle_timeout); + // The admin database is virtual and never part of pool construction; + // user and database overrides don't apply to it. + let admin = &self.config.admin; + let millis = if user == admin.user && database == admin.name { + self.config.general.client_idle_timeout + } else { + self.users + .users + .iter() + .filter(|u| u.name == user && u.has_database(database)) + // Overrides resolve per-setting: the last matching entry that + // configures the setting wins. Entries without the setting + // (including bare entries appended by passthrough auth) fall + // back to earlier matching entries rather than erasing them. + .filter_map(|u| u.client_idle_timeout) + .last() + .or_else(|| self.config.database_client_idle_timeout(database)) + .unwrap_or(self.config.general.client_idle_timeout) + }; if millis == 0 { Duration::MAX @@ -368,6 +367,18 @@ impl Config { } } + /// Effective `client_idle_timeout` override for a logical database. + /// + /// A logical database can have several entries (shards and replicas); the + /// first configured override wins, so it doesn't have to be on the first + /// physical server entry. [`Config::check`] warns about conflicting values. + pub fn database_client_idle_timeout(&self, name: &str) -> Option { + self.databases + .iter() + .filter(|d| d.name == name) + .find_map(|d| d.client_idle_timeout) + } + pub fn omnisharded_tables(&self) -> HashMap> { let mut tables = HashMap::new(); @@ -524,16 +535,15 @@ impl Config { ); } if let Some(client_idle_timeout) = database.client_idle_timeout { - if existing - .client_idle_timeout - .is_some_and(|existing| existing != client_idle_timeout) - { - warn!( - "database \"{}\" (shard={}, role={}) has a conflicting \"client_idle_timeout\" setting, using the first configured value", - database.name, database.shard, database.role, - ); - } else if existing.client_idle_timeout.is_none() { - existing.client_idle_timeout = Some(client_idle_timeout); + match existing.client_idle_timeout { + Some(first) if first != client_idle_timeout => { + warn!( + "database \"{}\" (shard={}, role={}) has a conflicting \"client_idle_timeout\" setting, using the first configured value", + database.name, database.shard, database.role, + ); + } + Some(_) => {} + None => existing.client_idle_timeout = Some(client_idle_timeout), } } let auto = existing.role == Role::Auto || database.role == Role::Auto; @@ -934,6 +944,87 @@ mod tests { ); } + #[test] + fn client_idle_timeout_survives_later_entry_without_override() { + // A later matching entry that doesn't configure the setting must not + // erase an earlier matching override. Passthrough auth appends bare + // entries (name + database + password only) at the end of the list, + // which would otherwise shadow a wildcard exemption. + let config = ConfigAndUsers { + config: Config { + general: General { + client_idle_timeout: 60_000, + ..Default::default() + }, + ..Default::default() + }, + users: Users { + users: vec![ + User { + name: "listener".into(), + all_databases: true, + client_idle_timeout: Some(0), + ..Default::default() + }, + // Bare entry appended by passthrough auth on first login. + User { + name: "listener".into(), + database: "production".into(), + ..Default::default() + }, + ], + ..Default::default() + }, + ..Default::default() + }; + + assert_eq!( + config.client_idle_timeout("listener", "production"), + Duration::MAX + ); + } + + #[test] + fn client_idle_timeout_ignores_overrides_for_admin_database() { + // The admin database is virtual and never part of pool construction; + // a wildcard user override must not leak onto admin console sessions. + let config = ConfigAndUsers { + config: Config { + general: General { + client_idle_timeout: 60_000, + ..Default::default() + }, + ..Default::default() + }, + users: Users { + users: vec![User { + name: config_admin_user(), + all_databases: true, + client_idle_timeout: Some(0), + ..Default::default() + }], + ..Default::default() + }, + ..Default::default() + }; + + let admin_user = config.config.admin.user.clone(); + let admin_database = config.config.admin.name.clone(); + assert_eq!( + config.client_idle_timeout(&admin_user, &admin_database), + Duration::from_millis(60_000) + ); + // The same user connecting to a regular database keeps the override. + assert_eq!( + config.client_idle_timeout(&admin_user, "production"), + Duration::MAX + ); + } + + fn config_admin_user() -> String { + ConfigAndUsers::default().config.admin.user + } + #[test] fn test_basic() { let pgdog_source = r#" diff --git a/pgdog/src/frontend/client/mod.rs b/pgdog/src/frontend/client/mod.rs index 63c4f63aa..40ea8a291 100644 --- a/pgdog/src/frontend/client/mod.rs +++ b/pgdog/src/frontend/client/mod.rs @@ -650,10 +650,10 @@ impl Client { let config = config::config(); // Configure prepared statements cache. self.prepared_statements.level = config.prepared_statements(); - let timeouts_current = self - .timeouts_config - .upgrade() - .is_some_and(|previous| Arc::ptr_eq(&previous, &config)); + // Our own weak handle keeps the old allocation from being freed and + // reused, so pointer equality can't suffer ABA and no refcount + // traffic (`Weak::upgrade`) is needed on this hot path. + let timeouts_current = std::ptr::eq(self.timeouts_config.as_ptr(), Arc::as_ptr(&config)); if !timeouts_current { let (user, database) = user_database_from_params(&self.params); self.timeouts = Timeouts::from_config(&config, user, database); diff --git a/pgdog/src/frontend/client/test/mod.rs b/pgdog/src/frontend/client/test/mod.rs index c212a88a9..4e979764d 100644 --- a/pgdog/src/frontend/client/test/mod.rs +++ b/pgdog/src/frontend/client/test/mod.rs @@ -294,7 +294,8 @@ async fn test_client_idle_timeout() { #[tokio::test] async fn test_client_idle_timeout_user_override() { crate::logger(); - let (conn, mut client) = parallel_test_client().await; + // Keep `_conn` alive so the client's stream stays open while idle. + let (_conn, mut client) = parallel_test_client().await; let mut config = (*config()).clone(); // General timeout is short, but this user is exempt. @@ -314,8 +315,6 @@ async fn test_client_idle_timeout_user_override() { .is_err(), "user override should exempt this client from the idle timeout" ); - - drop(conn); } #[tokio::test] From 65802703567c7b53d64995ab36304120b6493351 Mon Sep 17 00:00:00 2001 From: Manuel Vidaurre Date: Tue, 1 Sep 2026 06:58:19 -0600 Subject: [PATCH 7/8] style: idiomatic iteration and test cleanup in idle timeout resolver MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replace filter_map().last() with rev().find_map() — short-circuits from the end instead of walking every user entry, and avoids clippy's double_ended_iterator_last. Drop the redundant config_admin_user() test helper in favor of Admin::default(). Co-Authored-By: Claude Sonnet 5 --- pgdog-config/src/core.rs | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) diff --git a/pgdog-config/src/core.rs b/pgdog-config/src/core.rs index b252819ea..77151bdde 100644 --- a/pgdog-config/src/core.rs +++ b/pgdog-config/src/core.rs @@ -188,13 +188,13 @@ impl ConfigAndUsers { self.users .users .iter() + .rev() .filter(|u| u.name == user && u.has_database(database)) // Overrides resolve per-setting: the last matching entry that // configures the setting wins. Entries without the setting // (including bare entries appended by passthrough auth) fall // back to earlier matching entries rather than erasing them. - .filter_map(|u| u.client_idle_timeout) - .last() + .find_map(|u| u.client_idle_timeout) .or_else(|| self.config.database_client_idle_timeout(database)) .unwrap_or(self.config.general.client_idle_timeout) }; @@ -988,6 +988,7 @@ mod tests { fn client_idle_timeout_ignores_overrides_for_admin_database() { // The admin database is virtual and never part of pool construction; // a wildcard user override must not leak onto admin console sessions. + let admin = Admin::default(); let config = ConfigAndUsers { config: Config { general: General { @@ -998,7 +999,7 @@ mod tests { }, users: Users { users: vec![User { - name: config_admin_user(), + name: admin.user.clone(), all_databases: true, client_idle_timeout: Some(0), ..Default::default() @@ -1008,23 +1009,17 @@ mod tests { ..Default::default() }; - let admin_user = config.config.admin.user.clone(); - let admin_database = config.config.admin.name.clone(); assert_eq!( - config.client_idle_timeout(&admin_user, &admin_database), + config.client_idle_timeout(&admin.user, &admin.name), Duration::from_millis(60_000) ); // The same user connecting to a regular database keeps the override. assert_eq!( - config.client_idle_timeout(&admin_user, "production"), + config.client_idle_timeout(&admin.user, "production"), Duration::MAX ); } - fn config_admin_user() -> String { - ConfigAndUsers::default().config.admin.user - } - #[test] fn test_basic() { let pgdog_source = r#" From 304b997072fa5d5ae1e97a1ec5b112dad83f2e66 Mon Sep 17 00:00:00 2001 From: Manuel Vidaurre Date: Tue, 1 Sep 2026 08:49:08 -0600 Subject: [PATCH 8/8] fix: complete client idle timeout hardening --- .schema/pgdog.schema.json | 2 +- .schema/users.schema.json | 2 +- docs/CLIENT_CONNECTION.md | 12 ++++ example.pgdog.toml | 7 ++ example.users.toml | 3 + pgdog-config/src/core.rs | 68 ++++++++++++++++++- pgdog-config/src/database.rs | 4 +- pgdog-config/src/general.rs | 26 +++++++ pgdog-config/src/url.rs | 8 ++- pgdog-config/src/users.rs | 2 +- pgdog/src/backend/databases.rs | 48 +++++++++++++ .../src/backend/pool/connection/mirror/mod.rs | 2 +- pgdog/src/frontend/client/mod.rs | 10 ++- pgdog/src/frontend/client/test/mod.rs | 41 ++++++++++- pgdog/src/frontend/client/timeouts.rs | 54 +++++++++++++-- 15 files changed, 271 insertions(+), 18 deletions(-) diff --git a/.schema/pgdog.schema.json b/.schema/pgdog.schema.json index 8f6cbc938..91b6aa3e2 100644 --- a/.schema/pgdog.schema.json +++ b/.schema/pgdog.schema.json @@ -405,7 +405,7 @@ "type": "object", "properties": { "client_idle_timeout": { - "description": "Overrides the `client_idle_timeout` setting. Client connections to this database that haven't sent any queries for this long will be disconnected.\n\n**Note:** Set to `0` to exempt clients of this database from the client idle timeout entirely, e.g. for `LISTEN`/`NOTIFY` subscribers that are expected to stay quiet for long periods.\n\n", + "description": "Overrides the `client_idle_timeout` setting for this logical database. Client connections to this database that haven't sent any queries for this long will be disconnected.\n\nAll shards and replicas with the same `name` share one frontend timeout. The first configured non-`None` value is used, and conflicting values produce a warning. Set to `0` to exempt clients of this database from the client idle timeout entirely, e.g. for `LISTEN`/`NOTIFY` subscribers that are expected to stay quiet for long periods.\n\n", "type": [ "integer", "null" diff --git a/.schema/users.schema.json b/.schema/users.schema.json index 18b02dc8a..16b11fdcc 100644 --- a/.schema/users.schema.json +++ b/.schema/users.schema.json @@ -108,7 +108,7 @@ "default": false }, "client_idle_timeout": { - "description": "Overrides [`client_idle_timeout`](https://docs.pgdog.dev/configuration/pgdog.toml/general/#client_idle_timeout) for this user. Client connections that haven't sent any queries for this long will be disconnected.\n\n**Note:** Set to `0` to exempt this user from the client idle timeout entirely, e.g. for `LISTEN`/`NOTIFY` subscribers that are expected to stay quiet for long periods.\n\n", + "description": "Overrides [`client_idle_timeout`](https://docs.pgdog.dev/configuration/pgdog.toml/general/#client_idle_timeout) for this user. Client connections that haven't sent any queries for this long will be disconnected.\n\n**Note:** Set to `0` to exempt this user from the client idle timeout entirely, e.g. for `LISTEN`/`NOTIFY` subscribers that are expected to stay quiet for long periods. If multiple entries for this user match through `database`, `databases`, or `all_databases`, the last matching entry that configures this setting is used; entries without an override do not erase an earlier one.\n\n", "type": [ "integer", "null" diff --git a/docs/CLIENT_CONNECTION.md b/docs/CLIENT_CONNECTION.md index a52c8a056..1931f08dd 100644 --- a/docs/CLIENT_CONNECTION.md +++ b/docs/CLIENT_CONNECTION.md @@ -145,6 +145,18 @@ flowchart TD `self.buffer(client_state)` reads bytes from the client socket into a `ClientRequest` ([`frontend/client_request.rs`](../pgdog/src/frontend/client_request.rs)). A request is complete (`ClientRequest::is_complete()`) when the last message code is one of `{H, S, Q, c, f, F}`, or when a `CopyData` chunk reaches 4 KB. `'X'` (Terminate) triggers a graceful disconnect. +### Client idle timeout + +At the start of each `buffer()` invocation, the client checks the cached `client_idle_timeout` for its authenticated startup user and logical database. Resolution precedence is: + +1. The last matching `[[users]]` entry that configures `client_idle_timeout`, including matches through `databases` or `all_databases`. A later matching entry without the setting does not erase an earlier override. +2. The first configured non-`None` value among `[[databases]]` entries with the logical database name. Shards and replicas share this frontend policy, and conflicting values produce a configuration warning. +3. `[general].client_idle_timeout`. + +A value of `0` at the selected level disables the timeout for that client. This can exempt intentionally quiet sessions such as `LISTEN`/`NOTIFY` subscribers without disabling idle-client protection globally. Authenticated admin sessions always use the general timeout because the admin database is virtual and has no user or backend database configuration. + +The resolved timeout is cached with a weak identity handle to the configuration snapshot. A reload is applied on the next `buffer()` invocation; a socket read already waiting when the reload occurs keeps its current deadline. The full configuration snapshot is released before awaiting the frontend socket, so an indefinitely idle client does not retain obsolete configuration data. + ### Maintenance mode Before dispatching, `client_messages()` checks `maintenance_mode::waiter(&database)` ([`backend/maintenance_mode.rs`](../pgdog/src/backend/maintenance_mode.rs)). If a waiter is active and the client is not in a transaction, the client parks until `maintenance_mode::stop()` fires. diff --git a/example.pgdog.toml b/example.pgdog.toml index 0bbea3411..b2ed13b39 100644 --- a/example.pgdog.toml +++ b/example.pgdog.toml @@ -277,6 +277,9 @@ idle_timeout = 60_000 # Client idle timeout. How long to wait for clients to send another transaction # before disconnecting them. # +# Set to 0 to disable. This can also be overridden for one logical database +# or user in the corresponding configuration entry. +# # Default: unlimited client_idle_timeout = 60_000 # Size of the mirror queue. Queries that don't fit are dropped. @@ -340,6 +343,10 @@ port = 5432 # - replica # role = "primary" +# Optional client idle timeout for this logical database. All entries with the +# same name (shards and replicas) share the first configured value. Set to 0 to +# disable the timeout, for example for intentionally quiet LISTEN clients. +# client_idle_timeout = 0 # # Add a replica and automatically load balance queries. diff --git a/example.users.toml b/example.users.toml index da1d40337..66b0578f3 100644 --- a/example.users.toml +++ b/example.users.toml @@ -6,6 +6,9 @@ name = "pgdog" database = "pgdog" password = "pgdog" +# Optional client idle timeout for this user. Set to 0 to exempt intentionally +# quiet sessions such as LISTEN/NOTIFY subscribers. +# client_idle_timeout = 0 [[users]] name = "pgdog" diff --git a/pgdog-config/src/core.rs b/pgdog-config/src/core.rs index 77151bdde..76b09d0f0 100644 --- a/pgdog-config/src/core.rs +++ b/pgdog-config/src/core.rs @@ -176,8 +176,10 @@ impl ConfigAndUsers { /// Resolve `client_idle_timeout` for a connecting user/database pair. /// /// Precedence is user, then database, then general, matching - /// [`crate::pool::PoolConfig::resolve`]. `0` at any level means the - /// client is exempt from the timeout. + /// [`crate::pool::PoolConfig::resolve`]. Among overlapping user entries, + /// the last matching entry that configures this setting wins; entries + /// without an override do not erase an earlier one. `0` at any level means + /// the client is exempt from the timeout. pub fn client_idle_timeout(&self, user: &str, database: &str) -> Duration { // The admin database is virtual and never part of pool construction; // user and database overrides don't apply to it. @@ -844,6 +846,36 @@ mod tests { assert!(!single.has_database("missing")); } + #[test] + fn config_check_handles_conflicting_client_idle_timeouts() { + let mut config = Config { + databases: vec![ + Database { + name: "production".into(), + client_idle_timeout: Some(30_000), + ..Default::default() + }, + Database { + name: "production".into(), + client_idle_timeout: Some(60_000), + ..Default::default() + }, + ], + ..Default::default() + }; + + // Exercises the conflict-warning path. Validation is diagnostic and + // must leave the entries untouched while resolution keeps the first + // configured value. + config.check(); + assert_eq!(config.databases[0].client_idle_timeout, Some(30_000)); + assert_eq!(config.databases[1].client_idle_timeout, Some(60_000)); + assert_eq!( + config.database_client_idle_timeout("production"), + Some(30_000) + ); + } + #[test] fn client_idle_timeout_falls_back_to_general() { let config = ConfigAndUsers { @@ -893,6 +925,38 @@ mod tests { ); } + #[test] + fn client_idle_timeout_user_can_enable_disabled_general_timeout() { + let config = ConfigAndUsers { + config: Config { + general: General { + client_idle_timeout: 0, + ..Default::default() + }, + ..Default::default() + }, + users: Users { + users: vec![User { + name: "alice".into(), + database: "production".into(), + client_idle_timeout: Some(25_000), + ..Default::default() + }], + ..Default::default() + }, + ..Default::default() + }; + + assert_eq!( + config.client_idle_timeout("alice", "production"), + Duration::from_millis(25_000) + ); + assert_eq!( + config.client_idle_timeout("bob", "production"), + Duration::MAX + ); + } + #[test] fn client_idle_timeout_user_overrides_database() { let config = ConfigAndUsers { diff --git a/pgdog-config/src/database.rs b/pgdog-config/src/database.rs index a1b7d3999..7a302c35d 100644 --- a/pgdog-config/src/database.rs +++ b/pgdog-config/src/database.rs @@ -191,9 +191,9 @@ pub struct Database { /// /// pub idle_timeout: Option, - /// Overrides the `client_idle_timeout` setting. Client connections to this database that haven't sent any queries for this long will be disconnected. + /// Overrides the `client_idle_timeout` setting for this logical database. Client connections to this database that haven't sent any queries for this long will be disconnected. /// - /// **Note:** Set to `0` to exempt clients of this database from the client idle timeout entirely, e.g. for `LISTEN`/`NOTIFY` subscribers that are expected to stay quiet for long periods. + /// All shards and replicas with the same `name` share one frontend timeout. The first configured non-`None` value is used, and conflicting values produce a warning. Set to `0` to exempt clients of this database from the client idle timeout entirely, e.g. for `LISTEN`/`NOTIFY` subscribers that are expected to stay quiet for long periods. /// /// pub client_idle_timeout: Option, diff --git a/pgdog-config/src/general.rs b/pgdog-config/src/general.rs index 6b0dd03d2..610b2f6c5 100644 --- a/pgdog-config/src/general.rs +++ b/pgdog-config/src/general.rs @@ -1234,6 +1234,17 @@ impl General { Duration::from_millis(self.prepared_statements_ttl_jitter.min(ttl)) } + /// Effective general client idle timeout. + /// + /// A configured value of `0` disables the timeout. + pub fn client_idle_timeout(&self) -> Duration { + if self.client_idle_timeout == 0 { + Duration::MAX + } else { + Duration::from_millis(self.client_idle_timeout) + } + } + pub fn connect_attempt_delay(&self) -> Duration { Duration::from_millis(self.connect_attempt_delay) } @@ -1597,6 +1608,21 @@ mod tests { use super::*; use crate::test_utils::*; + #[test] + fn client_idle_timeout_zero_is_disabled() { + let general = General { + client_idle_timeout: 0, + ..Default::default() + }; + assert_eq!(general.client_idle_timeout(), Duration::MAX); + + let general = General { + client_idle_timeout: 25_000, + ..Default::default() + }; + assert_eq!(general.client_idle_timeout(), Duration::from_millis(25_000)); + } + #[test] fn test_prepared_statements_ttl_defaults() { let general = General::default(); diff --git a/pgdog-config/src/url.rs b/pgdog-config/src/url.rs index 1a2ab8b4d..85bfb2d87 100644 --- a/pgdog-config/src/url.rs +++ b/pgdog-config/src/url.rs @@ -68,6 +68,11 @@ impl From<&Url> for Database { database.idle_timeout = Some(timeout); } } + "client_idle_timeout" => { + if let Ok(timeout) = val.parse::() { + database.client_idle_timeout = Some(timeout); + } + } "read_only" => { if let Ok(read_only) = val.parse::() { database.read_only = Some(read_only); @@ -188,13 +193,14 @@ mod test { #[test] fn test_numeric_fields_from_query_params() { - let url = Url::parse("postgres://user:password@host:5432/name?pool_size=10&min_pool_size=2&statement_timeout=5000&idle_timeout=300&server_lifetime=3600&server_lifetime_jitter=600").unwrap(); + let url = Url::parse("postgres://user:password@host:5432/name?pool_size=10&min_pool_size=2&statement_timeout=5000&idle_timeout=300&client_idle_timeout=600&server_lifetime=3600&server_lifetime_jitter=600").unwrap(); let database = Database::from(&url); assert_eq!(database.pool_size, Some(10)); assert_eq!(database.min_pool_size, Some(2)); assert_eq!(database.statement_timeout, Some(5000)); assert_eq!(database.idle_timeout, Some(300)); + assert_eq!(database.client_idle_timeout, Some(600)); assert_eq!(database.server_lifetime, Some(3600)); assert_eq!(database.server_lifetime_jitter, Some(600)); } diff --git a/pgdog-config/src/users.rs b/pgdog-config/src/users.rs index 2b5098ce8..6da75eac9 100644 --- a/pgdog-config/src/users.rs +++ b/pgdog-config/src/users.rs @@ -373,7 +373,7 @@ pub struct User { pub idle_timeout: Option, /// Overrides [`client_idle_timeout`](https://docs.pgdog.dev/configuration/pgdog.toml/general/#client_idle_timeout) for this user. Client connections that haven't sent any queries for this long will be disconnected. /// - /// **Note:** Set to `0` to exempt this user from the client idle timeout entirely, e.g. for `LISTEN`/`NOTIFY` subscribers that are expected to stay quiet for long periods. + /// **Note:** Set to `0` to exempt this user from the client idle timeout entirely, e.g. for `LISTEN`/`NOTIFY` subscribers that are expected to stay quiet for long periods. If multiple entries for this user match through `database`, `databases`, or `all_databases`, the last matching entry that configures this setting is used; entries without an override do not erase an earlier one. /// /// pub client_idle_timeout: Option, diff --git a/pgdog/src/backend/databases.rs b/pgdog/src/backend/databases.rs index 75fac4512..110c613a8 100644 --- a/pgdog/src/backend/databases.rs +++ b/pgdog/src/backend/databases.rs @@ -635,6 +635,9 @@ pub(crate) fn from_config(config: &ConfigAndUsers) -> Databases { // The schema cache is shared between all databases. let schema_cache = SchemaCache::default(); + // Later overlapping user entries overwrite earlier clusters. Timeout + // resolution also prefers the last matching entry that configures its + // override; see `ConfigAndUsers::client_idle_timeout`. for user in &config.users.users { for database in config.config.user_databases(user) { let mut user = user.clone(); @@ -1649,6 +1652,51 @@ mod tests { assert_eq!(databases.all().len(), 3); } + #[test] + fn test_overlapping_user_and_timeout_resolution_choose_same_explicit_entry() { + let config = ConfigAndUsers { + config: Config { + databases: vec![Database { + name: "production".into(), + host: "localhost".into(), + role: Role::Primary, + ..Default::default() + }], + ..Default::default() + }, + users: crate::config::Users { + users: vec![ + crate::config::User { + name: "alice".into(), + all_databases: true, + pool_size: Some(10), + client_idle_timeout: Some(10_000), + ..Default::default() + }, + crate::config::User { + name: "alice".into(), + database: "production".into(), + pool_size: Some(20), + client_idle_timeout: Some(20_000), + ..Default::default() + }, + ], + ..Default::default() + }, + ..Default::default() + }; + + let databases = from_config(&config); + let cluster = databases.cluster(("alice", "production")).unwrap(); + let pools = cluster.shards()[0].pools(); + + assert_eq!(pools[0].config().max, 20); + assert_eq!( + config.client_idle_timeout("alice", "production"), + std::time::Duration::from_millis(20_000) + ); + } + #[test] fn test_new_pool_returns_none_for_nonexistent_database() { let config = Config::default(); // No databases configured diff --git a/pgdog/src/backend/pool/connection/mirror/mod.rs b/pgdog/src/backend/pool/connection/mirror/mod.rs index 4b52d3f2a..759788c3f 100644 --- a/pgdog/src/backend/pool/connection/mirror/mod.rs +++ b/pgdog/src/backend/pool/connection/mirror/mod.rs @@ -58,7 +58,7 @@ impl Mirror { id: FrontendPid::new(), prepared_statements, params: params.clone(), - timeouts: Timeouts::from_config(config, user, database), + timeouts: Timeouts::from_config(config, user, database, false), stream: Stream::dev_null(), transaction: None, } diff --git a/pgdog/src/frontend/client/mod.rs b/pgdog/src/frontend/client/mod.rs index 40ea8a291..bc4774b29 100644 --- a/pgdog/src/frontend/client/mod.rs +++ b/pgdog/src/frontend/client/mod.rs @@ -424,7 +424,7 @@ impl Client { params: params.clone(), prepared_statements: PreparedStatements::new(), transaction: None, - timeouts: Timeouts::from_config(&config, user, database), + timeouts: Timeouts::from_config(&config, user, database, admin), timeouts_config: Arc::downgrade(&config), client_request: ClientRequest::default(), stream_buffer: MessageBuffer::new( @@ -465,7 +465,7 @@ impl Client { prepared_statements, admin: false, transaction: None, - timeouts: Timeouts::from_config(&config, user, database), + timeouts: Timeouts::from_config(&config, user, database, false), timeouts_config: Arc::downgrade(&config), client_request: ClientRequest::default(), stream_buffer: MessageBuffer::new( @@ -656,13 +656,17 @@ impl Client { let timeouts_current = std::ptr::eq(self.timeouts_config.as_ptr(), Arc::as_ptr(&config)); if !timeouts_current { let (user, database) = user_database_from_params(&self.params); - self.timeouts = Timeouts::from_config(&config, user, database); + self.timeouts = Timeouts::from_config(&config, user, database, self.admin); self.timeouts_config = Arc::downgrade(&config); } self.query_log_stdout = config.config.general.query_log_stdout; self.query_size_limit = config.config.general.query_size_limit; self.stream_buffer .set_size_limit_block(config.config.general.frontend_query_size_limit_block()); + // Do not retain a full configuration snapshot while waiting on an idle + // client. `timeouts_config` keeps only the weak identity handle needed + // to detect a reload on the next invocation. + drop(config); while !self.client_request.is_complete() { let idle_timeout = self diff --git a/pgdog/src/frontend/client/test/mod.rs b/pgdog/src/frontend/client/test/mod.rs index 4e979764d..744276dd8 100644 --- a/pgdog/src/frontend/client/test/mod.rs +++ b/pgdog/src/frontend/client/test/mod.rs @@ -1,4 +1,7 @@ -use std::time::{Duration, Instant}; +use std::{ + sync::Arc, + time::{Duration, Instant}, +}; use pgdog_config::{PoolerMode, QuerySizeLimitAction}; use tokio::{ @@ -291,6 +294,42 @@ async fn test_client_idle_timeout() { ); } +#[tokio::test] +async fn test_idle_client_does_not_retain_config_snapshot() { + crate::logger(); + + let mut initial = (*config()).clone(); + initial.config.general.client_idle_timeout = 0; + initial.config.databases.clear(); + initial.users.users.clear(); + set(initial).unwrap(); + + let (conn, mut client) = parallel_test_client().await; + let previous = config(); + let previous_weak = Arc::downgrade(&previous); + drop(previous); + + let mut buffer = Box::pin(client.buffer(State::Idle)); + assert!( + timeout(Duration::from_millis(10), &mut buffer) + .await + .is_err(), + "disabled idle timeout should leave the frontend read pending" + ); + + let mut replacement = (*config()).clone(); + replacement.config.general.client_idle_timeout = 25; + set(replacement).unwrap(); + + assert!( + previous_weak.upgrade().is_none(), + "a pending idle read must not retain the previous configuration snapshot" + ); + + drop(buffer); + drop(conn); +} + #[tokio::test] async fn test_client_idle_timeout_user_override() { crate::logger(); diff --git a/pgdog/src/frontend/client/timeouts.rs b/pgdog/src/frontend/client/timeouts.rs index 66fd0386e..36acfff79 100644 --- a/pgdog/src/frontend/client/timeouts.rs +++ b/pgdog/src/frontend/client/timeouts.rs @@ -20,10 +20,19 @@ impl Default for Timeouts { } impl Timeouts { - pub(crate) fn from_config(config: &ConfigAndUsers, user: &str, database: &str) -> Self { + pub(crate) fn from_config( + config: &ConfigAndUsers, + user: &str, + database: &str, + admin: bool, + ) -> Self { Self { query_timeout: config.config.general.query_timeout(), - client_idle_timeout: config.client_idle_timeout(user, database), + client_idle_timeout: if admin { + config.config.general.client_idle_timeout() + } else { + config.client_idle_timeout(user, database) + }, idle_in_transaction_timeout: config.config.general.client_idle_in_transaction_timeout(), } } @@ -74,7 +83,7 @@ mod test { #[test] fn test_idle_in_transaction_timeout() { let config = config(); // Will be default. - let timeout = Timeouts::from_config(&config, "postgres", "postgres"); + let timeout = Timeouts::from_config(&config, "postgres", "postgres", false); let actual = timeout.client_idle_timeout(&State::IdleInTransaction, &ClientRequest::default()); @@ -112,10 +121,45 @@ mod test { ..Default::default() }; - let timeouts = Timeouts::from_config(&config, "listener", "pgdog"); + let timeouts = Timeouts::from_config(&config, "listener", "pgdog", false); assert_eq!(timeouts.client_idle_timeout, Duration::MAX); - let timeouts = Timeouts::from_config(&config, "other", "pgdog"); + let timeouts = Timeouts::from_config(&config, "other", "pgdog", false); assert_eq!(timeouts.client_idle_timeout, Duration::from_millis(60_000)); } + + #[test] + fn authenticated_admin_keeps_general_timeout_after_admin_config_changes() { + use pgdog_config::{Config, ConfigAndUsers, General, User, Users}; + + let mut config = ConfigAndUsers { + config: Config { + general: General { + client_idle_timeout: 60_000, + ..Default::default() + }, + ..Default::default() + }, + users: Users { + users: vec![User { + name: "admin".into(), + all_databases: true, + client_idle_timeout: Some(0), + ..Default::default() + }], + ..Default::default() + }, + ..Default::default() + }; + // Simulate a reload that changes the configured admin identity while + // an existing admin session remains authenticated as admin/admin. + config.config.admin.user = "root".into(); + config.config.admin.name = "control".into(); + + let admin = Timeouts::from_config(&config, "admin", "admin", true); + assert_eq!(admin.client_idle_timeout, Duration::from_millis(60_000)); + + let regular = Timeouts::from_config(&config, "admin", "admin", false); + assert_eq!(regular.client_idle_timeout, Duration::MAX); + } }