| From: | Michael Paquier <michael(at)paquier(dot)xyz> |
|---|---|
| To: | Tatsuya Kawata <kawatatatsuya0913(at)gmail(dot)com> |
| Cc: | Bertrand Drouvot <bertranddrouvot(dot)pg(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 02:11:06 |
| Message-ID: | akHUOpGGdt3_28BP@paquier.xyz |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Sun, Jun 28, 2026 at 02:50:38AM +0900, Tatsuya Kawata wrote:
> Thank you for your review!
Catching up with the business happening here..
I'm actually convinced by this patch, based on the point that
pg_stat_lock is the only stats-related view that does not use double
precision for the report. All the others use float8. Let's adjust it
now as it is not released yet.
>> === 1
>>
>> -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).
>> 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.
>> === 4
>>
>> 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?
--
Michael
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Haibo Yan | 2026-06-29 02:20:20 | Re: Optimize UUID parse using SIMD |
| Previous Message | Thomas Munro | 2026-06-29 01:54:04 | Re: Heads Up: cirrus-ci is shutting down June 1st |