From 5a14d4d4f4885b60051693dd7af93c80268520e6 Mon Sep 17 00:00:00 2001 From: ChangAo Chen Date: Fri, 28 Aug 2026 17:01:23 +0800 Subject: [PATCH v5] Fix WAIT FOR LSN timeout handling. Limit WAIT FOR LSN timeouts to the int range and update the WaitForLSN() interface and its callers accordingly. This prevents large timeout values from overflowing while calculating the deadline. Check for negative values before rounding, so negative sub-millisecond timeouts cannot be rounded to zero and interpreted as an indefinite wait. Likewise, round positive values smaller than one millisecond up to one millisecond, since zero means waiting indefinitely. --- doc/src/sgml/ref/wait_for.sgml | 2 ++ src/backend/access/transam/xlogwait.c | 2 +- src/backend/commands/repack_worker.c | 4 +-- src/backend/commands/wait.c | 46 ++++++++++++++++--------- src/include/access/xlogwait.h | 2 +- src/test/recovery/t/049_wait_for_lsn.pl | 27 +++++++++++++++ 6 files changed, 63 insertions(+), 20 deletions(-) diff --git a/doc/src/sgml/ref/wait_for.sgml b/doc/src/sgml/ref/wait_for.sgml index 01dc2a84a1a..9177d29c75a 100644 --- a/doc/src/sgml/ref/wait_for.sgml +++ b/doc/src/sgml/ref/wait_for.sgml @@ -152,6 +152,8 @@ WAIT FOR LSN 'lsn' milliseconds. Also it might be given as string literal with integer number of milliseconds or a number with unit (see ). + The valid range is from 0 to 2,147,483,647 milliseconds, inclusive. + A value of zero means waiting indefinitely. diff --git a/src/backend/access/transam/xlogwait.c b/src/backend/access/transam/xlogwait.c index eee90e7f626..2ea8c24a74f 100644 --- a/src/backend/access/transam/xlogwait.c +++ b/src/backend/access/transam/xlogwait.c @@ -437,7 +437,7 @@ WaitLSNTypeRequiresRecovery(WaitLSNType t) * or replica got promoted before the target LSN reached. */ WaitLSNResult -WaitForLSN(WaitLSNType lsnType, XLogRecPtr targetLSN, int64 timeout) +WaitForLSN(WaitLSNType lsnType, XLogRecPtr targetLSN, int timeout) { XLogRecPtr currentLSN; WaitLSNProcInfo *procInfo; diff --git a/src/backend/commands/repack_worker.c b/src/backend/commands/repack_worker.c index af7e2a94764..a9870d9c8f2 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -447,7 +447,7 @@ decode_concurrent_changes(LogicalDecodingContext *ctx, if (record == NULL) { - int64 timeout = 0; + int timeout = 0; WaitLSNResult res; /* @@ -466,7 +466,7 @@ decode_concurrent_changes(LogicalDecodingContext *ctx, * should already have been flushed to disk. */ if (!XLogRecPtrIsValid(lsn_upto)) - timeout = 100L; + timeout = 100; res = WaitForLSN(WAIT_LSN_TYPE_PRIMARY_FLUSH, ctx->reader->EndRecPtr + 1, timeout); diff --git a/src/backend/commands/wait.c b/src/backend/commands/wait.c index 9ba4c75021e..36b83c62250 100644 --- a/src/backend/commands/wait.c +++ b/src/backend/commands/wait.c @@ -35,7 +35,7 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, DestReceiver *dest) { XLogRecPtr lsn; - int64 timeout = 0; + int timeout = 0; WaitLSNResult waitLSNResult; WaitLSNType lsnType = WAIT_LSN_TYPE_STANDBY_REPLAY; /* default */ bool throw = true; @@ -105,28 +105,42 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, ereport(ERROR, errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid timeout value: \"%s\"", timeout_str), - hintmsg ? errhint("%s", _(hintmsg)) : 0); + hintmsg ? errhint("%s", _(hintmsg)) : 0, + parser_errposition(pstate, defel->location)); } - /* - * Get rid of any fractional part in the input. This is so we - * don't fail on just-out-of-range values that would round into - * range. - */ - dval = rint(dval); + if (dval < 0.0) + ereport(ERROR, + errcode(ERRCODE_INVALID_PARAMETER_VALUE), + errmsg("timeout cannot be negative"), + parser_errposition(pstate, defel->location)); + + if (dval > 0.0 && dval < 1.0) + { + /* + * Round values in (0, 1) up to 1 to avoid treating them as + * zero, which means waiting indefinitely. + */ + dval = 1.0; + } + else + { + /* + * Get rid of any fractional part in the input. This is so we + * don't fail on just-out-of-range values that would round + * into range. + */ + dval = rint(dval); + } /* Range check */ - if (unlikely(isnan(dval) || !FLOAT8_FITS_IN_INT64(dval))) + if (unlikely(isnan(dval) || !FLOAT8_FITS_IN_INT32(dval))) ereport(ERROR, errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("timeout value is out of range")); - - if (dval < 0) - ereport(ERROR, - errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("timeout cannot be negative")); + errmsg("timeout value is out of range"), + parser_errposition(pstate, defel->location)); - timeout = (int64) dval; + timeout = (int) dval; } else if (strcmp(defel->defname, "no_throw") == 0) { diff --git a/src/include/access/xlogwait.h b/src/include/access/xlogwait.h index 07157f220ea..2bf0263e9e2 100644 --- a/src/include/access/xlogwait.h +++ b/src/include/access/xlogwait.h @@ -104,6 +104,6 @@ extern XLogRecPtr GetCurrentLSNForWaitType(WaitLSNType lsnType); extern void WaitLSNWakeup(WaitLSNType lsnType, XLogRecPtr currentLSN); extern void WaitLSNCleanup(void); extern WaitLSNResult WaitForLSN(WaitLSNType lsnType, XLogRecPtr targetLSN, - int64 timeout); + int timeout); #endif /* XLOG_WAIT_H */ diff --git a/src/test/recovery/t/049_wait_for_lsn.pl b/src/test/recovery/t/049_wait_for_lsn.pl index cb7d4d461de..758a2e20fb6 100644 --- a/src/test/recovery/t/049_wait_for_lsn.pl +++ b/src/test/recovery/t/049_wait_for_lsn.pl @@ -343,6 +343,20 @@ $node_standby->psql( stderr => \$stderr); ok($stderr =~ /timeout cannot be negative/, "get error for negative timeout"); +# Test negative timeout in [-0.5, 0) +$node_standby->psql( + 'postgres', + "WAIT FOR LSN '${test_lsn}' WITH (timeout '-0.1ms');", + stderr => \$stderr); +ok($stderr =~ /timeout cannot be negative/, "get error for negative timeout"); + +# Test timeout out of range +$node_standby->psql( + 'postgres', + "WAIT FOR LSN '${test_lsn}' WITH (timeout '2147483648ms');", + stderr => \$stderr); +ok($stderr =~ /timeout value is out of range/, "get error for out of range timeout"); + # Test unknown parameter with WITH clause $node_standby->psql( 'postgres', @@ -407,6 +421,19 @@ $output = $node_standby->safe_psql( ok($output eq "timeout", "WAIT FOR WITH clause returns correct timeout status"); +# Test maximum timeout +$output = $node_standby->safe_psql( + 'postgres', qq[ + WAIT FOR LSN '${lsn2}' WITH (timeout '2147483647ms', no_throw);]); +ok($output eq "success", "maximum timeout value is accepted"); + +# Test sub-millisecond timeout doesn't become an indefinite wait +$output = $node_standby->safe_psql( + 'postgres', qq[ + SET statement_timeout = '1s'; + WAIT FOR LSN '${lsn3}' WITH (timeout '0.1ms', no_throw);]); +ok($output eq "timeout", "sub-millisecond timeout works correctly"); + # Test WITH clause error case - invalid option $node_standby->psql( 'postgres', -- 2.53.0