public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
To: Alexander Korotkov <[email protected]>
Cc: Andrey Borodin <[email protected]>
Cc: Japin Li <[email protected]>
Cc: Peter Smith <[email protected]>
Cc: Junwang Zhao <[email protected]>
Cc: 邱宇航 <[email protected]>
Cc: Fujii Masao <[email protected]>
Cc: Andrew Borodin <[email protected]>
Cc: Michael Paquier <[email protected]>
Cc: Nikolay Samokhvalov <[email protected]>
Cc: pgsql-hackers mailing list <[email protected]>
Cc: pgsql-hackers mailing list <[email protected]>
Subject: Re: Transaction timeout
Date: Thu, 15 Feb 2024 15:08:56 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAPpHfds=ByL_MEZunF7P+AaDJ7xv7LD8kHcV9awWZx5fOJZBRw@mail.gmail.com>
References: <[email protected]>
<ME3P282MB3166A9BFA45239291EBC5F1CB667A@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM>
<[email protected]>
<CAHut+PuOdzypMrK3HsSr0F2fyn8VO5z2+EZ=6Sz39QNbBrisHQ@mail.gmail.com>
<[email protected]>
<ME3P282MB316697D91841CF4EE86CF882B6792@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM>
<[email protected]>
<ME3P282MB3166A01CE3BC1B8B9C846A75B67C2@ME3P282MB3166.AUSP282.PROD.OUTLOOK.COM>
<[email protected]>
<CAPpHfds=ByL_MEZunF7P+AaDJ7xv7LD8kHcV9awWZx5fOJZBRw@mail.gmail.com>
Hi,
On 2024-02-13 23:42:35 +0200, Alexander Korotkov wrote:
> diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
> index 464858117e0..a124ba59330 100644
> --- a/src/backend/access/transam/xact.c
> +++ b/src/backend/access/transam/xact.c
> @@ -2139,6 +2139,10 @@ StartTransaction(void)
> */
> s->state = TRANS_INPROGRESS;
>
> + /* Schedule transaction timeout */
> + if (TransactionTimeout > 0)
> + enable_timeout_after(TRANSACTION_TIMEOUT, TransactionTimeout);
> +
> ShowTransactionState("StartTransaction");
> }
Isn't it a problem that all uses of StartTransaction() trigger a timeout, but
transaction commit/abort don't? What if e.g. logical replication apply starts
a transaction, commits it, and then goes idle? The timer would still be
active, afaict?
I don't think it works well to enable timeouts in xact.c and to disable them
in PostgresMain().
> @@ -4491,12 +4511,18 @@ PostgresMain(const char *dbname, const char *username)
> pgstat_report_activity(STATE_IDLEINTRANSACTION_ABORTED, NULL);
>
> /* Start the idle-in-transaction timer */
> - if (IdleInTransactionSessionTimeout > 0)
> + if (IdleInTransactionSessionTimeout > 0
> + && (IdleInTransactionSessionTimeout < TransactionTimeout || TransactionTimeout == 0))
> {
> idle_in_transaction_timeout_enabled = true;
> enable_timeout_after(IDLE_IN_TRANSACTION_SESSION_TIMEOUT,
> IdleInTransactionSessionTimeout);
> }
> +
> + /* Schedule or reschedule transaction timeout */
> + if (TransactionTimeout > 0 && !get_timeout_active(TRANSACTION_TIMEOUT))
> + enable_timeout_after(TRANSACTION_TIMEOUT,
> + TransactionTimeout);
> }
> else if (IsTransactionOrTransactionBlock())
> {
> @@ -4504,12 +4530,18 @@ PostgresMain(const char *dbname, const char *username)
> pgstat_report_activity(STATE_IDLEINTRANSACTION, NULL);
>
> /* Start the idle-in-transaction timer */
> - if (IdleInTransactionSessionTimeout > 0)
> + if (IdleInTransactionSessionTimeout > 0
> + && (IdleInTransactionSessionTimeout < TransactionTimeout || TransactionTimeout == 0))
> {
> idle_in_transaction_timeout_enabled = true;
> enable_timeout_after(IDLE_IN_TRANSACTION_SESSION_TIMEOUT,
> IdleInTransactionSessionTimeout);
> }
> +
> + /* Schedule or reschedule transaction timeout */
> + if (TransactionTimeout > 0 && !get_timeout_active(TRANSACTION_TIMEOUT))
> + enable_timeout_after(TRANSACTION_TIMEOUT,
> + TransactionTimeout);
> }
> else
> {
Why do we need to do anything in these cases if the timer is started in
StartTransaction()?
> new file mode 100644
> index 00000000000..ce2c9a43011
> --- /dev/null
> +++ b/src/test/isolation/specs/timeouts-long.spec
> @@ -0,0 +1,35 @@
> +# Tests for transaction timeout that require long wait times
> +
> +session s7
> +step s7_begin
> +{
> + BEGIN ISOLATION LEVEL READ COMMITTED;
> + SET transaction_timeout = '1s';
> +}
> +step s7_commit_and_chain { COMMIT AND CHAIN; }
> +step s7_sleep { SELECT pg_sleep(0.6); }
> +step s7_abort { ABORT; }
> +
> +session s8
> +step s8_begin
> +{
> + BEGIN ISOLATION LEVEL READ COMMITTED;
> + SET transaction_timeout = '900ms';
> +}
> +# to test that quick query does not restart transaction_timeout
> +step s8_select_1 { SELECT 1; }
> +step s8_sleep { SELECT pg_sleep(0.6); }
> +
> +session checker
> +step checker_sleep { SELECT pg_sleep(0.3); }
Isn't this test going to be very fragile on busy / slow machines? What if the
pg_sleep() takes one second, because there were other tasks to schedule? I'd
be surprised if this didn't fail under valgrind, for example.
Greetings,
Andres Freund
view thread (61+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Transaction timeout
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox