Re: ReadRecentBuffer() doesn't scale well

From: Andres Freund <andres(at)anarazel(dot)de>
To: Thomas Munro <thomas(dot)munro(at)gmail(dot)com>
Cc: Peter Geoghegan <pg(at)bowt(dot)ie>, pgsql-hackers(at)postgresql(dot)org, Ants Aasma <ants(at)cybertec(dot)at>
Subject: Re: ReadRecentBuffer() doesn't scale well
Date: 2025-09-18 12:35:58
Message-ID: 627qqnivcplnzbikq7uss7nzy67vqvrcy5er7t4h5hgjz6o6t4@pe3clcvtswlv
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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(dot)munro(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Daniel Gustafsson 2025-09-18 12:43:52 Re: someone else to do the list of acknowledgments
Previous Message Aleksander Alekseev 2025-09-18 12:31:55 Re: [PATCH] Check that index can return in get_actual_variable_range()