Date: Sat, 27 Jul 2019 17:41:30 -0400
From: Bruce Momjian <bruce@momjian.us>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Daniel Gustafsson <daniel@yesql.se>, basil.bourque@gmail.com,
	PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>
Subject: Re: BUG #15912: The units of `autovacuum_vacuum_cost_delay` setting
 should be documented
Message-ID: <20190727214130.bjgyv2holjng3oeb@momjian.us>
References: <C6E10B26-97E0-4B4D-9AEE-E26A7E2263F9@yesql.se>
 <20190726220242.GA16906@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="m6wz5mc25st5pqev"
Content-Disposition: inline
In-Reply-To: <20190726220242.GA16906@alvherre.pgsql>
User-Agent: NeoMutt/20170113 (1.7.2)
Precedence: bulk


--m6wz5mc25st5pqev
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Fri, Jul 26, 2019 at 06:02:42PM -0400, Alvaro Herrera wrote:
> Now you could complain that this is inconsistent with other
> descriptions; for example, log_autovacuum_min_duration talks about
> milliseconds, which sounds a bit archaic to me:
> 
>    Causes each action executed by autovacuum to be logged if it ran for
>    at least the specified number of milliseconds. Setting this to zero
>    logs all autovacuum actions. -1 (the default) disables logging
>    autovacuum actions. For example, if you set this to 250ms then all
>    automatic vacuums and analyzes that run 250ms or longer will be
>    logged. In addition, when this parameter is set to any value other
>    than -1, a message will be logged if an autovacuum action is skipped
>    due to a conflicting lock or a concurrently dropped relation.
>    Enabling this parameter can be helpful in tracking autovacuum
>    activity. This parameter can only be set in the postgresql.conf file
>    or on the server command line; but the setting can be overridden for
>    individual tables by changing table storage parameters.
> 
> 
> I'm not really sure what's a good way to attack this problem, but I
> doubt that focusing on just one description is a sufficient solution.

Yes, I looked at this earlier in the week and had the same conclusion. 
I went over config.sgml and saw many inconsistencies of the same type
being complained about here.

I went through the file and found a number of cases using milliseconds
and kilobytes that were unclear, and adjusted them.  I dealt only with
the cases where the base unit (seconds/bytes) was not the default unit. 
Patch attached.

-- 
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

+ As you are, so once was I.  As I am, so you will be. +
+                      Ancient Roman grave inscription +

--m6wz5mc25st5pqev
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="default.diff"

diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index c91e3e1550..15d0a75d4c 100644
--- a/doc/src/sgml/config.sgml
+++ b/doc/src/sgml/config.sgml
@@ -947,7 +947,7 @@ include_dir 'conf.d'
       </term>
       <listitem>
        <para>
-        Specifies the number of milliseconds that transmitted data may
+        Specifies duration (defaults to milliseconds) that transmitted data may
         remain unacknowledged before a connection is forcibly closed.
         A value of 0 uses the system default.
         This parameter is supported only on systems that support
@@ -1805,7 +1805,7 @@ include_dir 'conf.d'
         for temporary files, such as sort and hash temporary files, or the
         storage file for a held cursor.  A transaction attempting to exceed
         this limit will be canceled.
