| From: | Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> |
|---|---|
| To: | cca5507 <cca5507(at)qq(dot)com> |
| Cc: | Xuneng Zhou <xunengzhou(at)gmail(dot)com>, 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-28 07:17:21 |
| Message-ID: | CAD21AoC-rMqs0Wxovh0X1+tkd-SDMTh9kMGovb615OAJ8W02xA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Wed, Aug 19, 2026 at 11:18 PM cca5507 <cca5507(at)qq(dot)com> wrote:
>
> > We have a loop in the wait infra, which means that the waiter could
> > fall asleep several times. Each time takes a INT_MAX maximumly, added
> > up toward a value larger than INT_MAX. That is why I was wondering
> > whether the bug itself warrants a truncation from 64 to 32. If there
> > are user needs like absurdly long timeouts, then we better satisfy
> > them and there're ways to do so. But in my poor imagination, waiting
> > greater than 25 days seems unlikely in reality. So I voted for the
> > limitation of the range.
> >
> > > And I think it's ok to use int64
> > > as the argument in WaitForLSN() because convert int32 to int64 is always
> > > safe.
> >
> > Yeah, it is safe only if we handle the checking/rejection properly.
> > The wait for infra is also used by repack, which uses timeout as zero
> > for an indefinite wait and 100 milliseconds for a finite wait. So it
> > is not vulnerable to edge cases like this. But the infra could have
> > more potential callers in the future, we better not let them bother
> > the extra safety if we can deal with it easily. Sorry if this seems
> > nitpicking to you.
>
> Make sense to me. The v2 patch forgets to update the type of the timeout
> variable in repack worker, fixed in v3.
I found another issue around timeout value handling: if we specify a
timeout in [-0.5, 0.5], the WAIT FOR command waits forever. A negative
timeout in [-0.5, 0) should be rejected. ISTM a timeout in (0, 0.5] is
rounded down to 0, disabling the timeout essentially, which would
surprise users. I think we can either round up timeout in (0, 1] to 1,
or reject sub-millisecond values. I think we can fix both in the same
patch that fixes the overflow issue.
Regards,
--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Dmitry Dolgov | 2026-08-28 07:28:28 | Re: File locks for data directory lockfile in the context of Linux namespaces |
| Previous Message | Shinya Kato | 2026-08-28 07:15:40 | Re: Logical replication row filter loses unchanged toasted columns |