Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry

On Mon, Mar 20, 2023 at 6:58 AM Drouvot, Bertrand
<bertranddrouvot.pg@gmail.com> wrote:
>
> Hi,
>
> On 3/20/23 12:43 AM, Michael Paquier wrote:
> > At the
> > end, documenting both still sounds like the best move to me.
>
> Agree.
>
> Please find attached v1-0001-pg_stat_get_xact_blocks_fetched-and_hit-doc.patch doing so.
>
> I did not put the exact same wording as the one being removed in ddfc2d9, as:
>
> - For pg_stat_get_xact_blocks_hit(), I think it's better to be closer to say the
> pg_statio_all_tables.heap_blks_hit definition.
>
> - For pg_stat_get_xact_blocks_fetched(), I think that using "buffer" is better (than block) as at the
> end it's related to pgstat_count_buffer_read().
>
> At the end there is a choice to be made for both for the wording between block and buffer. Indeed their
> counters get incremented in "buffer" macros while retrieved in those "blocks" functions.
>
> "Buffer" sounds more appropriate to me, so the attached has been done that way.

Apologies as I know this docs update has already been committed, but
buffers fetched and blocks fetched both feel weird to me. If you have a
cache hit, you don't end up really "fetching" anything at all (since
pgstat_count_buffer_read() is called before ReadBuffer_common() and we
don't know if it is a hit or miss yet). And, I would normally associate
fetching with fetching a block into a buffer. It seems like this counter
is really reflecting the number of buffers acquired or used.

tuples_fetched makes more sense because a tuple is "fetched" into a
slot.

This isn't really the fault of this patch since that member was already
called blocks_fetched.

- Melanie