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

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@gmail.com> wrote:
> On Mon, 17 Feb 2025 at 19:59, Andres Freund <andres@anarazel.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