Re: Extension Enhancement: Buffer Invalidation in pg_buffercache

On Sat, Apr 6, 2024 at 7:08 PM Thomas Munro <thomas(dot)munro(at)gmail(dot)com> wrote:
>
> On second thoughts, I think the original "invalidate" terminology was
> fine, no need to invent a new term.
>
> I thought of a better name for the bufmgr.c function though:
> InvalidateUnpinnedBuffer(). That name seemed better to me after I
> festooned it with warnings about why exactly it's inherently racy and
> only for testing use.
>
> I suppose someone could propose an additional function
> pg_buffercache_invalidate(db, tbspc, rel, fork, blocknum) that would
> be slightly better in the sense that it couldn't accidentally evict
> some innocent block that happened to replace the real target just
> before it runs, but I don't think it matters much for this purpose and
> it would still be racy on return (vacuum decides to load your block
> back in) so I don't think it's worth bothering with.
>
> So this is the version I plan to commit.

I've reviewed v6. I think you should mention in the docs that it only
works for shared buffers -- so specifically not buffers containing
blocks of temp tables.

In the function pg_buffercache_invalidate(), why not use the
BufferIsValid() function?

- if (buf < 1 || buf > NBuffers)
+ if (!BufferIsValid(buf) || buf > NBuffers)

I thought the below would be more clear for the comment above
InvalidateUnpinnedBuffer().

- * Returns true if the buffer was valid and it has now been made invalid.
- * Returns false if the wasn't valid, or it couldn't be evicted due to a pin,
- * or if the buffer becomes dirty again while we're trying to write it out.
+ * Returns true if the buffer was valid and has now been made invalid. Returns
+ * false if it wasn't valid, if it couldn't be evicted due to a pin, or if the
+ * buffer becomes dirty again while we're trying to write it out.

Some of that probably applies for the docs too (i.e. you have some
similar wording in the docs). There is actually one typo in your
version, so even if you don't adopt my suggestion, you should fix that
typo.

I didn't notice anything else out of place. I tried it and it worked
as expected. I'm excited to have this feature!

I didn't read through this whole thread, but was there any talk of
adding other functions to let me invalidate a bunch of buffers at once
or even some options -- like invalidate every 3rd buffer or whatever?
(Not the concern of this patch, but just wondering because that would
be a useful future enhancement IMO).

- Melanie