Re: Transaction timeout

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