Re: Add shared buffer hits to pg_stat_io

Thanks for the review!

On Tue, Feb 28, 2023 at 7:36 AM Drouvot, Bertrand
<bertranddrouvot(dot)pg(at)gmail(dot)com> wrote:
> BufferDesc *
> LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum,
> - bool *foundPtr, IOContext *io_context)
> + bool *foundPtr, IOContext io_context)
> {
> BufferTag newTag; /* identity of requested block */
> LocalBufferLookupEnt *hresult;
> @@ -128,14 +128,6 @@ LocalBufferAlloc(SMgrRelation smgr, ForkNumber forkNum, BlockNumber blockNum,
> hresult = (LocalBufferLookupEnt *)
> hash_search(LocalBufHash, &newTag, HASH_FIND, NULL);
>
> - /*
> - * IO Operations on local buffers are only done in IOCONTEXT_NORMAL. Set
> - * io_context here (instead of after a buffer hit would have returned) for
> - * convenience since we don't have to worry about the overhead of calling
> - * IOContextForStrategy().
> - */
> - *io_context = IOCONTEXT_NORMAL;
>
>
> It looks like that io_context is not used in LocalBufferAlloc() anymore and then can be removed as an argument.

Good catch. Updated patchset attached.

> > While adding this, I noticed that I had made all of the IOOP columns
> > int8 in the view, and I was wondering if this is sufficient for hits (I
> > imagine you could end up with quite a lot of those).
> >
>
> I think that's ok and bigint is what is already used for pg_statio_user_tables.heap_blks_hit for example.

Ah, I was silly and didn't understand that the SQL type int8 is eight
bytes and not 1. That makes a lot of things make more sense :)

https://www.postgresql.org/docs/current/xfunc-c.html#XFUNC-C-TYPE-TABLE

- Melanie