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

Hi,

So, I attached a rough implementation of both the autovacuum failsafe
reverts to shared buffers and the vacuum option (no tests or docs or
anything).

The first three patches in the set are just for enabling use of shared
buffers in failsafe mode for autovacuum. I haven't actually ensured it
works (i.e. triggering failsafe mode and checking the stats for whether
or not shared buffers were used).

I was wondering about the status of the autovacuum wraparound failsafe
test suggested in [1]. I don't see it registered for the March's
commitfest. I'll probably review it since it will be useful for this
patchset.

The first patch in the set is to free the BufferAccessStrategy object
that is made in do_autovacuum() -- I don't see when the memory context
it is allocated in is destroyed, so it seems like it might be a leak?

The last patch in the set is a trial implementation of the VACUUM option
suggested -- BUFFER_USAGE_LIMIT. More on that below.

On Wed, Jan 11, 2023 at 4:39 PM Andres Freund <andres(at)anarazel(dot)de> wrote:

> Hi,
>
> On 2023-01-11 16:18:34 -0500, Tom Lane wrote:
> > Peter Geoghegan <pg(at)bowt(dot)ie> writes:
> > > On Wed, Jan 11, 2023 at 11:18 AM Andres Freund <andres(at)anarazel(dot)de>
> wrote:
> > >> I don't like that - it's also quite useful to disable use of
> ringbuffers when
> > >> you actually need to clean up indexes. Especially when we have a lot
> of dead
> > >> tuples we'll rescan indexes over and over...
> >
> > > That's a fair point.
> >
> > > My vote goes to "REUSE_BUFFERS", then.
> >
> > I wonder whether it could make sense to allow a larger ringbuffer size,
> > rather than just the limit cases of "on" and "off".
>
> I can see that making sense, particularly if we were to later extend this
> to
> other users of ringbuffers. E.g. COPYs us of the ringbuffer makes loading
> of
> data > 16MB but also << s_b vastly slower, but it can still be very
> important
> to use if there's lots of parallel processes loading data.
>
> Maybe BUFFER_USAGE_LIMIT, with a value from -1 to N, with -1 indicating the
> default value, 0 preventing use of a buffer access strategy, and 1..N
> indicating the size in blocks?
>
>
I have found the implementation you suggested very hard to use.
The attached fourth patch in the set implements it the way you suggest.
I had to figure out what number to set the BUFER_USAGE_LIMIT to -- and,
since I don't specify shared buffers in units of nbuffer, it's pretty
annoying to have to figure out a valid number.

I think that it would be better to have it be either a percentage of
shared buffers or a size in units of bytes/kb/mb like that of shared
buffers.

Using a fraction or percentage appeals to me because you don't need to
reference your shared buffers setting and calculate what size you want
to set it to. Also, parsing the size in different units sounds like more
work.

Unfortunately, the fraction doesn't really work if we cap the ring size
of a buffer access strategy to NBuffers / 8. Also, there are other
issues like what would 0% and 100% mean.

I have a list of other questions, issues, and TODOs related to the code
I wrote to implement BUFFER_USAGE_LIMIT, but I'm not sure those are
worth discussing until we shape up the interface.

> Would we want to set an upper limit lower than implied by the memory limit
> for
> the BufferAccessStrategy allocation?
>
>
So, I was wondering what you thought about NBuffers / 8 (the current
limit). Does it make sense?

If we clamp the user-specified value to this, I think we definitely need
to inform them through some kind of logging or message. I am sure there
are lots of other gucs doing this -- do you know any off the top of your
head?

- Melanie

[1]
https://www.postgresql.org/message-id/flat/CAB8KJ%3Dj1b3kscX8Cg5G%3DQ39ZQsv2x4URXsuTueJLz%3DfcvJ3eoQ%40mail.gmail.com#ee67664e85c4d11596a92cc71780d29c