Re: Extension Enhancement: Buffer Invalidation in pg_buffercache

Hi,

I wanted this feature a couple times before...

On 2023-07-03 13:56:29 +0530, Palak Chaturvedi wrote:
> +PG_FUNCTION_INFO_V1(pg_buffercache_invalidate);
> +Datum
> +pg_buffercache_invalidate(PG_FUNCTION_ARGS)

I don't think "invalidating" is the right terminology. Note that we already
have InvalidateBuffer() - but it's something we can't allow users to do, as it
throws away dirty buffer contents (it's used for things like dropping a
table).

How about using "discarding" for this functionality?

Using the buffer ID as the identifier doesn't seem great, because what that
buffer is used for, could have changed since the buffer ID has been acquired
(via the pg_buffercache view presumably)?

My suspicion is that the usual usecase for this would be to drop all buffers
that can be dropped?

> + if (bufnum < 0 || bufnum > NBuffers)
> + {
> + ereport(ERROR,
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("buffernum is not valid")));
> +
> + }
> +
> + result = TryInvalidateBuffer(bufnum);
> + PG_RETURN_BOOL(result);
> +}

I think this should be restricted to superuser by default (by revoking
permissions from PUBLIC). We allow normal users to use pg_prewarm(...), true -
but we perform an ACL check on the relation, so it can only be used for
relations you have access too. This function could be used to affect
performance of other users quite substantially.

> +/*
> +Try Invalidating a buffer using bufnum.
> +If the buffer is invalid, the function returns false.
> +The function checks for dirty buffer and flushes the dirty buffer before invalidating.
> +If the buffer is still dirty it returns false.
> +*/
> +bool
> +TryInvalidateBuffer(Buffer bufnum)
> +{
> + BufferDesc *bufHdr = GetBufferDescriptor(bufnum - 1);
> + uint32 buf_state;
> +
> + ReservePrivateRefCountEntry();
> +
> + buf_state = LockBufHdr(bufHdr);
> + if ((buf_state & BM_VALID) == BM_VALID)
> + {
> + /*
> + * The buffer is pinned therefore cannot invalidate.
> + */
> + if (BUF_STATE_GET_REFCOUNT(buf_state) > 0)
> + {
> + UnlockBufHdr(bufHdr, buf_state);
> + return false;
> + }
> + if ((buf_state & BM_DIRTY) == BM_DIRTY)
> + {
> + /*
> + * Try once to flush the dirty buffer.
> + */
> + PinBuffer_Locked(bufHdr);
> + LWLockAcquire(BufferDescriptorGetContentLock(bufHdr), LW_SHARED);
> + FlushBuffer(bufHdr, NULL, IOOBJECT_RELATION, IOCONTEXT_NORMAL);
> + LWLockRelease(BufferDescriptorGetContentLock(bufHdr));
> + UnpinBuffer(bufHdr);
> + buf_state = LockBufHdr(bufHdr);
> + if (BUF_STATE_GET_REFCOUNT(buf_state) > 0)
> + {
> + UnlockBufHdr(bufHdr, buf_state);
> + return false;
> + }
> +
> + /*
> + * If its dirty again or not valid anymore give up.
> + */
> +
> + if ((buf_state & (BM_DIRTY | BM_VALID)) != (BM_VALID))
> + {
> + UnlockBufHdr(bufHdr, buf_state);
> + return false;
> + }
> +
> + }
> +
> + InvalidateBuffer(bufHdr);

I'm wary of using InvalidateBuffer() here, it's typically used for different
purposes, including throwing valid contents away. That seems a bit scary.

I think you should be able to just use InvalidateVictimBuffer()?

Greetings,

Andres Freund