On Thu, Aug 20, 2026 at 11:12 AM cca5507 <[email protected]> wrote: > > > 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. > > The max timeout value supported by WaitLatch() is INT_MAX, so I think > it's reasonable to limit the range to int32.
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. -- Regards, Xuneng Zhou HighGo Software Co., Ltd.
