mirrors
/
postgres
mirror of https://github.com/postgres/postgres
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Followup fixes for transaction_timeout 

Don't deal with transaction timeout in PostgresMain().  Instead, release
transaction timeout activated by StartTransaction() in
CommitTransaction()/AbortTransaction()/PrepareTransaction().  Deal with both
enabling and disabling transaction timeout in assign_transaction_timeout().

Also, remove potentially flaky timeouts-long isolation test, which has no
guarantees to pass on slow/busy machines.

Reported-by: Andres Freund
Discussion: https://postgr.es/m/20240215230856.pc6k57tqxt7fhldm%40awork3.anarazel.de
This commit is contained in:
Alexander Korotkov 2024-02-16 03:36:38 +02:00
parent 51efe38cb9
commit bf82f43790
4 changed files with 23 additions and 56 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

12
src/backend/access/transam/xact.c
View File

@ -2262,6 +2262,10 @@ CommitTransaction(void)
s->state = TRANS_COMMIT;
s->parallelModeLevel = 0;
/* Disable transaction timeout */
if (TransactionTimeout > 0)
disable_timeout(TRANSACTION_TIMEOUT, false);
if (!is_parallel_worker)
{
/*
@ -2535,6 +2539,10 @@ PrepareTransaction(void)
*/
s->state = TRANS_PREPARE;
/* Disable transaction timeout */
if (TransactionTimeout > 0)
disable_timeout(TRANSACTION_TIMEOUT, false);
prepared_at = GetCurrentTimestamp();
/*
@ -2707,6 +2715,10 @@ AbortTransaction(void)
/* Prevent cancel/die interrupt while cleaning up */
HOLD_INTERRUPTS();
/* Disable transaction timeout */
if (TransactionTimeout > 0)
disable_timeout(TRANSACTION_TIMEOUT, false);
/* Make sure we have a valid memory context and resource owner */
AtAbort_Memory();
AtAbort_ResourceOwner();

31
src/backend/tcop/postgres.c
View File

@ -3647,9 +3647,17 @@ check_log_stats(bool *newval, void **extra, GucSource source)
void
assign_transaction_timeout(int newval, void *extra)
{
if (TransactionTimeout <= 0 &&
get_timeout_active(TRANSACTION_TIMEOUT))
disable_timeout(TRANSACTION_TIMEOUT, false);
if (IsTransactionState())
{
/*
* If transaction_timeout GUC has changes within the transaction block
* enable or disable the timer correspondingly.
*/
if (newval > 0 && !get_timeout_active(TRANSACTION_TIMEOUT))
enable_timeout_after(TRANSACTION_TIMEOUT, newval);
else if (newval <= 0 && get_timeout_active(TRANSACTION_TIMEOUT))
disable_timeout(TRANSACTION_TIMEOUT, false);
}
}
@ -4510,11 +4518,6 @@ PostgresMain(const char *dbname, const char *username)
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())
{
@ -4529,11 +4532,6 @@ PostgresMain(const char *dbname, const char *username)
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
{
@ -4586,13 +4584,6 @@ PostgresMain(const char *dbname, const char *username)
enable_timeout_after(IDLE_SESSION_TIMEOUT,
IdleSessionTimeout);
}
/*
* If GUC is changed then it's handled in
* assign_transaction_timeout().
*/
if (TransactionTimeout > 0 && get_timeout_active(TRANSACTION_TIMEOUT))
disable_timeout(TRANSACTION_TIMEOUT, false);
}
/* Report any recently-changed GUC options */

1
src/test/isolation/isolation_schedule
View File

@ -89,7 +89,6 @@ test: sequence-ddl
test: async-notify
test: vacuum-no-cleanup-lock
test: timeouts
test: timeouts-long
test: vacuum-concurrent-drop
test: vacuum-conflict
test: vacuum-skip-locked

35
src/test/isolation/specs/timeouts-long.spec
View File

@ -1,35 +0,0 @@
# 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); }
step s7_check { SELECT count(*) FROM pg_stat_activity WHERE application_name = 'isolation/timeouts/s7'; }
step s8_check { SELECT count(*) FROM pg_stat_activity WHERE application_name = 'isolation/timeouts/s8'; }
# COMMIT AND CHAIN must restart transaction timeout
permutation s7_begin s7_sleep s7_commit_and_chain s7_sleep s7_check s7_abort
# transaction timeout expires in presence of query flow, session s7 FATAL-out
# this relatevely long sleeps are picked to ensure 300ms gap between check and timeouts firing
# expected flow: timeouts is scheduled after s8_begin and fires approximately after checker_sleep (300ms before check)
# possible buggy flow: timeout is schedules after s8_select_1 and fires 300ms after s8_check
# to ensure this 300ms gap we need minimum transaction_timeout of 300ms
permutation s8_begin s8_sleep s8_select_1 s8_check checker_sleep checker_sleep s8_check