On Mon, Mar 20, 2023 at 6:58 AM Drouvot, Bertrand <bertranddrouvot...@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