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

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Justin Pryzby <pryzby(at)telsasoft(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>, 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>, Melanie Plageman <melanieplageman(at)gmail(dot)com>
Subject: Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode
Date: 2023-04-15 00:59:52
Message-ID: CAApHDvrYafdxurB+TuCs0+kZXphsHa+VBBSd3QaE2eHycXL3vw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, 14 Apr 2023 at 19:20, Peter Eisentraut
<peter(dot)eisentraut(at)enterprisedb(dot)com> wrote:
>
> I came across these new options and had a little bit of trouble figuring
> them out from the documentation. Maybe this could be polished a bit.
>
> vacuumdb --help says
>
> --buffer-usage-limit=BUFSIZE
>
> I can guess what a "SIZE" might be, but is "BUFSIZE" different from a
> "SIZE"? Maybe simplify here.
>
> On the vacuumdb man page, the placeholder is
>
> <replaceable class="parameter">buffer_usage_limit</replaceable>
>
> which is yet another way of phrasing it. Maybe also use "size" here?
>
> The VACUUM man page says
>
> BUFFER_USAGE_LIMIT [ <replaceable ...>string</replaceable> ]
>
> which had me really confused. The detailed description later doesn't
> give any further explanation of possible values, except that
> <literal>0</literal> is apparently a possible value, which in my mind is
> not a string. Then there is a link to guc-vacuum-buffer-usage-limit,
> which lifts the mystery that this is really just an integer setting with
> possible memory-size units, but it was really hard to figure that out
> from the start!
>
> Moreover, on the VACUUM man page, right below BUFFER_USAGE_LIMIT, it
> explains the different kinds of accepted values, and "string" wasn't
> added there. Maybe also change this to "size" here and add an
> explanation there what kinds of sizes are possible.
>
> Finally, the locations of the new options in the various documentation
> places seems a bit random. The vacuumdb --help output and the man page
> appear to be mostly alphabetical, so --buffer-usage-limit should be
> after -a/--all. (Also note that right now the option isn't even in the
> same place in the --help output versus the man page.)

These are all valid points. I've attached a patch aiming to address
each of them.

> The order of the options on the VACUUM man page doesn't make any sense
> anymore. This isn't really the fault of this patch, but maybe it's time
> to do a fresh reordering there.

Agreed, that likely wasn't a big problem say about 5 years ago when we
had far fewer options, but the number has grown quite a bit since
then.

Right after I fix the points you've mentioned seems a good time to address that.

David

Attachment Content-Type Size
fix_buffer_usage_limit_docs.patch application/octet-stream 7.3 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2023-04-15 01:02:43 Re: v16dev: invalid memory alloc request size 8488348128
Previous Message Thomas Munro 2023-04-15 00:26:53 Re: Direct I/O