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

From: Melanie Plageman <melanieplageman(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, 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>
Subject: Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode
Date: 2023-04-17 21:21:09
Message-ID: 20230417212109.imuaim2gvp5z7glj@liskov
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Apr 15, 2023 at 12:59:52PM +1200, David Rowley wrote:
> 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.

I like that we are now using "size" consistently instead of bufsize etc.

>
> > 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.

Are we still thinking that reordering the VACUUM (and ANALYZE) options
makes sense. And, if so, should it be alphabetical within parameter
category? That is, all actual parameters (e.g. FULL and FREEZE) are
alphabetically organized first followed by all parameter types (e.g.
boolean and size) alphabetically listed?

- Melanie

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Nathan Bossart 2023-04-17 21:21:41 Re: Clean up hba.c of code freeing regexps
Previous Message Nathan Bossart 2023-04-17 21:06:21 Re: check_strxfrm_bug()