Re: Add shared buffer hits to pg_stat_io

Hi,

On 3/6/23 4:38 PM, Melanie Plageman wrote:
> Thanks for the review!
> 
> On Tue, Feb 28, 2023 at 7:36 AM Drouvot, Bertrand
> <bertranddrouvot.pg@gmail.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.

Thanks for the update!

> 
>>> 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 :)

Oh, I see ;-)

I may give it another review but currently V2 looks good to me.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com