mirror of
https://github.com/postgres/postgres.git
synced 2026-10-23 13:29:09 +03:00
Fix timeout overflow in WAIT FOR LSN
Commit 447aae13b0 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 <cca5507@qq.com>
Author: ChangAo Chen <cca5507@qq.com>
Reviewed-by: Xuneng Zhou <xunengzhou@gmail.com>
Reviewed-by: Masahiko Sawada <sawada.mshk@gmail.com>
Discussion: https://postgr.es/m/tencent_86B83240785807077600A1778566B5C12908@qq.com
Backpatch-through: 19
This commit is contained in:
@@ -145,13 +145,21 @@ WAIT FOR LSN '<replaceable class="parameter">lsn</replaceable>'
|
||||
<para>
|
||||
When specified and <parameter>timeout</parameter> is greater than zero,
|
||||
the command waits until <parameter>lsn</parameter> is reached or
|
||||
the specified <parameter>timeout</parameter> has elapsed.
|
||||
the specified <parameter>timeout</parameter> has elapsed. A value
|
||||
of zero (the default) means the command waits indefinitely.
|
||||
</para>
|
||||
<para>
|
||||
The <parameter>timeout</parameter> 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 <xref linkend="config-setting-names-values"/>).
|
||||
The <parameter>timeout</parameter> 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 <xref linkend="config-setting-names-values"/>). The maximum
|
||||
value is <literal>2147483647 ms</literal>.
|
||||
</para>
|
||||
<para>
|
||||
Fractional values are rounded to the nearest millisecond. Note
|
||||
that a <parameter>timeout</parameter> of half a millisecond or
|
||||
less therefore rounds down to zero, which means waiting
|
||||
indefinitely.
|
||||
</para>
|
||||
</listitem>
|
||||
</varlistentry>
|
||||
|
||||
@@ -440,7 +440,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;
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -13,8 +13,6 @@
|
||||
*/
|
||||
#include "postgres.h"
|
||||
|
||||
#include <math.h>
|
||||
|
||||
#include "access/xlog.h"
|
||||
#include "access/xlogrecovery.h"
|
||||
#include "access/xlogwait.h"
|
||||
@@ -37,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;
|
||||
@@ -94,7 +92,6 @@ ExecWaitStmt(ParseState *pstate, WaitStmt *stmt, bool isTopLevel,
|
||||
{
|
||||
char *timeout_str;
|
||||
const char *hintmsg;
|
||||
double dval;
|
||||
|
||||
if (timeout_specified)
|
||||
errorConflictingDefElem(defel, pstate);
|
||||
@@ -102,33 +99,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);
|
||||
}
|
||||
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);
|
||||
|
||||
/* 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)
|
||||
{
|
||||
|
||||
@@ -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 */
|
||||
|
||||
@@ -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',
|
||||
|
||||
Reference in New Issue
Block a user