Re: [PATCH] Change wait_time column of pg_stat_lock to double precision

From: Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Tatsuya Kawata <kawatatatsuya0913(at)gmail(dot)com>, Kyotaro Horiguchi <horikyota(dot)ntt(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Date: 2026-06-29 04:27:55
Message-ID: akH0SxXlXPNjD+R5@bdtpg
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Smith 2026-06-29 04:30:22 Re: DOCS - "Get Object DDL Functions" table improvements
Previous Message Amit Kapila 2026-06-29 04:27:54 Re: Bug in ALTER SUBSCRIPTION ... SERVER / ... CONNECTION with broken old server