[prev in list] [next in list] [prev in thread] [next in thread]
List: postgresql-hackers
Subject: Re: Transaction timeout
From: "Andrey M. Borodin" <x4mmm () yandex-team ! ru>
Date: 2023-11-30 17:06:11
Message-ID: D40137D0-65E1-40C9-8C0C-820AF252BE31 () yandex-team ! ru
[Download RAW message or body]
> On 20 Nov 2023, at 06:33, =E9=82=B1=E5=AE=87=E8=88=AA =
<iamqyh@gmail.com> wrote:
Nikolay, Peter, Fujii, Tung, Yuhang, thank you for reviewing this.
I'll address feedback soon, this patch has been for a long time on my =
TODO list.
I've started with fixing problem of COMMIT AND CHAIN by restarting =
timeout counter.
Tomorrow I plan to fix raising of the timeout when the transaction is =
idle.
Renaming transaction_timeout to something else (to avoid confusion with =
prepared xacts) also seems correct to me.
Thanks!
Best regards, Andrey Borodin.
["v5-0001-Intorduce-transaction_timeout.patch" (v5-0001-Intorduce-transaction_timeout.patch)]
From 7c84e5a6786c61487bcd667b3c05ddb4c139bca2 Mon Sep 17 00:00:00 2001
From: Andrey Borodin <xformmm@amazon.com>
Date: Fri, 2 Dec 2022 21:01:29 -0800
Subject: [PATCH v5] Intorduce transaction_timeout
Just like statement_timeout, but for transaction.
Author: Andrey Borodin
Reviewed-by: TODO
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 | 52 +++++++++++--------
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 | 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/expected/timeouts.out | 18 +++++++
src/test/isolation/specs/timeouts.spec | 8 +++
16 files changed, 108 insertions(+), 25 deletions(-)
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index bd70ff2e4b..c4cb2655da 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -9058,6 +9058,30 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
</listitem>
</varlistentry>
+ <varlistentry id="guc-transaction-timeout" xreflabel="transaction_timeout">
+ <term><varname>transaction_timeout</varname> (<type>integer</type>)
+ <indexterm>
+ <primary><varname>transaction_timeout</varname> configuration \
parameter</primary> + </indexterm>
+ </term>
+ <listitem>
+ <para>
+ Cancel any transaction that spans longer than the specified amount of
+ time. The limit applies both to explicit transactions (started with
+ <command>BEGIN</command>) 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.
+ </para>
+
+ <para>
+ Setting <varname>transaction_timeout</varname> in
+ <filename>postgresql.conf</filename> is not recommended because it would
+ affect all sessions.
+ </para>
+ </listitem>
+ </varlistentry>
+
<varlistentry id="guc-lock-timeout" xreflabel="lock_timeout">
<term><varname>lock_timeout</varname> (<type>integer</type>)
<indexterm>
diff --git a/src/backend/postmaster/autovacuum.c \
b/src/backend/postmaster/autovacuum.c index 3a6f24a023..46a0d1cc82 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -599,6 +599,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 e9e445bb21..2ba5ab00a4 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 6a070b5d8c..4ad4c1d8b1 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;
}
@@ -3352,40 +3356,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();
@@ -4562,6 +4569,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/postinit.c b/src/backend/utils/init/postinit.c
index 552cf9d950..1fdaf7f5ec 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);
@@ -761,9 +761,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,
@@ -1383,10 +1384,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 7605eff9b9..af86fcbfc1 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 e48c066a5b..1abc976607 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -692,6 +692,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 e863913849..3b9f081c04 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -1242,6 +1242,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 ef74f32693..00ba63f827 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/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'; <waiting \
...> 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) <waiting ...>
+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) <waiting ...>
+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.1 (Apple Git-137.1)
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic