Re: Make ringbuffer threshold and ringbuffer sizes configurable?

Hi,

On 2020-02-06 07:18:00 +0100, Laurenz Albe wrote:
> On Wed, 2020-02-05 at 20:00 -0800, Andres Freund wrote:
> > The ringbuffers we use for seqscans, vacuum, copy etc can cause very
> > drastic slowdowns (see e.g. [1]), an can cause some workloads to
> > practically never end up utilizing shared buffers. ETL workloads
> > e.g. regularly fight with that problem.
> >
> > I think it would make sense to have seqscan_ringbuffer_threshold,
> > {bulkread,bulkwrite,vacuum}_ringbuffer_size. I think they often sensibly
> > are set in proportion of shared_buffers, so I suggest defining them as
> > floats, where negative values divide shared_buffers, whereas positive
> > values are absolute sizes, and 0 disables the use of ringbuffers.
>
> Sounds reasonable.

> I feel that it should be as few GUCs as possible, so I think that
> having one per type of operation might be too granular.

They already are set to different sizes, so I don't really see how
that's something doable. The relevant bits of code are:

BufferAccessStrategy
GetAccessStrategy(BufferAccessStrategyType btype)
{
BufferAccessStrategy strategy;
int ring_size;

/*
* Select ring size to use. See buffer/README for rationales.
*
* Note: if you change the ring size for BAS_BULKREAD, see also
* SYNC_SCAN_REPORT_INTERVAL in access/heap/syncscan.c.
*/
switch (btype)
{
case BAS_NORMAL:
/* if someone asks for NORMAL, just give 'em a "default" object */
return NULL;

case BAS_BULKREAD:
ring_size = 256 * 1024 / BLCKSZ;
break;
case BAS_BULKWRITE:
ring_size = 16 * 1024 * 1024 / BLCKSZ;
break;
case BAS_VACUUM:
ring_size = 256 * 1024 / BLCKSZ;
break;

and

/*
* If the table is large relative to NBuffers, use a bulk-read access
* strategy and enable synchronized scanning (see syncscan.c). Although
* the thresholds for these features could be different, we make them the
* same so that there are only two behaviors to tune rather than four.
* (However, some callers need to be able to disable one or both of these
* behaviors, independently of the size of the table; also there is a GUC
* variable that can disable synchronized scanning.)
*
* Note that table_block_parallelscan_initialize has a very similar test;
* if you change this, consider changing that one, too.
*/
if (!RelationUsesLocalBuffers(scan->rs_base.rs_rd) &&
scan->rs_nblocks > NBuffers / 4)
{
allow_strat = (scan->rs_base.rs_flags & SO_ALLOW_STRAT) != 0;
allow_sync = (scan->rs_base.rs_flags & SO_ALLOW_SYNC) != 0;
}
else
allow_strat = allow_sync = false;

> This should of course also be a storage parameter that can be
> set per table.

I honestly don't quite get that. What precisely is this addressing? I
mean fine, I can add that, but it's sufficiently more complicated than
the GUCs, and I don't really forsee that being particularly useful to
tune on a per table basis. A lot of the reason behind having the
ringbuffers is about managing the whole system impact, rather than
making individual table fast/slow.

- Andres