Re: timeout value overflow in wait for lsn

From: Xuneng Zhou <xunengzhou(at)gmail(dot)com>
To: cca5507 <cca5507(at)qq(dot)com>
Cc: pgsql-hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Alexander Korotkov <aekorotkov(at)gmail(dot)com>
Subject: Re: timeout value overflow in wait for lsn
Date: 2026-08-20 01:58:28
Message-ID: CABPTF7UvxPOLPrajRJmKhkfR7ZYJHJGJaJKX7-V8KPViV5RdVA@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Aug 19, 2026 at 11:09 PM Xuneng Zhou <xunengzhou(at)gmail(dot)com> wrote:
>
> Hi ChangAo,
>
> On Wed, Aug 19, 2026 at 9:41 PM cca5507 <cca5507(at)qq(dot)com> wrote:
> >
> > Hi,
> >
> > The valid range of the timeout value in wait for lsn command is int64, which can overflow
> > later in WaitForLSN():
> >
> > ```
> > postgres=# wait for lsn '99/99999999' with (mode 'primary_flush', timeout '10000000000000000ms');
> > ERROR: timed out while waiting for target LSN 99/99999999 to be flushed; current primary_flush LSN 0/017CA348
> > Time: 0.584 ms
> > ```
> >
> > To fix it, change the valid range to int32, just like deadlock_timeout and many other GUCs.
> >
> > Thoughts?
>
> Good catch. I am not sure about the fix. The likely cause of overflow is:
>
> #define TimestampTzPlusMilliseconds(tz, ms) \
> ((tz) + ((ms) * (int64) 1000))
>
> values far greater than int32 could be a problem. But does this
> warrant a truncation to int32? Yeah, from a pragmatic perspective,
> these off-charts values are not expected in practice since the users
> don't have and better not have this amount of patience for latency.
> But truncating it alone like
>
> + if (unlikely(isnan(dval) || !FLOAT8_FITS_IN_INT32(dval)))
>
> seems not adequate to me -- the interface supports int 64, it seems
> not good to accept it first and then reject it loudly later. If this
> change is desired, we might need to change the interface as well.
> Another direction is to prevent the overflow while preserving the
> current value by checking the timeout with
>
> if (pg_mul_s64_overflow(timeout, USECS_PER_MSEC, &timeout_us) ||
> pg_add_s64_overflow(now, timeout_us, &endtime) ||
> !IS_VALID_TIMESTAMP(endtime))

Sleeping with it for a night, the above writing seems to be somewhat
confusing. Here's a version improved by Sol:

Changing the check to FLOAT8_FITS_IN_INT32 would reject larger values.
If we choose that limit, the timeout variable and the WaitForLSN()
argument should also use int so that the interface matches the
accepted range.

Alternatively, we can preserve the int64 interface and check the
deadline calculation in WaitForLSN():

if (pg_mul_s64_overflow(timeout, USECS_PER_MSEC, &timeout_us) ||
pg_add_s64_overflow(now, timeout_us, &endtime) ||
!IS_VALID_TIMESTAMP(endtime))

-----------------

Another option is to reject values greater than or equal with
INT64_MAX/1000, which seems a bit hacky to me.

--
Regards,
Xuneng Zhou
HighGo Software Co., Ltd.

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Chao Li 2026-08-20 02:08:24 Re: Fix signed/unsigned integer handling in pg_restore_relation_stats()
Previous Message Bingshuai Li 2026-08-20 01:40:44 Re: Logical Replication - revisit `is_table_publication` function implementation