On Sat, Apr 6, 2024 at 7:08 PM Thomas Munro <thomas.mu...@gmail.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
