From 7cec923b6c772ac1a8bc5a96c540f4894e61ec90 Mon Sep 17 00:00:00 2001 From: "Andrey M. Borodin" Date: Sun, 3 Dec 2023 23:18:00 +0500 Subject: [PATCH v6] Introduce transaction_timeout This commit adds timeout that is expected to be used as a prevention of long-running queries. Any session within transaction will be terminated after spanning longer than this timeout. However, this timeout is not applied to prepared transactions. Only transactions with user connections are affected. Author: Andrey Borodin Reviewed-by: Nikolay Samokhvalov Reviewed-by: Andres Freund Reviewed-by: Fujii Masao Reviewed-by: bt23nguyent Reviewed-by: Yuhang Qiu Discussion: https://postgr.es/m/CAAhFRxiQsRs2Eq5kCo9nXE3HTugsAAJdSQSmxncivebAxdmBjQ%40mail.gmail.com --- doc/src/sgml/config.sgml | 28 ++++++++++++++++ src/backend/postmaster/autovacuum.c | 1 + src/backend/storage/lmgr/proc.c | 1 + src/backend/tcop/postgres.c | 18 ++++++++++ src/backend/utils/errcodes.txt | 1 + src/backend/utils/init/globals.c | 1 + src/backend/utils/init/postinit.c | 10 ++++++ src/backend/utils/misc/guc_tables.c | 11 +++++++ src/backend/utils/misc/postgresql.conf.sample | 1 + src/bin/pg_dump/pg_backup_archiver.c | 1 + src/bin/pg_dump/pg_dump.c | 2 ++ src/bin/pg_rewind/libpq_source.c | 1 + src/include/miscadmin.h | 1 + src/include/storage/proc.h | 1 + src/include/utils/timeout.h | 1 + src/test/isolation/Makefile | 3 ++ src/test/isolation/expected/timeouts.out | 33 ++++++++++++++++++- src/test/isolation/specs/timeouts.spec | 25 +++++++++++++- 18 files changed, 138 insertions(+), 2 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 94d1eb2b81..14c3d1727d 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -9075,6 +9075,34 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv; + + transaction_timeout (integer) + + transaction_timeout configuration parameter + + + + + Cancel any transaction that spans longer than the specified amount of + time. The limit applies both to explicit transactions (started with + BEGIN) and to implicitly started transaction + corresponding to single statement. + If this value is specified without units, it is taken as milliseconds. + A value of zero (the default) disables the timeout. + + + + Setting transaction_timeout in + postgresql.conf is not recommended because it would + affect all sessions. + + + + Prepared transactions are not subject for this timeout. + + + + lock_timeout (integer) diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 3e037248d6..d5617b3199 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -586,6 +586,7 @@ AutoVacLauncherMain(int argc, char *argv[]) * regular maintenance from being executed. */ SetConfigOption("statement_timeout", "0", PGC_SUSET, PGC_S_OVERRIDE); + SetConfigOption("transaction_timeout", "0", PGC_SUSET, PGC_S_OVERRIDE); SetConfigOption("lock_timeout", "0", PGC_SUSET, PGC_S_OVERRIDE); SetConfigOption("idle_in_transaction_session_timeout", "0", PGC_SUSET, PGC_S_OVERRIDE); diff --git a/src/backend/storage/lmgr/proc.c b/src/backend/storage/lmgr/proc.c index b6451d9d08..4be06c1e5d 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -59,6 +59,7 @@ int DeadlockTimeout = 1000; int StatementTimeout = 0; int LockTimeout = 0; int IdleInTransactionSessionTimeout = 0; +int TransactionTimeout = 0; int IdleSessionTimeout = 0; bool log_lock_waits = false; diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c index 7298a187d1..be985b5994 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -2745,6 +2745,10 @@ start_xact_command(void) { StartTransactionCommand(); + /* Schedule or reschedule transaction timeout */ + if (TransactionTimeout > 0) + enable_timeout_after(TRANSACTION_TIMEOUT, TransactionTimeout); + xact_started = true; } @@ -3426,6 +3430,17 @@ ProcessInterrupts(void) IdleInTransactionSessionTimeoutPending = false; } + if (TransactionTimeoutPending) + { + /* As above, ignore the signal if the GUC has been reset to zero. */ + if (TransactionTimeout > 0) + ereport(FATAL, + (errcode(ERRCODE_TRANSACTION_TIMEOUT), + errmsg("terminating connection due to transaction timeout"))); + else + TransactionTimeoutPending = false; + } + if (IdleSessionTimeoutPending) { /* As above, ignore the signal if the GUC has been reset to zero. */ @@ -4562,6 +4577,9 @@ PostgresMain(const char *dbname, const char *username) enable_timeout_after(IDLE_SESSION_TIMEOUT, IdleSessionTimeout); } + + if (get_timeout_active(TRANSACTION_TIMEOUT)) + disable_timeout(TRANSACTION_TIMEOUT, false); } /* Report any recently-changed GUC options */ diff --git a/src/backend/utils/errcodes.txt b/src/backend/utils/errcodes.txt index 8e97a0150f..8f1157afee 100644 --- a/src/backend/utils/errcodes.txt +++ b/src/backend/utils/errcodes.txt @@ -252,6 +252,7 @@ Section: Class 25 - Invalid Transaction State 25P01 E ERRCODE_NO_ACTIVE_SQL_TRANSACTION no_active_sql_transaction 25P02 E ERRCODE_IN_FAILED_SQL_TRANSACTION in_failed_sql_transaction 25P03 E ERRCODE_IDLE_IN_TRANSACTION_SESSION_TIMEOUT idle_in_transaction_session_timeout +25P04 E ERRCODE_TRANSACTION_TIMEOUT transaction_timeout Section: Class 26 - Invalid SQL Statement Name diff --git a/src/backend/utils/init/globals.c b/src/backend/utils/init/globals.c index 60bc1217fb..fd586c193c 100644 --- a/src/backend/utils/init/globals.c +++ b/src/backend/utils/init/globals.c @@ -33,6 +33,7 @@ volatile sig_atomic_t ProcDiePending = false; volatile sig_atomic_t CheckClientConnectionPending = false; volatile sig_atomic_t ClientConnectionLost = false; volatile sig_atomic_t IdleInTransactionSessionTimeoutPending = false; +volatile sig_atomic_t TransactionTimeoutPending = false; volatile sig_atomic_t IdleSessionTimeoutPending = false; volatile sig_atomic_t ProcSignalBarrierPending = false; volatile sig_atomic_t LogMemoryContextPending = false; diff --git a/src/backend/utils/init/postinit.c b/src/backend/utils/init/postinit.c index 552cf9d950..64be4de0c7 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -75,6 +75,7 @@ static void ShutdownPostgres(int code, Datum arg); static void StatementTimeoutHandler(void); static void LockTimeoutHandler(void); static void IdleInTransactionSessionTimeoutHandler(void); +static void TransactionTimeoutHandler(void); static void IdleSessionTimeoutHandler(void); static void IdleStatsUpdateTimeoutHandler(void); static void ClientCheckTimeoutHandler(void); @@ -764,6 +765,7 @@ InitPostgres(const char *in_dbname, Oid dboid, RegisterTimeout(LOCK_TIMEOUT, LockTimeoutHandler); RegisterTimeout(IDLE_IN_TRANSACTION_SESSION_TIMEOUT, IdleInTransactionSessionTimeoutHandler); + RegisterTimeout(TRANSACTION_TIMEOUT, TransactionTimeoutHandler); RegisterTimeout(IDLE_SESSION_TIMEOUT, IdleSessionTimeoutHandler); RegisterTimeout(CLIENT_CONNECTION_CHECK_TIMEOUT, ClientCheckTimeoutHandler); RegisterTimeout(IDLE_STATS_UPDATE_TIMEOUT, @@ -1395,6 +1397,14 @@ LockTimeoutHandler(void) kill(MyProcPid, SIGINT); } +static void +TransactionTimeoutHandler(void) +{ + TransactionTimeoutPending = true; + InterruptPending = true; + SetLatch(MyLatch); +} + static void IdleInTransactionSessionTimeoutHandler(void) { diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 6474e35ec0..4b479e9be9 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -2544,6 +2544,17 @@ struct config_int ConfigureNamesInt[] = NULL, NULL, NULL }, + { + {"transaction_timeout", PGC_USERSET, CLIENT_CONN_STATEMENT, + gettext_noop("Sets the maximum allowed in a transaction."), + gettext_noop("A value of 0 turns off the timeout."), + GUC_UNIT_MS + }, + &TransactionTimeout, + 0, 0, INT_MAX, + NULL, NULL, NULL + }, + { {"idle_session_timeout", PGC_USERSET, CLIENT_CONN_STATEMENT, gettext_noop("Sets the maximum allowed idle time between queries, when not in a transaction."), diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample index cf9f283cfe..07ebec7709 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -695,6 +695,7 @@ #default_transaction_deferrable = off #session_replication_role = 'origin' #statement_timeout = 0 # in milliseconds, 0 is disabled +#transaction_timeout = 0 # in milliseconds, 0 is disabled #lock_timeout = 0 # in milliseconds, 0 is disabled #idle_in_transaction_session_timeout = 0 # in milliseconds, 0 is disabled #idle_session_timeout = 0 # in milliseconds, 0 is disabled diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 256d1e35a4..d97ebaff5b 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -3115,6 +3115,7 @@ _doSetFixedOutputState(ArchiveHandle *AH) ahprintf(AH, "SET statement_timeout = 0;\n"); ahprintf(AH, "SET lock_timeout = 0;\n"); ahprintf(AH, "SET idle_in_transaction_session_timeout = 0;\n"); + ahprintf(AH, "SET transaction_timeout = 0;\n"); /* Select the correct character set encoding */ ahprintf(AH, "SET client_encoding = '%s';\n", diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 8c0b5486b9..ea0ebb791e 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -1251,6 +1251,8 @@ setup_connection(Archive *AH, const char *dumpencoding, ExecuteSqlStatement(AH, "SET lock_timeout = 0"); if (AH->remoteVersion >= 90600) ExecuteSqlStatement(AH, "SET idle_in_transaction_session_timeout = 0"); + if (AH->remoteVersion >= 160000) + ExecuteSqlStatement(AH, "SET transaction_timeout = 0"); /* * Quote all identifiers, if requested. diff --git a/src/bin/pg_rewind/libpq_source.c b/src/bin/pg_rewind/libpq_source.c index 417c74cfef..9cda3f3667 100644 --- a/src/bin/pg_rewind/libpq_source.c +++ b/src/bin/pg_rewind/libpq_source.c @@ -117,6 +117,7 @@ init_libpq_conn(PGconn *conn) run_simple_command(conn, "SET statement_timeout = 0"); run_simple_command(conn, "SET lock_timeout = 0"); run_simple_command(conn, "SET idle_in_transaction_session_timeout = 0"); + run_simple_command(conn, "SET transaction_timeout = 0"); /* * we don't intend to do any updates, put the connection in read-only mode diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h index f0cc651435..732ca7d0f6 100644 --- a/src/include/miscadmin.h +++ b/src/include/miscadmin.h @@ -91,6 +91,7 @@ extern PGDLLIMPORT volatile sig_atomic_t InterruptPending; extern PGDLLIMPORT volatile sig_atomic_t QueryCancelPending; extern PGDLLIMPORT volatile sig_atomic_t ProcDiePending; extern PGDLLIMPORT volatile sig_atomic_t IdleInTransactionSessionTimeoutPending; +extern PGDLLIMPORT volatile sig_atomic_t TransactionTimeoutPending; extern PGDLLIMPORT volatile sig_atomic_t IdleSessionTimeoutPending; extern PGDLLIMPORT volatile sig_atomic_t ProcSignalBarrierPending; extern PGDLLIMPORT volatile sig_atomic_t LogMemoryContextPending; diff --git a/src/include/storage/proc.h b/src/include/storage/proc.h index 4b25961249..a49a83607f 100644 --- a/src/include/storage/proc.h +++ b/src/include/storage/proc.h @@ -428,6 +428,7 @@ extern PGDLLIMPORT int DeadlockTimeout; extern PGDLLIMPORT int StatementTimeout; extern PGDLLIMPORT int LockTimeout; extern PGDLLIMPORT int IdleInTransactionSessionTimeout; +extern PGDLLIMPORT int TransactionTimeout; extern PGDLLIMPORT int IdleSessionTimeout; extern PGDLLIMPORT bool log_lock_waits; diff --git a/src/include/utils/timeout.h b/src/include/utils/timeout.h index 8a61853371..608a83d5a8 100644 --- a/src/include/utils/timeout.h +++ b/src/include/utils/timeout.h @@ -31,6 +31,7 @@ typedef enum TimeoutId STANDBY_TIMEOUT, STANDBY_LOCK_TIMEOUT, IDLE_IN_TRANSACTION_SESSION_TIMEOUT, + TRANSACTION_TIMEOUT, IDLE_SESSION_TIMEOUT, IDLE_STATS_UPDATE_TIMEOUT, CLIENT_CONNECTION_CHECK_TIMEOUT, diff --git a/src/test/isolation/Makefile b/src/test/isolation/Makefile index ade2256ed3..482bb31949 100644 --- a/src/test/isolation/Makefile +++ b/src/test/isolation/Makefile @@ -72,3 +72,6 @@ installcheck-prepared-txns: all temp-install check-prepared-txns: all temp-install $(pg_isolation_regress_check) --schedule=$(srcdir)/isolation_schedule prepared-transactions prepared-transactions-cic + +check-timeouts: all temp-install + $(pg_isolation_regress_check) timeouts diff --git a/src/test/isolation/expected/timeouts.out b/src/test/isolation/expected/timeouts.out index 9328676f1c..fe9e5dab87 100644 --- a/src/test/isolation/expected/timeouts.out +++ b/src/test/isolation/expected/timeouts.out @@ -1,4 +1,4 @@ -Parsed test spec with 2 sessions +Parsed test spec with 5 sessions starting permutation: rdtbl sto locktbl step rdtbl: SELECT * FROM accounts; @@ -79,3 +79,34 @@ step slto: SET lock_timeout = '10s'; SET statement_timeout = '10ms'; step update: DELETE FROM accounts WHERE accountid = 'checking'; step update: <... completed> ERROR: canceling statement due to statement timeout + +starting permutation: stt1_set stt1_begin sleep_here stt2_set stt2_begin sleep_there stt3_check stt2_check +step stt1_set: SET transaction_timeout = '1ms'; +step stt1_begin: BEGIN ISOLATION LEVEL READ COMMITTED; +step sleep_here: SELECT pg_sleep(0.01); +FATAL: terminating connection due to transaction timeout +server closed the connection unexpectedly + This probably means the server terminated abnormally + before or while processing the request. + +step stt2_set: SET transaction_timeout = '1ms'; +step stt2_begin: BEGIN ISOLATION LEVEL READ COMMITTED; +step sleep_there: SELECT pg_sleep(0.01); +pg_sleep +-------- + +(1 row) + +step stt3_check: SELECT count(*) FROM pg_stat_activity WHERE application_name = 'isolation/timeouts/stt2' +count +----- + 0 +(1 row) + +step stt2_check: SELECT 1; +step stt2_check: <... completed> +FATAL: terminating connection due to transaction timeout +server closed the connection unexpectedly + This probably means the server terminated abnormally + before or while processing the request. + diff --git a/src/test/isolation/specs/timeouts.spec b/src/test/isolation/specs/timeouts.spec index c747b4ae28..10f55f1322 100644 --- a/src/test/isolation/specs/timeouts.spec +++ b/src/test/isolation/specs/timeouts.spec @@ -18,7 +18,7 @@ step wrtbl { UPDATE accounts SET balance = balance + 100; } teardown { ABORT; } session s2 -setup { BEGIN ISOLATION LEVEL READ COMMITTED; } +setup { SET transaction_timeout = '10s'; BEGIN ISOLATION LEVEL READ COMMITTED; } step sto { SET statement_timeout = '10ms'; } step lto { SET lock_timeout = '10ms'; } step lsto { SET lock_timeout = '10ms'; SET statement_timeout = '10s'; } @@ -27,6 +27,25 @@ step locktbl { LOCK TABLE accounts; } step update { DELETE FROM accounts WHERE accountid = 'checking'; } teardown { ABORT; } +session stt1 +# enable statement_timeout to check interaction +setup { SET statement_timeout = '10s'; SET lock_timeout = '10s'; } +step stt1_set { SET transaction_timeout = '1ms'; } +step stt1_begin { BEGIN ISOLATION LEVEL READ COMMITTED; } +step sleep_here { SELECT pg_sleep(0.01); } + +session stt2 +setup { SET statement_timeout = '10s'; SET lock_timeout = '10s'; } +step stt2_set { SET transaction_timeout = '1ms'; } +step stt2_begin { BEGIN ISOLATION LEVEL READ COMMITTED; } +# Session stt2 is terminated in the background. However, isolation tester needs a step to observe it. +step stt2_check { SELECT 1; } + +session stt3 +step sleep_there{ SELECT pg_sleep(0.01); } +# Observe that stt2 died +step stt3_check { SELECT count(*) FROM pg_stat_activity WHERE application_name = 'isolation/timeouts/stt2' } + # It's possible that the isolation tester will not observe the final # steps as "waiting", thanks to the relatively short timeouts we use. # We can ensure consistent test output by marking those steps with (*). @@ -47,3 +66,7 @@ permutation wrtbl lto update(*) permutation wrtbl lsto update(*) # statement timeout expires first, row-level lock permutation wrtbl slto update(*) + +# timeout of active query, idle transaction timeout +permutation stt1_set stt1_begin sleep_here stt2_set stt2_begin sleep_there stt3_check stt2_check(*) +# can't run tests after this, sessions stt1 and stt2 are expected to FATAL-out -- 2.37.1 (Apple Git-137.1)