Re: pg_stat_io_histogram - Mailing list pgsql-hackers

From Jakub Wartak
Subject Re: pg_stat_io_histogram
Date
Msg-id CAKZiRmyuuToHXESA0pCQ1ZT6_auhAOHzV0Eb+5r2tRaTOtfrFw@mail.gmail.com
Whole thread
In response to Re: pg_stat_io_histogram  (Ants Aasma <ants.aasma@cybertec.at>)
List pgsql-hackers
On Tue, Aug 25, 2026 at 9:40 AM Jakub Wartak <jakub.wartak@enterprisedb.com>
wrote:
>
> On Mon, Aug 10, 2026 at 12:54 PM Jakub Wartak
> <jakub.wartak@enterprisedb.com> wrote:
> [..]
> > > > > At the end of the day, I'll squeze 000[123] into just one, but
wanted
> > > > > to ease the
> > > > > review first a bit. Of course this is material for PG20.
> > > >
> > > > Just noticed it needed a rebase (due to c7cb8e5b73c6;
renumber_oids.pl), so v11
> > > > attached before I forget.
> > >
> > > Just another rebase due to 3b066de6c0a1 (pg_stat_kind_info).
> >
> > Yet another rebase, v13 attached.
>
> Rebased, v14 attached. The only change is that I've removed
> PGSTAT_FILE_FORMAT_ID
> bumps (to potentially reduce number of rebases needed in future).

I've used LLM to further self review and to catch bugs and coding style
issues. Changes to v15 include:
* added missing HAVE__BUILTIN_CLZL in pg_config.h*
* pg_stat_io_histogram_build_tuples() I've changed the fetching OID of
  "int4range" to direct "INT4RANGEOID" to avoid any search_path issues
  (previous versions could be forced to use search_path potenially)
* clarified in documentation note that by enabling track_*io_timing the
  feature is enabled, but effectively only for newly spawned backends
  (the test was already assuming so, but it was just docs missing this)
* fixed lots of typos, comments referencing older name of functions, etc.
* added some safeguards about reading data from PGSTAT file (to avoid loading
  corrupted data)
* extended TAP test to ensure we test / load pgstatdata histogram from disk
  too
* removed some unused C includes
* tuned prorows estimate (from 30 to proper row count 680) in pg_proc.dat to
  avoid misleading optimizer estimates (just in case)
* applied missing pgindents, however now unoptimized path - without
  __builtin_clzll() - in pg_leading_zero_bits64() looks kinda ugly, but it's
  compliant...

What's not changed, but I'm not planning to do so unless somebody objects:

1. PGSTAT_FILE_FORMAT_ID bump / catversion bump / and proper OID assigment
   for the view were not done. I think the committer should do that, right?

2. The bucket labels are mathematically sligthly off. Bucket calculation is in
   nanoseconds "[8192, 16384)", but then we show it as "[8, 16)" as
   microseconds in the view. So bucket edges are kind of very closed
   approximatations, as this was done to avoid using GET_MICROSECONDS as it
   contains division and the top critiera here was to avoid any kind of
   regression at all costs when changing the I/O path.

-J.

Attachment

pgsql-hackers by date:

Previous
From: Vaibhav Dalvi
Date:
Subject: Re: Proposal: INSERT ... BY NAME
Next
From: Etsuro Fujita
Date:
Subject: Re: Several issues with postgres_fdw stats import