| From: | Bertrand Drouvot <bertranddrouvot(dot)pg(at)gmail(dot)com> |
|---|---|
| To: | Tatsuya Kawata <kawatatatsuya0913(at)gmail(dot)com> |
| Cc: | 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-22 04:26:42 |
| Message-ID: | aji5gsDcPC3wxPuO@bdtpg |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi Kawata-san,
On Sat, Jun 20, 2026 at 03:44:52PM +0900, Tatsuya Kawata wrote:
> Hi Bertrand-san, Horiguchi-san,
>
> Thanks for confirming the original intent.
>
> Before we conclude, I would like to share a couple of points that
> make me wonder whether changing the type might still be worth
> considering.
>
> 1. pg_stat_lock is new in v19, so the type can still be changed
> before release without any backwards-compatibility cost. This
> makes now a relatively low-risk moment to revisit the choice.
>
> 2. Looking across the other stats views, the "sub-millisecond
> precision is not particularly useful" criterion does not seem to
> be the basis for picking a type in general.
> pg_stat_database.session_time, for example, can accumulate to
> large values for which sub-millisecond precision is also noise,
> yet it uses double precision. From a user's point of view,
> the common pattern across the stats views seems to be
> "measured time columns are double precision", regardless of
> expected magnitude or required precision.
>
> 3. As a minor point, deadlock_timeout is a GUC and can be lowered,
> so under diagnostic configurations sub-millisecond precision
> in wait_time is not entirely hypothetical.
>
> So my point is not that the original bigint choice was wrong, but
> that pg_stat_lock currently differs from the other stats views in
> this respect, and v19 may be a good moment to make it uniform.
I can see your points though I'm not fully convinced yet. OTOH, would that
hurt to be consistent (even if, I think, it does not really provide real
actionable added value).
> If the consensus after considering these points is still that the
> existing bigint type is preferable, I am happy to withdraw and send
> a docs-only patch making the rationale explicit instead.
If we were to update something then I think I'd prefer to change the code.
So, looking at your v1:
=== 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?
and so:
=== 2
- pgstat_count_lock_waits(locallock->tag.lock.locktag_type, msecs);
+ pgstat_count_lock_waits(locallock->tag.lock.locktag_type,
+ (double) msecs + (double) usecs / 1000.0);
would:
- msecs = secs * 1000 + usecs / 1000;
- usecs = usecs % 1000;
/* Increment the lock statistics counters if done waiting. */
if (myWaitStatus == PROC_WAIT_STATUS_OK)
- pgstat_count_lock_waits(locallock->tag.lock.locktag_type, msecs);
+ pgstat_count_lock_waits(locallock->tag.lock.locktag_type, secs * 1000000 + usecs);
+
+ msecs = secs * 1000 + usecs / 1000;
+ usecs = usecs % 1000;
=== 3
- values[i++] = Int64GetDatum(lck_stats->wait_time);
+ values[i++] = Float8GetDatum(lck_stats->wait_time);
Then, what about doing:
values[i++] = Float8GetDatum((double) lck_stats->wait_time / 1000.0);
instead?
=== 4
and instead of:
- PgStat_Counter wait_time; /* time in milliseconds */
+ double wait_time; /* time in milliseconds */
only change the comment here: to microseconds (but keep PgStat_Counter as type).
The idea being to keep the PgStat_Counter type, the long parameter type and
do the conversion at display time.
Regards,
--
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com
| From | Date | Subject | |
|---|---|---|---|
| Previous Message | vignesh C | 2026-06-22 04:03:44 | Re: Proposal: Conflict log history table for Logical Replication |