Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode

From: Melanie Plageman <melanieplageman(at)gmail(dot)com>
To: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Geoghegan <pg(at)bowt(dot)ie>, pgsql-hackers(at)postgresql(dot)org, Robert Haas <robertmhaas(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>
Subject: Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode
Date: 2023-03-11 14:55:33
Message-ID: CAAKRu_ZEPLwTL==L5jurkbr-wXK+-n+sph5-hFJ9wkNRBoH-dw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Tue, Feb 28, 2023 at 3:16 AM Bharath Rupireddy
> <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
>
> > On Thu, Jan 12, 2023 at 6:06 AM Andres Freund <andres(at)anarazel(dot)de> wrote:
> > >
> > > On 2023-01-11 17:26:19 -0700, David G. Johnston wrote:
> > > > Should we just add "ring_buffers" to the existing "shared_buffers" and
> > > > "temp_buffers" settings?
> > >
> > > The different types of ring buffers have different sizes, for good reasons. So
> > > I don't see that working well. I also think it'd be more often useful to
> > > control this on a statement basis - if you have a parallel import tool that
> > > starts NCPU COPYs you'd want a smaller buffer than a single threaded COPY. Of
> > > course each session can change the ring buffer settings, but still.
> >
> > How about having GUCs for each ring buffer (bulk_read_ring_buffers,
> > bulk_write_ring_buffers, vacuum_ring_buffers - ah, 3 more new GUCs)?
> > These options can help especially when statement level controls aren't
> > easy to add (COPY, CREATE TABLE AS/CTAS, REFRESH MAT VIEW/RMV)? If
> > needed users can also set them at the system level. For instance, one
> > can set bulk_write_ring_buffers to other than 16MB or -1 to disable
> > the ring buffer to use shared_buffers and run a bunch of bulk write
> > queries.

In attached v3, I've changed the name of the guc from buffer_usage_limit
to vacuum_buffer_usage_limit, since it is only used for vacuum and
autovacuum.

I haven't added the other suggested strategy gucs, as those could easily
be done in a future patchset.

I've also changed GetAccessStrategyExt() to GetAccessStrategyWithSize()
-- similar to initArrayResultWithSize().

And I've added tab completion for BUFFER_USAGE_LIMIT so that it is
easier to try out my patch.

Most of the TODOs in the code are related to the question of how
autovacuum uses the guc vacuum_buffer_usage_limit. autovacuum creates
the buffer access strategy ring in do_autovacuum() before looping
through and vacuuming tables. It passes this strategy object on to
vacuum(). Since we reuse the same strategy object for all tables in a
given invocation of do_autovacuum(), only failsafe autovacuum would
change buffer access strategies. This is probably okay, but it does mean
that the table-level VacuumParams variable, ring_size, means something
different for autovacuum than vacuum. Autovacuum workers will always
have set it to -1. We won't ever reach code in vacuum() which relies on
VacuumParams->ring_size as long as autovacuum workers pass a non-NULL
BufferAccessStrategy object to vacuum(), though.

- Melanie

Attachment Content-Type Size
v3-0002-use-shared-buffers-when-failsafe-active.patch text/x-patch 969 bytes
v3-0003-add-vacuum-option-to-specify-ring_size-and-guc.patch text/x-patch 11.9 KB
v3-0001-remove-global-variable-vac_strategy.patch text/x-patch 3.4 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2023-03-11 15:54:40 Re: [PoC] Improve dead tuple storage for lazy vacuum
Previous Message Andrei Zubkov 2023-03-11 11:49:50 Re: [PATCH] Tracking statements entry timestamp in pg_stat_statements