Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
From: Bertrand Drouvot <bertranddrouvot.pg@gmail.com>
To: Michael Paquier <michael@paquier.xyz>
Cc: Tatsuya Kawata <kawatatatsuya0913@gmail.com>, Kyotaro Horiguchi <horikyota.ntt@gmail.com>, pgsql-hackers@lists.postgresql.org
Date: 2026-06-29T04:27:55Z
Lists: pgsql-hackers
Hi,
On Mon, Jun 29, 2026 at 11:11:06AM +0900, Michael Paquier wrote:
> On Sun, Jun 28, 2026 at 02:50:38AM +0900, Tatsuya Kawata wrote:
> > Thank you for your review!
>
> >> -pgstat_count_lock_waits(uint8 locktag_type, long msecs)
> >> +pgstat_count_lock_waits(uint8 locktag_type, double msecs)
> >>
> >> What about keeping the long and rename to usecs?
> >
> > Agreed. Fixed.
>
> Relying on long here is not reliable on WIN32, where the value could
> overflow for long wait times (where sizeof(long) == 4). I'd suggest a
> int64 or a uint64 instead for the API to be able to store wait times
> in usecs longer than the overflow threshold. That was another design
> oversight. My oversight here (aka I want to abolish completely the
> use of long in the core code; it leads to insanity).
That makes sense and the lock time that would produce the overflow is realistic
(about 35 minutes) if my math is correct.
> >> values[i++] = Float8GetDatum((double) lck_stats->wait_time / 1000.0);
> >>
> >> instead?
> >
> > Agreed. Fixed.
>
> We have a pg_stat_us_to_ms() that can do this job as well for the
> values displayed. Why duplicate this code? This conversion routine
> is used by pg_stat_io.
Right. That said I just observed that it can produces things like:
postgres=# select read_time from pg_stat_io where read_time > 0;
read_time
---------------------
2.2640000000000002
0.08700000000000001
Maybe it should be changed to?
return (double) val_ms / 1000.0;
instead of:
return val_ms * (double) 0.001;
> >>
> >> The idea being to keep the PgStat_Counter type, the long parameter type
> > and
> >> do the conversion at display time.
> >
> > Agreed. Fixed.
>
> - PgStat_Counter wait_time; /* time in milliseconds */
> + PgStat_Counter wait_time; /* time in microseconds */
>
> The counter is stored in usecs, displayed in msecs. Documenting it as
> stored in msecs is incorrect, no?
Yeah, so this change.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
Commits
-
Change stat_lock.wait_time to double precision
- ff6f6e0470ec 19 (unreleased) landed
- c776550e4662 master landed