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.fujii@oss.nttdata.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