Hi,
On 2023-06-30 14:13:11 +1200, Thomas Munro wrote:
> > I do wonder if we should have an unlocked pre-check for a) the buffer being
> > valid and b) BufferTagsEqual() matching. With such a pre-check the race for
> > increasing the usage count of the wrong buffer is quite small, without the
> > pre-check it doesn't seem that small anymore.
>
> Yeah, that makes sense. Done in this version.
I'm planning to commit 0001 soon, unless you'd like to do the honors - I would
break it with some upcoming patches, and it's a good improvement. Those
patches also will PinBuffer_Locked() a bit slower, i.e. it'd be good to avoid
using it in ReadRecentBuffer() for that reason alone.
> From d5b9f345961e2adb31213c645e40037f15ba6a83 Mon Sep 17 00:00:00 2001 From:
> Thomas Munro <thomas.munro@gmail.com> Date: Thu, 29 Jun 2023 10:52:56 +1200
> Subject: [PATCH v2 1/2] Improve ReadRecentBuffer() scalability.
>
> While testing a new potential use for ReadRecentBuffer(), Andres
> reported that it scales badly when called concurrently for the same
> buffer by many backends. Instead of a naive (but wrong) coding with
> PinBuffer(), it used the spinlock, so that it could be careful to pin
> only if the buffer was valid and holding the expected block, to avoid
> breaking invariants in eg GetVictimBuffer(). Unfortunately that made it
> less scalable than PinBuffer(), which uses compare-exchange instead.
>
> We can fix that by giving PinBuffer() a new skip_if_not_valid mode that
> doesn't pin invalid buffers. It might occasionally skip when it
> shouldn't due to the unlocked read of the header flags, but that's
> unlikely and perfectly acceptable for an opportunistic optimisation
> routine, and it can only succeed when it really should due to the
> compare-exchange loop.
>
> XXX This also fixes ReadRecentBuffer()'s failure to bump the usage
> count. Fix separately or back-patch this?
FWIW, I'm inclined to not backpatch the usagecount change at this
point. Unless we have a clear case where it really hurts, I'm more worried
about disturbing working workloads...
Greetings,
Andres Freund
