Re: Speed-up shared buffers prewarming

On Wed, 15 Mar 2023 at 21:38, Konstantin Knizhnik <knizhnik(at)garret(dot)ru> wrote:
>
> Hi hackers,
>
> It is well known fact that queries using sequential scan can not be used to prewarm cache, because them are using ring buffer
> even if shared buffers are almost empty.
> I have searched hackers archive but failed to find any discussion about it.
> What are the drawbacks of using free buffers even with BAM_BULKREAD strategy?
> I mean the following trivial patch:
>
> diff --git a/src/backend/storage/buffer/freelist.c b/src/backend/storage/buffer/freelist.c
> index 6be80476db..243335d0e4 100644
> --- a/src/backend/storage/buffer/freelist.c
> +++ b/src/backend/storage/buffer/freelist.c
> @@ -208,8 +208,15 @@ StrategyGetBuffer(BufferAccessStrategy strategy, uint32 *buf_state)
> /*
> * If given a strategy object, see whether it can select a buffer. We
> * assume strategy objects don't need buffer_strategy_lock.
> */
> - if (strategy != NULL)
> + if (strategy != NULL && StrategyControl->firstFreeBuffer < 0)
> {
> buf = GetBufferFromRing(strategy, buf_state);
> if (buf != NULL)
>
> So if there are free buffers, then use normal buffer allocation instead of GetBufferFromRing.

Yes. As seen in [1], ring buffers aren't all that great in some cases,
and I think this is one. Buffer allocation should always make use of
the available resources, so that it doesn't take O(N/ring_size) scans
on a table to fill the buffers if that seqscan is the only workload of
the system.

> Definitely it is possible that seqscan limited by ring buffer will be completed faster than seqscan filling all shared buffers especially if
> size of shared buffers is large enough. OS will need some extra time to commit memory and may be swap out other regions to find enough physical
> memory for shared buffers.

Not just that, but it is also possible that by ignoring the ring we're
going to hit pages that aren't yet in the CPU caches and we would thus
need to fetch the data from RAM (or from another NUMA node), which
could be more expensive than reading it from a local kernel's file
cache and writing it to the local cache lines.

Anyway, I'm all for this change - I don't think we need to be careful
about trashing other workload's buffers if the buffer is useless for
literally every workload.

Kind regards,

Matthias van de Meent

[1] https://www.postgresql.org/message-id/flat/20230111182720.ejifsclfwymw2reb%40awork3.anarazel.de