Thread
Commits
-
Change stat_lock.wait_time to double precision
- ff6f6e0470ec 19 (unreleased) landed
- c776550e4662 master landed
-
[PATCH] Change wait_time column of pg_stat_lock to double precision
Tatsuya Kawata <kawatatatsuya0913@gmail.com> — 2026-06-15T06:54:24Z
Hi, While looking at the lock-related code, I noticed that pg_stat_lock is the only statistics view whose timing column (wait_time) uses bigint. Every other statistics view uses double precision for measured-time columns. I do not see a reason for pg_stat_lock to differ. The attached patch changes the column to double precision. Regards, Tatsuya Kawata
-
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Kyotaro Horiguchi <horikyota.ntt@gmail.com> — 2026-06-15T07:39:22Z
Hello. At Mon, 15 Jun 2026 15:54:24 +0900, Tatsuya Kawata <kawatatatsuya0913@gmail.com> wrote in > While looking at the lock-related code, I noticed that pg_stat_lock > is the only statistics view whose timing column (wait_time) uses > bigint. Every other statistics view uses double precision for > measured-time columns. I do not see a reason for pg_stat_lock to > differ. It seems to me that this was intentional. As described in the documentation, since wait_time is only accumulated for waits longer than deadlock_timeout, sub-millisecond precision was probably not considered particularly useful. Regards, -- Kyotaro Horiguchi NTT Open Source Software Center
-
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Tatsuya Kawata <kawatatatsuya0913@gmail.com> — 2026-06-15T13:12:08Z
Hi Horiguchi-san, Thanks for the review! > since wait_time is only accumulated for waits longer > than deadlock_timeout, sub-millisecond precision was probably not > considered particularly useful. You are right that, given the deadlock_timeout floor (which is a GUC, but defaults to one second), each individual wait is already on the order of one second, so the sub-millisecond remainder is at most about 0.1% of the value being recorded. The precision argument is therefore weak on its own, and I do not want to insist on it. The primary motivation I had in mind is consistency across the statistics views: pg_stat_lock is the only one whose timing column is not double precision. Even if the bigint choice was deliberate, the view ends up breaking the pattern. If we decide to keep bigint, I think it would be worth documenting the reason explicitly (e.g. that wait_time is only accumulated for waits longer than deadlock_timeout, hence sub-millisecond precision is not needed). Thoughts? Regards, Tatsuya Kawata
-
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-06-15T13:54:54Z
Hi, On Mon, Jun 15, 2026 at 04:39:22PM +0900, Kyotaro Horiguchi wrote: > Hello. > > At Mon, 15 Jun 2026 15:54:24 +0900, Tatsuya Kawata <kawatatatsuya0913@gmail.com> wrote in > > While looking at the lock-related code, I noticed that pg_stat_lock > > is the only statistics view whose timing column (wait_time) uses > > bigint. Every other statistics view uses double precision for > > measured-time columns. I do not see a reason for pg_stat_lock to > > differ. > > It seems to me that this was intentional. As described in the > documentation, since wait_time is only accumulated for waits longer > than deadlock_timeout, sub-millisecond precision was probably not > considered particularly useful. Yeah that was intentional (and for the reason you described above). Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
-
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Tatsuya Kawata <kawatatatsuya0913@gmail.com> — 2026-06-20T06:44:52Z
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. 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. Regards, Tatsuya Kawata
-
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-06-22T04:26:42Z
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 -
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Tatsuya Kawata <kawatatatsuya0913@gmail.com> — 2026-06-27T17:50:38Z
Hi Bertrand-san, Thank you for your review! > === 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. > === 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; Agreed. Fixed. > === 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? Agreed. Fixed. > === 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. Agreed. Fixed. v2 attached. Regards, Tatsuya Kawata
-
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Michael Paquier <michael@paquier.xyz> — 2026-06-29T02:11:06Z
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
-
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-06-29T04:27:55Z
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 -
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Michael Paquier <michael@paquier.xyz> — 2026-06-29T04:52:54Z
On Mon, Jun 29, 2026 at 04:27:55AM +0000, Bertrand Drouvot wrote: > postgres=# select read_time from pg_stat_io where read_time > 0; > read_time > --------------------- > 2.2640000000000002 > 0.08700000000000001 > > Maybe it should be changed to? pg_stat_io with track_io_timing enabled shows the same thing. FWIW, that does not bother me much. It's just more important to me to apply the same formulas across the board. -- Michael
-
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-06-29T04:56:11Z
Hi, On Mon, Jun 29, 2026 at 01:52:54PM +0900, Michael Paquier wrote: > On Mon, Jun 29, 2026 at 04:27:55AM +0000, Bertrand Drouvot wrote: > > postgres=# select read_time from pg_stat_io where read_time > 0; > > read_time > > --------------------- > > 2.2640000000000002 > > 0.08700000000000001 > > > > Maybe it should be changed to? > > pg_stat_io with track_io_timing enabled shows the same thing. Yeah, I agree, my query above is querying pg_stat_io. I agree that it makes sense to have both using pg_stat_us_to_ms(), I'm just wondering if pg_stat_us_to_ms() should be changed as I did propose. Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
-
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Bertrand Drouvot <bertranddrouvot.pg@gmail.com> — 2026-06-29T07:06:36Z
On Mon, Jun 29, 2026 at 04:56:11AM +0000, Bertrand Drouvot wrote: > Hi, > > On Mon, Jun 29, 2026 at 01:52:54PM +0900, Michael Paquier wrote: > > On Mon, Jun 29, 2026 at 04:27:55AM +0000, Bertrand Drouvot wrote: > > > postgres=# select read_time from pg_stat_io where read_time > 0; > > > read_time > > > --------------------- > > > 2.2640000000000002 > > > 0.08700000000000001 > > > > > > Maybe it should be changed to? > > > > pg_stat_io with track_io_timing enabled shows the same thing. > > Yeah, I agree, my query above is querying pg_stat_io. I agree that it makes > sense to have both using pg_stat_us_to_ms(), I'm just wondering if pg_stat_us_to_ms() > should be changed as I did propose. FWIW, I just created a dedicated thread [1] for this (to not deviate from the original purpose of this one). [1]: https://postgr.es/m/akIYkMK4bHe9qX/N%40bdtpg Regards, -- Bertrand Drouvot PostgreSQL Contributors Team RDS Open Source Databases Amazon Web Services: https://aws.amazon.com
-
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Michael Paquier <michael@paquier.xyz> — 2026-06-30T03:48:50Z
On Mon, Jun 29, 2026 at 04:27:55AM +0000, Bertrand Drouvot wrote: > Hi, > > On Mon, Jun 29, 2026 at 11:11:06AM +0900, Michael Paquier wrote: >> 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. Dealing with the "long" change, I have arrived at the conclusion that using int64 or even uint64 felt stupid because we already use a cast to PgStat_Counter in pgstat_count_lock_waits(), and some other stats just use PgStat_Counter when reporting usec times. So I have just switched the code to use PgStat_Counter. >> The counter is stored in usecs, displayed in msecs. Documenting it as >> stored in msecs is incorrect, no? > > Yeah, so this change. I need to re-learn how to read. And backpatched down to v19, catversion bump oblige. -- Michael
-
Re: [PATCH] Change wait_time column of pg_stat_lock to double precision
Tatsuya Kawata <kawatatatsuya0913@gmail.com> — 2026-07-01T15:28:16Z
Hi Michael-san, Sorry for the late reply. > And backpatched down to v19, catversion bump oblige. > So I have just switched the code to use PgStat_Counter. Thanks for picking this up and committing it, and also for catching the long overflow issue on Win32 along the way. Thanks also to Bertrand-san and Horiguchi-san for the review and discussion. Regards, Tatsuya Kawata