public inbox for [email protected]  
help / color / mirror / Atom feed
From: David Rowley <[email protected]>
To: Peter Eisentraut <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Justin Pryzby <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: Bharath Rupireddy <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Peter Geoghegan <[email protected]>
Cc: [email protected]
Cc: Robert Haas <[email protected]>
Cc: Melanie Plageman <[email protected]>
Subject: Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode
Date: Sat, 15 Apr 2023 12:59:52 +1200
Message-ID: <CAApHDvrYafdxurB+TuCs0+kZXphsHa+VBBSd3QaE2eHycXL3vw@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CAAKRu_ZevZBtvqM1Eh3Wr587wf2vCW=XxOkaUn=s12apE5ekZg@mail.gmail.com>
	<[email protected]>
	<CAAKRu_bJRKe+v_=OqwC+5sA3j5qv8rqdAwy3+yHaO3wmtfrCRg@mail.gmail.com>
	<CAAKRu_byOPANPzNiu=d2ACvfzWfNURPYPwo2v7iWddHLFLMcJA@mail.gmail.com>
	<CAApHDvo57+sYWx+nwM3DXM2m0S8f1XDruTURSWjdwOSRKu6s9Q@mail.gmail.com>
	<CAAKRu_bwE+kov=1Y35-dxF4jandvFWA1NiSyCG_=kUFs9xvw-Q@mail.gmail.com>
	<CAAKRu_YdzP42WTZPQO0QePosxFqjHavC1v87BMaaiXqD9b12wA@mail.gmail.com>
	<CAApHDvrDsCeyVxkkrfe5H4gsw4sBDSPL8cK2fAdhMEn+pY96Rw@mail.gmail.com>
	<20230406172056.gnp7ohxs4bykrn5z@liskov>
	<CAApHDvqhdpN6qdy4pTpt5FWm6A1M4O37fT+r7819t5OJJ4tfUg@mail.gmail.com>
	<20230406214431.2pkmcogizdamdmzz@liskov>
	<CAApHDvqs+Fcw9Yrn4ORCAX0xKK1-SiCC0w1j7Dhg=G9hrvag7g@mail.gmail.com>
	<[email protected]>

On Fri, 14 Apr 2023 at 19:20, Peter Eisentraut
<[email protected]> 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


Attachments:

  [application/octet-stream] fix_buffer_usage_limit_docs.patch (7.3K, ../CAApHDvrYafdxurB+TuCs0+kZXphsHa+VBBSd3QaE2eHycXL3vw@mail.gmail.com/2-fix_buffer_usage_limit_docs.patch)
  download | inline diff:
diff --git a/doc/src/sgml/ref/analyze.sgml b/doc/src/sgml/ref/analyze.sgml
index 94e30f1ce7..6f0bb4631b 100644
--- a/doc/src/sgml/ref/analyze.sgml
+++ b/doc/src/sgml/ref/analyze.sgml
@@ -28,7 +28,7 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
 
     VERBOSE [ <replaceable class="parameter">boolean</replaceable> ]
     SKIP_LOCKED [ <replaceable class="parameter">boolean</replaceable> ]
-    BUFFER_USAGE_LIMIT [ <replaceable class="parameter">string</replaceable> ]
+    BUFFER_USAGE_LIMIT [ <replaceable class="parameter">size</replaceable> ]
 
 <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase>
 
@@ -110,7 +110,8 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
       <xref linkend="guc-vacuum-buffer-usage-limit"/>.  Higher settings can
       allow <command>ANALYZE</command> to run more quickly, but having too
       large a setting may cause too many other useful pages to be evicted from
-      shared buffers.
+      shared buffers.  The minimum value is <literal>128 kB</literal> and the
+      maximum value is <literal>16 GB</literal>.
      </para>
     </listitem>
    </varlistentry>
