From 9de876a7ef09301bee4ddb4c22251fdd44794e62 Mon Sep 17 00:00:00 2001 From: ChangAo Chen Date: Thu, 10 Sep 2026 11:11:15 -0700 Subject: [PATCH v7] Fix timeout overflow in WAIT FOR LSN. Commit 447aae13b03 accepted TIMEOUT values up to INT64_MAX milliseconds, but computing the deadline multiplies by 1000, which caused an int64 overflow. And the command reported a timeout at once instead of waiting. Parse the value as an int instead, which simplifies the code and is consistent with other timeout values such as the statement_timeout GUC. The specified values are now rounded to the nearest millisecond, so a positive timeout below half a millisecond becomes zero and waits indefinitely. Backpatch to v19, where the TIMEOUT option was introduced. Reported-by: ChangAo Chen Author: ChangAo Chen Reviewed-by: Xuneng Zhou Reviewed-by: Masahiko Sawada Discussion: https://postgr.es/m/tencent_86B83240785807077600A1778566B5C12908@qq.com Backpatch-through: 19 --- doc/src/sgml/ref/wait_for.sgml | 18 ++++++++++---- src/backend/access/transam/xlogwait.c | 2 +- src/backend/commands/repack_worker.c | 4 ++-- src/backend/commands/wait.c | 32 ++++++------------------- src/include/access/xlogwait.h | 2 +- src/test/recovery/t/049_wait_for_lsn.pl | 14 +++++++++++ 6 files changed, 38 insertions(+), 34 deletions(-) diff --git a/doc/src/sgml/ref/wait_for.sgml b/doc/src/sgml/ref/wait_for.sgml index 04ca9400426..36b9ab61976 100644 --- a/doc/src/sgml/ref/wait_for.sgml +++ b/doc/src/sgml/ref/wait_for.sgml @@ -145,13 +145,21 @@ WAIT FOR LSN 'lsn' When specified and timeout is greater than zero, the command waits until lsn is reached or - the specified timeout has elapsed. + the specified timeout has elapsed. A value + of zero (the default) means the command waits indefinitely. - The timeout might be given as integer number of - milliseconds. Also it might be given as string literal with - integer number of milliseconds or a number with unit - (see ). + The timeout is an amount of time in + milliseconds. It may also be specified as a string containing the + numerical value followed by a time unit + (see ). The maximum + value is 2147483647 ms. + + + Fractional values are rounded to the nearest millisecond. Note + that a timeout of half a millisecond or + less therefore rounds down to zero, which 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 b4ba9cfc67b..bf2bc2dca13 100644 --- a/src/backend/commands/repack_worker.c +++ b/src/backend/commands/repack_worker.c @@ -453,7 +453,7 @@ decode_concurrent_changes(LogicalDecodingContext *ctx, if (record == NULL) { - int64 timeout = 0; + int timeout = 0; WaitLSNResult res; /* @@ -472,7 +472,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..bac38d17726 100644 --- a/src/backend/commands/wait.c +++ b/src/backend/commands/wait.c @@ -13,8 +13,6 @@ */ #include "postgres.h" -#include - #include "access/xlog.h" #include "access/xlogrecovery.h" #include "access/xlogwait.h" @@ -35,7 +33,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; @@ -92,7 +90,6 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, { char *timeout_str; const char *hintmsg; - double dval; if (timeout_specified) errorConflictingDefElem(defel, pstate); @@ -100,33 +97,18 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel, timeout_str = defGetString(defel); - if (!parse_real(timeout_str, &dval, GUC_UNIT_MS, &hintmsg)) - { + if (!parse_int(timeout_str, &timeout, GUC_UNIT_MS, &hintmsg)) ereport(ERROR, errcode(ERRCODE_INVALID_PARAMETER_VALUE), errmsg("invalid timeout value: \"%s\"", timeout_str), - hintmsg ? errhint("%s", _(hintmsg)) : 0); - } - - /* - * 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); + hintmsg ? errhint("%s", _(hintmsg)) : 0, + parser_errposition(pstate, defel->location)); - /* Range check */ - if (unlikely(isnan(dval) || !FLOAT8_FITS_IN_INT64(dval))) - ereport(ERROR, - errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE), - errmsg("timeout value is out of range")); - - if (dval < 0) + if (timeout < 0) ereport(ERROR, errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("timeout cannot be negative")); - - timeout = (int64) dval; + errmsg("timeout cannot be negative"), + parser_errposition(pstate, defel->location)); } 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..50c08dfac8f 100644 --- a/src/test/recovery/t/049_wait_for_lsn.pl +++ b/src/test/recovery/t/049_wait_for_lsn.pl @@ -343,6 +343,14 @@ $node_standby->psql( stderr => \$stderr); ok($stderr =~ /timeout cannot be negative/, "get error for negative timeout"); +# Test out of range timeout +$node_standby->psql( + 'postgres', + "WAIT FOR LSN '${test_lsn}' WITH (timeout '2147483648ms');", + stderr => \$stderr); +ok($stderr =~ /invalid timeout value: "2147483648ms"/, + "get error for out of range timeout"); + # Test unknown parameter with WITH clause $node_standby->psql( 'postgres', @@ -407,6 +415,12 @@ $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 WITH clause error case - invalid option $node_standby->psql( 'postgres', -- 2.55.0