From f75b18441db3a40f143ce094cb7e705ae34640db Mon Sep 17 00:00:00 2001 From: Andrey Borodin Date: Fri, 2 Dec 2022 21:01:29 -0800 Subject: [PATCH v4] Intorduce transaction_timeout Just like statement_timeout, but for transaction. Author: Andrey Borodin Reviewed-by: Andres Freund Discussion: https://postgr.es/m/CAAhFRxiQsRs2Eq5kCo9nXE3HTugsAAJdSQSmxncivebAxdmBjQ%40mail.gmail.com --- doc/src/sgml/config.sgml | 24 +++++++++ src/backend/postmaster/autovacuum.c | 1 + src/backend/storage/lmgr/proc.c | 1 + src/backend/tcop/postgres.c | 51 +++++++++++-------- src/backend/utils/errcodes.txt | 1 + src/backend/utils/init/postinit.c | 9 ++-- src/backend/utils/misc/guc_tables.c | 11 ++++ src/backend/utils/misc/postgresql.conf.sample | 7 +-- 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/expected/timeouts.out | 18 +++++++ src/test/isolation/specs/timeouts.spec | 8 +++ 16 files changed, 110 insertions(+), 28 deletions(-) diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml index 39d1c89e33..fd74db7527 100644 --- a/doc/src/sgml/config.sgml +++ b/doc/src/sgml/config.sgml @@ -9009,6 +9009,30 @@ 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. + + + + lock_timeout (integer) diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c index 601834d4b4..828a28af0a 100644 --- a/src/backend/postmaster/autovacuum.c +++ b/src/backend/postmaster/autovacuum.c @@ -588,6 +588,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 b1c35653fc..0170e226d0 100644 --- a/src/backend/storage/lmgr/proc.c +++ b/src/backend/storage/lmgr/proc.c @@ -61,6 +61,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 3082093d1e..1eaf2e054a 100644 --- a/src/backend/tcop/postgres.c +++ b/src/backend/tcop/postgres.c @@ -2691,6 +2691,9 @@ start_xact_command(void) { StartTransactionCommand(); + if (TransactionTimeout > 0 && !get_timeout_active(TRANSACTION_TIMEOUT)) + enable_timeout_after(TRANSACTION_TIMEOUT, TransactionTimeout); + xact_started = true; } @@ -3268,40 +3271,43 @@ ProcessInterrupts(void) { bool lock_timeout_occurred; bool stmt_timeout_occurred; + bool tx_timeout_occurred; + int err_code = ERRCODE_QUERY_CANCELED; QueryCancelPending = false; /* - * If LOCK_TIMEOUT and STATEMENT_TIMEOUT indicators are both set, we - * need to clear both, so always fetch both. + * If LOCK_TIMEOUT, STATEMENT_TIMEOUT and TRANSACTION indicators are set, we + * need to clear all of them, so always fetch each one. */ lock_timeout_occurred = get_timeout_indicator(LOCK_TIMEOUT, true); stmt_timeout_occurred = get_timeout_indicator(STATEMENT_TIMEOUT, true); - - /* - * If both were set, we want to report whichever timeout completed - * earlier; this ensures consistent behavior if the machine is slow - * enough that the second timeout triggers before we get here. A tie - * is arbitrarily broken in favor of reporting a lock timeout. - */ - if (lock_timeout_occurred && stmt_timeout_occurred && - get_timeout_finish_time(STATEMENT_TIMEOUT) < get_timeout_finish_time(LOCK_TIMEOUT)) - lock_timeout_occurred = false; /* report stmt timeout */ + tx_timeout_occurred = get_timeout_indicator(TRANSACTION_TIMEOUT, true); if (lock_timeout_occurred) + err_code = ERRCODE_LOCK_NOT_AVAILABLE; + + + if (lock_timeout_occurred || stmt_timeout_occurred || tx_timeout_occurred) { + /* Report all reasons for timeout */ + char* lock_reason = lock_timeout_occurred ? + _("lock timeout") : ""; + char* stmt_reason = stmt_timeout_occurred ? + _("statement timeout") : ""; + char* tx_reason = tx_timeout_occurred ? + _("transaction timeout") : ""; + char* comma1 = lock_timeout_occurred && stmt_timeout_occurred ? + "," : ""; + char* comma2 = (lock_timeout_occurred || stmt_timeout_occurred) + && tx_timeout_occurred ? "," : ""; LockErrorCleanup(); ereport(ERROR, - (errcode(ERRCODE_LOCK_NOT_AVAILABLE), - errmsg("canceling statement due to lock timeout"))); - } - if (stmt_timeout_occurred) - { - LockErrorCleanup(); - ereport(ERROR, - (errcode(ERRCODE_QUERY_CANCELED), - errmsg("canceling statement due to statement timeout"))); + (errcode(err_code), + errmsg("canceling statement due to %s%s%s%s%s", lock_reason, comma1, + stmt_reason, comma2, tx_reason))); } + if (IsAutoVacuumWorkerProcess()) { LockErrorCleanup(); @@ -4460,6 +4466,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 62418a051a..3ae2bbda70 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/postinit.c b/src/backend/utils/init/postinit.c index a990c833c5..779cb22915 100644 --- a/src/backend/utils/init/postinit.c +++ b/src/backend/utils/init/postinit.c @@ -73,7 +73,7 @@ static void PerformAuthentication(Port *port); static void CheckMyDatabase(const char *name, bool am_superuser, bool override_allow_connections); static void ShutdownPostgres(int code, Datum arg); static void StatementTimeoutHandler(void); -static void LockTimeoutHandler(void); +static void CancelOnTimeoutHandler(void); static void IdleInTransactionSessionTimeoutHandler(void); static void IdleSessionTimeoutHandler(void); static void IdleStatsUpdateTimeoutHandler(void); @@ -753,9 +753,10 @@ InitPostgres(const char *in_dbname, Oid dboid, { RegisterTimeout(DEADLOCK_TIMEOUT, CheckDeadLockAlert); RegisterTimeout(STATEMENT_TIMEOUT, StatementTimeoutHandler); - RegisterTimeout(LOCK_TIMEOUT, LockTimeoutHandler); + RegisterTimeout(LOCK_TIMEOUT, CancelOnTimeoutHandler); RegisterTimeout(IDLE_IN_TRANSACTION_SESSION_TIMEOUT, IdleInTransactionSessionTimeoutHandler); + RegisterTimeout(TRANSACTION_TIMEOUT, CancelOnTimeoutHandler); RegisterTimeout(IDLE_SESSION_TIMEOUT, IdleSessionTimeoutHandler); RegisterTimeout(CLIENT_CONNECTION_CHECK_TIMEOUT, ClientCheckTimeoutHandler); RegisterTimeout(IDLE_STATS_UPDATE_TIMEOUT, @@ -1340,10 +1341,10 @@ StatementTimeoutHandler(void) } /* - * LOCK_TIMEOUT handler: trigger a query-cancel interrupt. + * LOCK_TIMEOUT and TRANSACTION_TIMEOUT handler: trigger a query-cancel interrupt. */ static void -LockTimeoutHandler(void) +CancelOnTimeoutHandler(void) { #ifdef HAVE_SETSID /* try to signal whole process group */ diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index 1bf14eec66..ca21d2544c 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -2452,6 +2452,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 043864597f..a8c935f60c 100644 --- a/src/backend/utils/misc/postgresql.conf.sample +++ b/src/backend/utils/misc/postgresql.conf.sample @@ -687,10 +687,11 @@ #default_transaction_read_only = off #default_transaction_deferrable = off #session_replication_role = 'origin' -#statement_timeout = 0 # in milliseconds, 0 is disabled -#lock_timeout = 0 # in milliseconds, 0 is disabled +#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 +#idle_session_timeout = 0 # in milliseconds, 0 is disabled #vacuum_freeze_table_age = 150000000 #vacuum_freeze_min_age = 50000000 #vacuum_failsafe_age = 1600000000 diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index 0081873a72..5229fe3555 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -3089,6 +3089,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 44e8cd4704..87be74753d 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -1188,6 +1188,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 011c9cce6e..1b9674140a 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 795182fa51..484384b4ed 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 aa13e1d66e..a892c72765 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 c068986d09..c8fa0dc3d9 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/expected/timeouts.out b/src/test/isolation/expected/timeouts.out index 9328676f1c..6325ab4d00 100644 --- a/src/test/isolation/expected/timeouts.out +++ b/src/test/isolation/expected/timeouts.out @@ -79,3 +79,21 @@ 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: tsto sleep0 sleep1000 +step tsto: SET transaction_timeout = '30ms'; SET statement_timeout = '10s'; +step sleep0: SELECT pg_sleep(0.0001) +pg_sleep +-------- + +(1 row) + +step sleep1000: SELECT pg_sleep(1000) +step sleep1000: <... completed> +ERROR: canceling statement due to transaction timeout + +starting permutation: stto sleep1000 +step stto: SET transaction_timeout = '10s'; SET statement_timeout = '10ms'; +step sleep1000: SELECT pg_sleep(1000) +step sleep1000: <... completed> +ERROR: canceling statement due to statement timeout diff --git a/src/test/isolation/specs/timeouts.spec b/src/test/isolation/specs/timeouts.spec index c747b4ae28..82d7fc501a 100644 --- a/src/test/isolation/specs/timeouts.spec +++ b/src/test/isolation/specs/timeouts.spec @@ -23,6 +23,10 @@ step sto { SET statement_timeout = '10ms'; } step lto { SET lock_timeout = '10ms'; } step lsto { SET lock_timeout = '10ms'; SET statement_timeout = '10s'; } step slto { SET lock_timeout = '10s'; SET statement_timeout = '10ms'; } +step tsto { SET transaction_timeout = '30ms'; SET statement_timeout = '10s';} +step stto { SET transaction_timeout = '10s'; SET statement_timeout = '10ms';} +step sleep0 { SELECT pg_sleep(0.0001) } # expected to always finish in time +step sleep1000 { SELECT pg_sleep(1000) } # expected to timeout always step locktbl { LOCK TABLE accounts; } step update { DELETE FROM accounts WHERE accountid = 'checking'; } teardown { ABORT; } @@ -47,3 +51,7 @@ permutation wrtbl lto update(*) permutation wrtbl lsto update(*) # statement timeout expires first, row-level lock permutation wrtbl slto update(*) +# transaction timeout before statement timeout +permutation tsto sleep0 sleep1000(*) +# statement timeout before transaction timeout +permutation stto sleep1000(*) \ No newline at end of file -- 2.37.0 (Apple Git-136)