Re: Add pg_buffercache_evict_all() and pg_buffercache_mark_dirty[_all]() functions

From: Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>
To: Andres Freund <andres(at)anarazel(dot)de>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Add pg_buffercache_evict_all() and pg_buffercache_mark_dirty[_all]() functions
Date: 2025-03-14 13:25:00
Message-ID: CAN55FZ2mNTVK81jqsy2NXg9k_ex_LrdU6JmL1wvsW9-GPy9XuA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

Here is the v2. Changes prior to v1 are:

- pg_buffercache_evict_relation() function is introduced. Which takes
a relation as an argument and evicts all shared buffers in the
relation.
- It is mentioned in the docs that the #buffers_flushed in the
pg_buffercache_evict_relation() and pg_buffercache_evict_all()
functions do not necessarily mean these buffers are flushed by these
functions.

Remaining questions from the v1:

On Wed, 12 Mar 2025 at 19:15, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com> wrote:
> On Mon, 17 Feb 2025 at 19:59, Andres Freund <andres(at)anarazel(dot)de> wrote:
> > On 2024-12-25 15:57:34 +0300, Nazir Bilal Yavuz wrote:
> > > + */
> > > +Datum
> > > +pg_buffercache_evict_all(PG_FUNCTION_ARGS)
> > > +{
> > > + Datum result;
> > > + TupleDesc tupledesc;
> > > + HeapTuple tuple;
> > > + Datum values[NUM_BUFFERCACHE_EVICT_ALL_ELEM];
> > > + bool nulls[NUM_BUFFERCACHE_EVICT_ALL_ELEM] = {0};
> > > +
> > > + int32 buffers_evicted = 0;
> > > + int32 buffers_flushed = 0;
> > > + bool flushed;
> > > +
> > > + if (get_call_result_type(fcinfo, NULL, &tupledesc) != TYPEFUNC_COMPOSITE)
> > > + elog(ERROR, "return type must be a row type");
> > > +
> > > + if (!superuser())
> > > + ereport(ERROR,
> > > + (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
> > > + errmsg("must be superuser to use pg_buffercache_evict_all function")));
> > > +
> > > + for (int buf = 1; buf < NBuffers; buf++)
> > > + {
> > > + if (EvictUnpinnedBuffer(buf, &flushed))
> > > + buffers_evicted++;
> > > + if (flushed)
> > > + buffers_flushed++;
> >
> > I'd probably add a pre-check for the buffer not being in use. We don't need an
> > external function call, with an unconditional LockBufHdr() inside, for that
> > case.
>
> I did not understand why we would need this. Does not
> EvictUnpinnedBuffer() already check that the buffer is not in use?

> > > +/*
> > > + * MarkUnpinnedBufferDirty
> > > + *
> > > + * This function is intended for testing/development use only!
> > > + *
> > > + * To succeed, the buffer must not be pinned on entry, so if the caller had a
> > > + * particular block in mind, it might already have been replaced by some other
> > > + * block by the time this function runs. It's also unpinned on return, so the
> > > + * buffer might be occupied and flushed by the time control is returned. This
> > > + * inherent raciness without other interlocking makes the function unsuitable
> > > + * for non-testing usage.
> > > + *
> > > + * Returns true if the buffer was not dirty and it has now been marked as
> > > + * dirty. Returns false if it wasn't valid, if it couldn't be marked as dirty
> > > + * due to a pin, or if the buffer was already dirty.
> > > + */
> >
> > Hm. One potentially problematic thing with this is that it can pin and dirty a
> > buffer even while its relation is locked exclusively. Which i think some code
> > doesn't expect. Not sure if there's a way around that :(
>
> I see, I did not think about that. Since this function can be used
> only by superusers, that problem might not be a big issue. What do you
> think?

--
Regards,
Nazir Bilal Yavuz
Microsoft

Attachment Content-Type Size
v2-0001-Add-pg_buffercache_evict_-relation-all-functions-.patch text/x-patch 13.1 KB
v2-0002-Add-pg_buffercache_mark_dirty-_all-functions-for-.patch text/x-patch 8.7 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2025-03-14 13:38:06 Re: Changing the state of data checksums in a running cluster
Previous Message Greg Sabino Mullane 2025-03-14 13:11:15 Re: Allow default \watch interval in psql to be configured