Re: Transaction timeout

On 2023-09-06 20:32, Andrey M. Borodin wrote:
> Thanks for looking into this!
>
>> On 6 Sep 2023, at 13:16, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com>
>> wrote:
>>
>> While testing v4 patch, I noticed it doesn't handle the COMMIT AND
>> CHAIN case correctly.
>> When COMMIT AND CHAIN is executed, I believe the transaction timeout
>> counter should reset
>> and start from zero with the next transaction. However, it appears
>> that the current
>> v4 patch doesn't reset the counter in this scenario. Can you confirm
>> this?
> Yes, I was not aware of this feature. I'll test and fix this.
>
>> With the v4 patch, I found that timeout errors no longer occur during
>> the idle in
>> transaction phase. Instead, they occur when the next statement is
>> executed. Is this
>> the intended behavior?
> AFAIR I had been testing that behaviour of "idle in transaction" was
> intact. I'll check that again.
>
>> I thought some users might want to use the transaction timeout
>> feature to prevent prolonged transactions and promptly release
>> resources (e.g., locks)
>> in case of a timeout, similar to idle_in_transaction_session_timeout.
> Yes, this is exactly how I was expecting the feature to behave: empty
> up max_connections slots for long-hanging transactions.
>
> Thanks for your findings, I'll check and post new version!
>
>
> Best regards, Andrey Borodin.
Hi,

Thank you for implementing this nice feature!
I tested the v4 patch in the interactive transaction mode with 3
following cases:

1. Start a transaction with transaction_timeout=0 (i.e., timeout
disabled), and then change the timeout value to more than 0 during the
transaction.

=# SET transaction_timeout TO 0;
=# BEGIN; //timeout is not enabled
=# SELECT pg_sleep(5);
=# SET transaction_timeout TO '1s';
=# SELECT pg_sleep(10); //timeout is enabled with 1s
In this case, the transaction timeout happens during pg_sleep(10).

2. Start a transaction with transaction_timeout>0 (i.e., timeout
enabled), and then change the timeout value to more than 0 during the
transaction.

=# SET transaction_timeout TO '1000s';
=# BEGIN; //timeout is enabled with 1000s
=# SELECT pg_sleep(5);
=# SET transaction_timeout TO '1s';
=# SELECT pg_sleep(10); //timeout is not restarted and still running
with 1000s
In this case, the transaction timeout does NOT happen during
pg_sleep(10).

3. Start a transaction with transaction_timeout>0 (i.e., timeout
enabled), and then change the timeout value to 0 during the transaction.

=# SET transaction_timeout TO '10s';
=# BEGIN; //timeout is enabled with 10s
=# SELECT pg_sleep(5);
=# SET transaction_timeout TO 0;
=# SELECT pg_sleep(10); //timeout is NOT disabled and still running
with 10s
In this case, the transaction timeout happens during pg_sleep(10).

The first case where transaction_timeout is disabled before the
transaction begins is totally fine. However, in the second and third
cases, where transaction_timeout is enabled before the transaction
begins, since the timeout has already enabled with a certain value, it
will not be enabled again with a new setting value.

Furthermore, let's say I want to set a transaction_timeout value for all
transactions in postgresql.conf file so it would affect all sessions.
The same behavior happened but for all 3 cases, here is one example with
the second case:

=# BEGIN; SHOW transaction_timeout; select pg_sleep(10); SHOW
transaction_timeout; COMMIT;
BEGIN
transaction_timeout
---------------------
15s
(1 row)

2023-09-07 11:52:50.510 JST [23889] LOG: received SIGHUP, reloading
configuration files
2023-09-07 11:52:50.510 JST [23889] LOG: parameter
"transaction_timeout" changed to "5000"
pg_sleep
----------

(1 row)

transaction_timeout
---------------------
5s
(1 row)

COMMIT

I am of the opinion that these behaviors might lead to confusion among
users. Could you confirm if these are the intended behaviors?

Additionally, I think the short description should be "Sets the maximum
allowed time to commit a transaction." or "Sets the maximum allowed time
to wait before aborting a transaction." so that it could be more clear
and consistent with other %_timeout descriptions.

Also, there is a small whitespace error here:
src/backend/tcop/postgres.c:3373: space before tab in indent.
+
stmt_reason, comma2, tx_reason)));

On a side note, while testing the patch with pgbench, it came to my
attention that in scenarios involving the execution of multiple
concurrent transactions within a high contention environment and with
relatively short timeout durations, there is a potential for cascading
blocking. This phenomenon can lead to multiple transactions exceeding
their designated timeouts, consequently resulting in a degradation of
transaction processing performance. No?
Do you think this feature should be co-implemented with the existing
concurrency control protocol to maintain the transaction performance
(e.g. a transaction scheduling mechanism based on transaction timeout)?

Regards,
Tung Nguyen