Re: Add shared buffer hits to pg_stat_io - Mailing list pgsql-hackers

From Drouvot, Bertrand
Subject Re: Add shared buffer hits to pg_stat_io
Date
Msg-id a713dcf3-19df-90c6-db73-24c06a09fb64@gmail.com
Whole thread Raw
In response to Add shared buffer hits to pg_stat_io  (Melanie Plageman <melanieplageman@gmail.com>)
Responses Re: Add shared buffer hits to pg_stat_io
List pgsql-hackers
Hi,

On 2/25/23 9:16 PM, Melanie Plageman wrote:
> Hi,
> 
> As suggested in [1], the attached patch adds shared buffer hits to
> pg_stat_io.
> 

Thanks for the patch!

  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.

> 
> I am looking for input as to the order of this column in the view. I
> think it should go after op_bytes since it is not relevant for
> non-block-oriented IO. 

Agree.

> However, I'm not sure what the order of hits,
> evictions, and reuses should be (all have to do with buffers).
> 

I'm not sure there is a strong "correct" ordering but the proposed one looks natural to me.

> 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.

Regards,

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



pgsql-hackers by date:

Previous
From: Bharath Rupireddy
Date:
Subject: Re: Allow logical replication to copy tables in binary format
Next
From: James Coleman
Date:
Subject: Re: pg_rewind: warn when checkpoint hasn't happened after promotion