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

On Sat, Mar 11, 2023 at 11:55 PM Melanie Plageman
<melanieplageman(at)gmail(dot)com> wrote:
>
> > 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.

I've not reviewed the patchset in depth yet but I got assertion
failure and SEGV when using the buffer_usage_limit parameter.

postgres(1:471180)=# vacuum (buffer_usage_limit 10000000000) ;
2023-03-15 17:10:02.947 JST [471180] ERROR: buffer_usage_limit for a
vacuum must be between -1 and 16777216. 10000000000 is invalid. at
character 9

The message show the max value is 16777216, but when I set it, I got
an assertion failure:

postgres(1:470992)=# vacuum (buffer_usage_limit 16777216) ;
TRAP: failed Assert("ring_size < MAX_BAS_RING_SIZE_KB"), File:
"freelist.c", Line: 606, PID: 470992

Then when I used 1 byte lower value, 16777215, I got a SEGV:

postgres(1:471180)=# vacuum (buffer_usage_limit 16777215) ;
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: 2023-03-15
17:10:59.404 JST [471159] LOG: server process (PID 471180) was
terminated by signal 11: Segmentation fault

Finally, when I used a more lower value, 16777100, I got a memory
allocation error:

postgres(1:471361)=# vacuum (buffer_usage_limit 16777100) ;
2023-03-15 17:12:17.853 JST [471361] ERROR: invalid memory alloc
request size 18446744073709551572

Probably vacuum_buffer_usage_limit also has the same issue.

Also, should we support a table option for vacuum_buffer_usage_limit as well?

Regards,

--
Masahiko Sawada
Amazon Web Services: https://aws.amazon.com