-        The value is specified in kilobytes, and <literal>-1</literal> (the
+        The default unit is kilobytes, and <literal>-1</literal> (the
         default) means no limit.
         Only superusers can change this setting.
        </para>
@@ -1893,7 +1893,7 @@ include_dir 'conf.d'
        </term>
        <listitem>
         <para>
-         The length of time, in milliseconds, that the process will sleep
+         The duration (defaults to milliseconds) that the process will sleep
          when the cost limit has been exceeded.
          The default value is zero, which disables the cost-based vacuum
          delay feature.  Positive values enable cost-based vacuuming.
@@ -2018,11 +2018,11 @@ include_dir 'conf.d'
        </term>
        <listitem>
         <para>
-         Specifies the delay between activity rounds for the
+         Specifies the delay (defaults to milliseconds) between activity rounds for the
          background writer.  In each round the writer issues writes
          for some number of dirty buffers (controllable by the
          following parameters).  It then sleeps for <varname>bgwriter_delay</varname>
-         milliseconds, and repeats.  When there are no dirty buffers in the
+         duration, and repeats.  When there are no dirty buffers in the
          buffer pool, though, it goes into a longer sleep regardless of
          <varname>bgwriter_delay</varname>.  The default value is 200
          milliseconds (<literal>200ms</literal>). Note that on many systems, the
@@ -2781,9 +2781,10 @@ include_dir 'conf.d'
       <listitem>
       <para>
         Specifies how often the WAL writer flushes WAL. After flushing WAL it
-        sleeps for <varname>wal_writer_delay</varname> milliseconds, unless woken up
+        sleeps for <varname>wal_writer_delay</varname> duration (defaults
+        to milliseconds), unless woken up
         by an asynchronously committing transaction. If the last flush
-        happened less than <varname>wal_writer_delay</varname> milliseconds ago and
+        happened less than <varname>wal_writer_delay</varname> duration ago and
         less than <varname>wal_writer_flush_after</varname> bytes of WAL have been
         produced since, then WAL is only written to the operating system, not
         flushed to disk.
@@ -2806,7 +2807,8 @@ include_dir 'conf.d'
       <listitem>
       <para>
         Specifies how often the WAL writer flushes WAL. If the last flush
-        happened less than <varname>wal_writer_delay</varname> milliseconds ago and
+        happened less than <varname>wal_writer_delay</varname> duration ago
+        (defaults to milliseconds) and
         less than <varname>wal_writer_flush_after</varname> bytes of WAL have been
         produced since, then WAL is only written to the operating system, not
         flushed to disk.  If <varname>wal_writer_flush_after</varname> is set
@@ -3659,7 +3661,7 @@ restore_command = 'copy "C:\\server\\archivedir\\%f" "%p"'  # Windows
       <listitem>
        <para>
         Terminate replication connections that are inactive longer
-        than the specified number of milliseconds. This is useful for
+        than the specified duration (defaults to milliseconds). This is useful for
         the sending server to detect a standby crash or network outage.
         A value of zero disables the timeout mechanism. The default value
         is 60 seconds. With a cluster distributed across multiple geographic
@@ -4111,7 +4113,7 @@ ANY <replaceable class="parameter">num_sync</replaceable> ( <replaceable class="
       <listitem>
        <para>
         Terminate replication connections that are inactive longer
-        than the specified number of milliseconds. This is useful for
+        than the specified duration (defaults to milliseconds). This is useful for
         the receiving standby server to detect a primary node crash or network
         outage.
         A value of zero disables the timeout mechanism.  This parameter
@@ -5606,7 +5608,7 @@ local0.*    /var/log/postgresql
        <para>
         When <varname>logging_collector</varname> is enabled,
         this parameter determines the maximum size of an individual log file.
-        After this many kilobytes have been emitted into a log file,
+        After this many bytes (default units is kilobytes) have been emitted into a log file,
         a new log file will be created.  Set to zero to disable size-based
         creation of new log files.
         This parameter can only be set in the <filename>postgresql.conf</filename>
@@ -5849,8 +5851,8 @@ local0.*    /var/log/postgresql
        <listitem>
         <para>
          Causes the duration of each completed statement to be logged
-         if the statement ran for at least the specified number of
-         milliseconds, modulated by <varname>log_statement_sample_rate</varname>.
+         if the statement ran for at least the specified duration (defaults
+         to milliseconds), modulated by <varname>log_statement_sample_rate</varname>.
          Setting this to zero prints all statement durations.
          <literal>-1</literal> (the default) disables logging statements due to
          exceeding duration threshold; for example, if you set it to
@@ -6498,7 +6500,7 @@ log_line_prefix = '%m [%p] %q%u@%d/%a '
         A log entry is made for each temporary file when it is deleted.
         A value of zero logs all temporary file information, while positive
         values log only files whose size is greater than or equal to
-        the specified number of kilobytes.  The
+        the specified number of bytes (default units is kilobytes).  The
         default setting is -1, which disables such logging.
         Only superusers can change this setting.
        </para>
@@ -6946,7 +6948,7 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       <listitem>
        <para>
         Causes each action executed by autovacuum to be logged if it ran for at
-        least the specified number of milliseconds.  Setting this to zero logs
+        least the specified duration (defaults to milliseconds).  Setting this to zero logs
         all autovacuum actions. <literal>-1</literal> (the default) disables
         logging autovacuum actions.  For example, if you set this to
         <literal>250ms</literal> then all automatic vacuums and analyzes that run
@@ -7595,8 +7597,8 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       </term>
       <listitem>
        <para>
-        Abort any statement that takes more than the specified number of
-        milliseconds, starting from the time the command arrives at the server
+        Abort any statement that takes more than the specified duration
+        (defaults to milliseconds), starting from the time the command arrives at the server
         from the client.  If <varname>log_min_error_statement</varname> is set to
         <literal>ERROR</literal> or lower, the statement that timed out will also be
         logged.  A value of zero (the default) turns this off.
@@ -7618,8 +7620,8 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       </term>
       <listitem>
        <para>
-        Abort any statement that waits longer than the specified number of
-        milliseconds while attempting to acquire a lock on a table, index,
+        Abort any statement that waits longer than the specified duration
+        (defaults to milliseconds) while attempting to acquire a lock on a table, index,
         row, or other database object.  The time limit applies separately to
         each lock acquisition attempt.  The limit applies both to explicit
         locking requests (such as <command>LOCK TABLE</command>, or <command>SELECT
@@ -7654,7 +7656,7 @@ COPY postgres_log FROM '/full/path/to/logfile.csv' WITH csv;
       <listitem>
        <para>
        Terminate any session with an open transaction that has been idle for
-       longer than the specified duration in milliseconds. This allows any
+       longer than the specified duration (defaults to milliseconds). This allows any
        locks held by that session to be released and the connection slot to be reused;
        it also allows tuples visible only to this transaction to be vacuumed.  See
        <xref linkend="routine-vacuuming"/> for more details about this.
@@ -8490,7 +8492,7 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
       </term>
       <listitem>
        <para>
-        This is the amount of time, in milliseconds, to wait on a lock
+        This is the duration (defaults to milliseconds), to wait on a lock
         before checking to see if there is a deadlock condition. The
         check for deadlock is relatively expensive, so the server doesn't run
         it every time it waits for a lock. We optimistically assume
@@ -8509,7 +8511,7 @@ dynamic_library_path = 'C:\tools\postgresql;H:\my_project\lib;$libdir'
 
        <para>
         When <xref linkend="guc-log-lock-waits"/> is set,
-        this parameter also determines the length of time to wait before
+        this parameter also determines the duration to wait before
         a log message is issued about the lock wait.  If you are trying
         to investigate locking delays you might want to set a shorter than
         normal <varname>deadlock_timeout</varname>.

--m6wz5mc25st5pqev--