Re: Summary function for pg_buffercache

From: Aleksander Alekseev <aleksander(at)timescale(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Melih Mutlu <m(dot)melihmutlu(at)gmail(dot)com>
Subject: Re: Summary function for pg_buffercache
Date: 2022-09-09 14:36:45
Message-ID: CAJ7c6TOgWZSdf4=Zha9isN35rh0niprtP3Eqmm=u4-M6iYa-Zg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Melih,

> I would appreciate any feedback/comment on this change.

Another benefit of pg_buffercache_summary() you didn't mention is that
it allocates much less memory than pg_buffercache_pages() does.

Here is v3 where I added this to the documentation. The patch didn't
apply to the current master branch with the following error:

```
pg_buffercache_pages.c:286:19: error: no member named 'rlocator' in
'struct buftag'
if (bufHdr->tag.rlocator.relNumber != InvalidOid)
~~~~~~~~~~~ ^
1 error generated.
```

I fixed this too. Additionally, the patch was pgindent'ed and some
typos were fixed.

However I'm afraid you can't examine BufferDesc's without taking
locks. This is explicitly stated in buf_internals.h:

"""
Buffer header lock (BM_LOCKED flag) must be held to EXAMINE or change
TAG, state or wait_backend_pgprocno fields.
"""

Let's consider this code again (this is after my fix):

```
if (RelFileNumberIsValid(BufTagGetRelNumber(bufHdr))) {
/* ... */
}
```

When somebody modifies relNumber concurrently (e.g. calls
ClearBufferTag()) this will cause an undefined behaviour.

I suggest we focus on saving the memory first and then think about the
performance, if necessary.

--
Best regards,
Aleksander Alekseev

Attachment Content-Type Size
v3-0001-Added-pg_buffercache_summary-function.patch application/octet-stream 11.6 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2022-09-09 14:53:47 Re: HOT chain validation in verify_heapam()
Previous Message Tom Lane 2022-09-09 14:31:10 Re: Introduce wait_for_subscription_sync for TAP tests