On Wed, Aug 19, 2026 at 11:18 PM cca5507 <[email protected]> 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


Reply via email to