@@ -129,6 +130,19 @@ ANALYZE [ VERBOSE ] [ <replaceable class="parameter">table_and_columns</replacea
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><replaceable class="parameter">size</replaceable></term>
+    <listitem>
+     <para>
+      Specifies an amount of memory in kilobytes.  Sizes may also be specified
+      as a string containing the size followed by any one of the following
+      memory units: <literal>kB</literal> (kilobytes), <literal>MB</literal>
+      (megabytes), <literal>GB</literal> (gigabytes), or
+      <literal>TB</literal> (terabytes).
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><replaceable class="parameter">table_name</replaceable></term>
     <listitem>
diff --git a/doc/src/sgml/ref/vacuum.sgml b/doc/src/sgml/ref/vacuum.sgml
index dd0fbb8cb7..54f47a0c64 100644
--- a/doc/src/sgml/ref/vacuum.sgml
+++ b/doc/src/sgml/ref/vacuum.sgml
@@ -39,7 +39,7 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     PARALLEL <replaceable class="parameter">integer</replaceable>
     SKIP_DATABASE_STATS [ <replaceable class="parameter">boolean</replaceable> ]
     ONLY_DATABASE_STATS [ <replaceable class="parameter">boolean</replaceable> ]
-    BUFFER_USAGE_LIMIT [ <replaceable class="parameter">string</replaceable> ]
+    BUFFER_USAGE_LIMIT [ <replaceable class="parameter">size</replaceable> ]
 
 <phrase>and <replaceable class="parameter">table_and_columns</replaceable> is:</phrase>
 
@@ -364,7 +364,8 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
       <xref linkend="guc-vacuum-buffer-usage-limit"/>.  Higher settings can
       allow <command>VACUUM</command> to run more quickly, but having too
       large a setting may cause too many other useful pages to be evicted from
-      shared buffers.
+      shared buffers.  The minimum value is <literal>128 kB</literal> and the
+      maximum value is <literal>16 GB</literal>.
      </para>
     </listitem>
    </varlistentry>
@@ -392,6 +393,19 @@ VACUUM [ FULL ] [ FREEZE ] [ VERBOSE ] [ ANALYZE ] [ <replaceable class="paramet
     </listitem>
    </varlistentry>
 
+   <varlistentry>
+    <term><replaceable class="parameter">size</replaceable></term>
+    <listitem>
+     <para>
+      Specifies an amount of memory in kilobytes.  Sizes may also be specified
+      as a string containing the size followed by any one of the following
+      memory units: <literal>kB</literal> (kilobytes), <literal>MB</literal>
+      (megabytes), <literal>GB</literal> (gigabytes), or
+      <literal>TB</literal> (terabytes).
+     </para>
+    </listitem>
+   </varlistentry>
+
    <varlistentry>
     <term><replaceable class="parameter">table_name</replaceable></term>
     <listitem>
diff --git a/doc/src/sgml/ref/vacuumdb.sgml b/doc/src/sgml/ref/vacuumdb.sgml
index 8280cf0fb0..caf97af215 100644
--- a/doc/src/sgml/ref/vacuumdb.sgml
+++ b/doc/src/sgml/ref/vacuumdb.sgml
@@ -121,6 +121,19 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
+     <varlistentry>
+      <term><option>--buffer-usage-limit <replaceable class="parameter">size</replaceable></option></term>
+      <listitem>
+       <para>
+        Specifies the
+        <glossterm linkend="glossary-buffer-access-strategy">Buffer Access Strategy</glossterm>
+        ring buffer size for a given invocation of <application>vacuumdb</application>.
+        This size is used to calculate the number of shared buffers which will
+        be reused as part of this strategy.  See <xref linkend="sql-vacuum"/>.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry>
       <term><option><optional>-d</optional> <replaceable class="parameter">dbname</replaceable></option></term>
       <term><option><optional>--dbname=</optional><replaceable class="parameter">dbname</replaceable></option></term>
@@ -278,19 +291,6 @@ PostgreSQL documentation
       </listitem>
      </varlistentry>
 
-     <varlistentry>
-      <term><option>--buffer-usage-limit <replaceable class="parameter">buffer_usage_limit</replaceable></option></term>
-      <listitem>
-       <para>
-        Specifies the
-        <glossterm linkend="glossary-buffer-access-strategy">Buffer Access Strategy</glossterm>
-        ring buffer size for a given invocation of <application>vacuumdb</application>.
-        This size is used to calculate the number of shared buffers which will
-        be reused as part of this strategy.  See <xref linkend="sql-vacuum"/>.
-       </para>
-      </listitem>
-     </varlistentry>
-
      <varlistentry>
       <term><option>-n <replaceable class="parameter">schema</replaceable></option></term>
       <term><option>--schema=<replaceable class="parameter">schema</replaceable></option></term>
diff --git a/src/bin/scripts/vacuumdb.c b/src/bin/scripts/vacuumdb.c
index 1241644ed5..687af9c1f3 100644
--- a/src/bin/scripts/vacuumdb.c
+++ b/src/bin/scripts/vacuumdb.c
@@ -1128,6 +1128,7 @@ help(const char *progname)
 	printf(_("  %s [OPTION]... [DBNAME]\n"), progname);
 	printf(_("\nOptions:\n"));
 	printf(_("  -a, --all                       vacuum all databases\n"));
+	printf(_("      --buffer-usage-limit=SIZE   size of ring buffer used for vacuum\n"));
 	printf(_("  -d, --dbname=DBNAME             database to vacuum\n"));
 	printf(_("      --disable-page-skipping     disable all page-skipping behavior\n"));
 	printf(_("  -e, --echo                      show the commands being sent to the server\n"));
@@ -1136,7 +1137,6 @@ help(const char *progname)
 	printf(_("      --force-index-cleanup       always remove index entries that point to dead tuples\n"));
 	printf(_("  -j, --jobs=NUM                  use this many concurrent connections to vacuum\n"));
 	printf(_("      --min-mxid-age=MXID_AGE     minimum multixact ID age of tables to vacuum\n"));
-	printf(_("      --buffer-usage-limit=BUFSIZE size of ring buffer used for vacuum\n"));
 	printf(_("      --min-xid-age=XID_AGE       minimum transaction ID age of tables to vacuum\n"));
 	printf(_("      --no-index-cleanup          don't remove index entries that point to dead tuples\n"));
 	printf(_("      --no-process-main           skip the main relation\n"));


view thread (8+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
  Subject: Re: Option to not use ringbuffer in VACUUM, using it in failsafe mode
  In-Reply-To: <CAApHDvrYafdxurB+TuCs0+kZXphsHa+VBBSd3QaE2eHycXL3vw@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox