Repository navigation
Stop abandoned admin requests from holding database connections #583
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
5 commits
Select commit
Hold shift + click to select a range
8e1a2dc
Stop abandoned admin requests from holding database connections
claude c482b22
Document the admin statement timeout in the operations skill
claude 318233c
Merge remote-tracking branch 'origin/main' into ccr-37ca53f6-xza211-a…
claude 4a95095
Round sub-millisecond admin statement timeouts up to 1ms
claude c550b6a
Round admin statement timeouts up from nanoseconds
claude File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,117 @@ | ||
| //! Session limits for the connection pool that serves the admin UI. | ||
| //! | ||
| //! When a client gives up on a request (a closed tab, a proxy timeout, a | ||
| //! poll that outlives its interval), the handler is dropped but its query | ||
| //! keeps running on Postgres: nothing cancels it. sqlx then waits for that | ||
| //! query to finish before the connection can rejoin the pool. A slow admin | ||
| //! read polled from an open dashboard therefore stacks one running query | ||
| //! and one held connection per poll. | ||
| //! | ||
| //! These settings bound both: a statement timeout ends the abandoned query, | ||
| //! and the release hook returns its connection to the pool afterwards. | ||
| //! Without that hook, sqlx 0.9 treats the abandoned query's error as a broken | ||
| //! connection and closes it, and closing a TLS connection mid-stream hangs, | ||
| //! so the pool slot would never be freed. | ||
|
|
||
| use std::time::Duration; | ||
|
|
||
| use sqlx::postgres::{PgConnectOptions, PgConnection, PgPoolOptions}; | ||
| use sqlx::Connection; | ||
|
|
||
| /// `client_connection_check_interval` exists from Postgres 14. | ||
| const CLIENT_CONNECTION_CHECK_MIN_VERSION: u32 = 140_000; | ||
|
|
||
| /// Add `statement_timeout` to the connection's startup options, so no single | ||
| /// admin statement runs longer and `RESET statement_timeout` returns to this | ||
| /// value rather than to none. `None` or a zero duration leaves the server's | ||
| /// setting in place. | ||
| pub fn admin_connect_options( | ||
| options: PgConnectOptions, | ||
| statement_timeout: Option<Duration>, | ||
| ) -> PgConnectOptions { | ||
| match statement_timeout.filter(|timeout| !timeout.is_zero()) { | ||
| Some(timeout) => { | ||
| // Round up: a sub-millisecond timeout written as `0ms` would | ||
| // disable the limit instead of setting one. | ||
| let millis = timeout.as_nanos().div_ceil(1_000_000); | ||
| options.options([("statement_timeout", format!("{millis}ms"))]) | ||
| } | ||
| None => options, | ||
| } | ||
| } | ||
|
|
||
| /// Set the session limits that need a server version check, on each new | ||
| /// connection. Use with [`PgPoolOptions::after_connect`], or through | ||
| /// [`admin_pool_options`]. | ||
| pub async fn configure_admin_connection(conn: &mut PgConnection) -> Result<(), sqlx::Error> { | ||
| if conn | ||
| .server_version_num() | ||
| .is_some_and(|version| version >= CLIENT_CONNECTION_CHECK_MIN_VERSION) | ||
| { | ||
| // While a query runs, Postgres checks every 5 s whether the client is | ||
| // still connected, so a connection the UI server closed, or lost when | ||
| // it exited, stops its query instead of running it to the end. | ||
| sqlx::query("SET client_connection_check_interval = '5s'") | ||
| .execute(&mut *conn) | ||
| .await?; | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// Make a released connection reusable after its request was dropped | ||
| /// mid-query. Use with [`PgPoolOptions::after_release`], or through | ||
| /// [`admin_pool_options`]. | ||
| /// | ||
| /// The first ping waits for the abandoned query and reports its result: a | ||
| /// database error there (the statement timeout, say) belongs to that query, | ||
| /// not to the connection, which a second ping confirms. Never returns an | ||
| /// error: sqlx closes such a connection with a hard close, which hangs on a | ||
| /// TLS connection and leaks its pool slot. `Ok(false)` closes it gracefully. | ||
| pub async fn recover_admin_connection(conn: &mut PgConnection) -> Result<bool, sqlx::Error> { | ||
| match conn.ping().await { | ||
| Ok(()) => Ok(true), | ||
| Err(sqlx::Error::Database(_)) => Ok(conn.ping().await.is_ok()), | ||
| Err(_) => Ok(false), | ||
| } | ||
| } | ||
|
|
||
| /// `options` with [`configure_admin_connection`] on every new connection and | ||
| /// [`recover_admin_connection`] on every release. | ||
| pub fn admin_pool_options(options: PgPoolOptions) -> PgPoolOptions { | ||
| options | ||
| .after_connect(|conn, _meta| Box::pin(configure_admin_connection(conn))) | ||
| .after_release(|conn, _meta| Box::pin(recover_admin_connection(conn))) | ||
| } | ||
|
|
||
| #[cfg(test)] | ||
| mod tests { | ||
| use super::*; | ||
|
|
||
| fn timeout_option(timeout: Option<Duration>) -> Option<String> { | ||
| admin_connect_options(PgConnectOptions::new(), timeout) | ||
| .get_options() | ||
| .map(str::to_owned) | ||
| } | ||
|
|
||
| #[test] | ||
| fn statement_timeout_is_written_in_whole_milliseconds() { | ||
| assert_eq!( | ||
| timeout_option(Some(Duration::from_secs(60))).as_deref(), | ||
| Some("-c statement_timeout=60000ms") | ||
| ); | ||
| assert_eq!( | ||
| timeout_option(Some(Duration::from_nanos(1))).as_deref(), | ||
| Some("-c statement_timeout=1ms") | ||
| ); | ||
| assert_eq!( | ||
| timeout_option(Some(Duration::from_micros(500))).as_deref(), | ||
| Some("-c statement_timeout=1ms") | ||
| ); | ||
| assert_eq!( | ||
| timeout_option(Some(Duration::from_micros(1_500))).as_deref(), | ||
| Some("-c statement_timeout=2ms") | ||
| ); | ||
| assert_eq!(timeout_option(Some(Duration::ZERO)), None); | ||
| assert_eq!(timeout_option(None), None); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,91 @@ | ||
| //! The admin UI pool's session limits bound queries that a dropped request | ||
| //! leaves running on Postgres. | ||
|
|
||
| use std::time::{Duration, Instant}; | ||
|
|
||
| use sqlx::postgres::{PgConnectOptions, PgPoolOptions}; | ||
| use sqlx::PgPool; | ||
|
|
||
| async fn show(conn: &mut sqlx::PgConnection, setting: &str) -> String { | ||
| sqlx::query_scalar(sqlx::AssertSqlSafe(format!("SHOW {setting}"))) | ||
| .fetch_one(conn) | ||
| .await | ||
| .unwrap() | ||
| } | ||
|
|
||
| async fn admin_pool(statement_timeout: Duration) -> PgPool { | ||
| let options: PgConnectOptions = awa_testing::setup::database_url().parse().unwrap(); | ||
| awa_ui::admin_pool_options(PgPoolOptions::new()) | ||
| .max_connections(1) | ||
| .acquire_timeout(Duration::from_secs(20)) | ||
| .connect_with(awa_ui::admin_connect_options( | ||
| options, | ||
| Some(statement_timeout), | ||
| )) | ||
| .await | ||
| .unwrap() | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn statement_timeout_is_the_session_default() { | ||
| let pool = admin_pool(Duration::from_secs(2)).await; | ||
| let mut conn = pool.acquire().await.unwrap(); | ||
| assert_eq!(show(&mut conn, "statement_timeout").await, "2s"); | ||
| // A startup option, not a session SET, so RESET returns to it. | ||
| sqlx::query("SET statement_timeout = 0") | ||
| .execute(&mut *conn) | ||
| .await | ||
| .unwrap(); | ||
| sqlx::query("RESET statement_timeout") | ||
| .execute(&mut *conn) | ||
| .await | ||
| .unwrap(); | ||
| assert_eq!(show(&mut conn, "statement_timeout").await, "2s"); | ||
|
|
||
| let version: i32 = sqlx::query_scalar("SELECT current_setting('server_version_num')::int") | ||
| .fetch_one(&mut *conn) | ||
| .await | ||
| .unwrap(); | ||
| if version >= 140_000 { | ||
| assert_eq!( | ||
| show(&mut conn, "client_connection_check_interval").await, | ||
| "5s" | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| #[tokio::test] | ||
| async fn a_dropped_request_frees_its_connection_at_the_statement_timeout() { | ||
| let pool = admin_pool(Duration::from_secs(2)).await; | ||
| let backend: i32 = sqlx::query_scalar("SELECT pg_backend_pid()") | ||
| .fetch_one(&pool) | ||
| .await | ||
| .unwrap(); | ||
|
|
||
| // The client gives up on a slow read: its future is dropped mid-query. | ||
| let abandoned = tokio::time::timeout( | ||
| Duration::from_millis(300), | ||
| sqlx::query("SELECT pg_sleep(30)").execute(&pool), | ||
| ) | ||
| .await; | ||
| assert!( | ||
| abandoned.is_err(), | ||
| "the read was still running when dropped" | ||
| ); | ||
|
|
||
| // The pool's one connection waits for that query to end before it can | ||
| // be reused; the statement timeout ends it, rather than its 30 s sleep. | ||
| let started = Instant::now(); | ||
| let after: i32 = sqlx::query_scalar("SELECT pg_backend_pid()") | ||
| .fetch_one(&pool) | ||
| .await | ||
| .unwrap(); | ||
| // The same connection, recovered rather than closed: closing it after | ||
| // the timeout's error is the path that hangs over TLS. | ||
| assert_eq!(after, backend); | ||
| assert!( | ||
| started.elapsed() < Duration::from_secs(10), | ||
| "the next request waited {:?} for the abandoned query", | ||
| started.elapsed() | ||
| ); | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.