Date: Tue, 28 Mar 2023 12:36:15 +0900 (JST)
Message-Id: <20230328.123615.1049160234680207474.horikyota.ntt@gmail.com>
To: michael@paquier.xyz
Cc: melanieplageman@gmail.com, bertranddrouvot.pg@gmail.com,
 andres@anarazel.de, pgsql-hackers@lists.postgresql.org
Subject: Re: Reconcile stats in find_tabstat_entry() and get rid of
 PgStat_BackendFunctionEntry
From: Kyotaro Horiguchi <horikyota.ntt@gmail.com>
In-Reply-To: <ZCIa4alHwix6o0if@paquier.xyz>
References: <ZBuEPgVYzigWcldN@paquier.xyz>
	<CAAKRu_aULzTDWqOHvy9bnS9reFs6TUYhdUe8Dcjog2FU+f+KVw@mail.gmail.com>
	<ZCIa4alHwix6o0if@paquier.xyz>
User-Agent: Mew version 6.8 on Emacs 26.1
Mime-Version: 1.0
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Archived-At: <https://www.postgresql.org/message-id/20230328.123615.1049160234680207474.horikyota.ntt%40gmail.com>
Precedence: bulk

At Tue, 28 Mar 2023 07:38:25 +0900, Michael Paquier <michael@paquier.xyz> wrote in 
> On Mon, Mar 27, 2023 at 10:24:46AM -0400, Melanie Plageman wrote:
> > I do like/prefer "block read requests" and
> > "blocks requested found in cache"
> > Though, now I fear my initial complaint may have been a bit pedantic.
> 
> That's fine.  Let's ask for extra opinions, then.
> 
> So, have others an opinion to share here?

I do not have a strong preference for the wording, but consistency is
important.  IMHO simply swapping out a few words won't really improve
things.

I found that commit ddfc2d9a37 removed the descriptions for
pg_stat_get_blocks_fetched and pg_stat_get_blocks_hit. Right before
that commit, monitoring.sgml had these lines:

-     <function>pg_stat_get_blocks_fetched</function> minus
-     <function>pg_stat_get_blocks_hit</function> gives the number of kernel
-     <function>read()</> calls issued for the table, index, or
-     database; the number of actual physical reads is usually
-     lower due to kernel-level buffering.  The <literal>*_blks_read</>
-     statistics columns use this subtraction, i.e., fetched minus hit.

The commit then added the following sentence to the description for
pg_statio_all_tables.heap_blks_read.

>     <entry>Number of disk blocks read from this table.
>     This value can also be returned by directly calling
>     the <function>pg_stat_get_blocks_fetched</function> and
>     <function>pg_stat_get_blocks_hit</function> functions and
>     subtracting the results.</entry>

Later, in 5f2b089387 it twas revised as:
+     <entry>Number of disk blocks read in this database</entry>

This revision lost the explantion regarding the relationship among
fetch, hit and read, as it became hidden in the views' definitions.

As the result, in the current state, it doesn't make sense to just add
a description for pg_stat_get_xact_blocks_fetched().

The confusion stems from the inconsistency between the views and
underlying functions related to block reads and hits. If we add
descriptions for the two functions, we should also explain their
relationship.  Otherwise, it might be better to add the functions
pg_stat_*_blocks_read() instead.

regards.

-- 
Kyotaro Horiguchi
NTT Open Source Software Center