wal_sender_timeout / wal_receiver_timeout - seconds or milliseconds?

On Thu, Oct 24, 2019 at 01:06:45PM +0000, PG Doc comments form wrote:

The following documentation comment has been logged on the website:

Page: https://www.postgresql.org/docs/12/runtime-config-replication.html
Description:

Dear sir or madam!

The documentation about the parameter wal_sender_timeout refers to an
integer value in milliseconds in the first sentence. Reading up to the
default value in the paragraph, it is set to 60 seconds. The same difference
of milliseconds and seconds is included in postgresql.conf:

#wal_sender_timeout = 60s # in milliseconds; 0 disables

This difference is found in the documentation and postgresql.conf for
parameter wal_receiver_timeout, too.

#wal_receiver_timeout = 60s # time that receiver waits for
# communication from master
# in milliseconds; 0 disables

Please check and verify. - Thank you!

The default _value_ is 60 seconds, and we use the 's' to specify
seconds. What the comment is saying is that if you _don't_ specify any
units, the integer is in milliseconds. I am not sure how to improve
that.

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

On Mon, Nov 04, 2019 at 09:52:34PM -0500, Bruce Momjian wrote:

The default _value_ is 60 seconds, and we use the 's' to specify
seconds. What the comment is saying is that if you _don't_ specify any
units, the integer is in milliseconds. I am not sure how to improve
that.

I do have a suggestion as per the attached, say:
+#wal_sender_timeout = 60s      # in milliseconds if no unit specified;
+                               # 0 disables
--
Michael

On Tue, Nov 5, 2019 at 01:27:16PM +0900, Michael Paquier wrote:

On Mon, Nov 04, 2019 at 09:52:34PM -0500, Bruce Momjian wrote:

The default _value_ is 60 seconds, and we use the 's' to specify
seconds. What the comment is saying is that if you _don't_ specify any
units, the integer is in milliseconds. I am not sure how to improve
that.

I do have a suggestion as per the attached, say:
+#wal_sender_timeout = 60s      # in milliseconds if no unit specified;
+                               # 0 disables

Ugh, the "if no unit specified" is true of all the settings. Should we
make that clearer in a more central location.

---------------------------------------------------------------------------

--
Michael

diff --git a/src/backend/utils/misc/postgresql.conf.sample b/src/backend/utils/misc/postgresql.conf.sample
index 0fc23e3a61..eea017e840 100644
--- a/src/backend/utils/misc/postgresql.conf.sample
+++ b/src/backend/utils/misc/postgresql.conf.sample
@@ -286,7 +286,8 @@
#max_wal_senders = 10		# max number of walsender processes
# (change requires restart)
#wal_keep_segments = 0		# in logfile segments; 0 disables
-#wal_sender_timeout = 60s	# in milliseconds; 0 disables
+#wal_sender_timeout = 60s	# in milliseconds if no unit specified;
+				# 0 disables

#max_replication_slots = 10	# max number of replication slots
# (change requires restart)
@@ -326,7 +327,8 @@
# query conflicts
#wal_receiver_timeout = 60s		# time that receiver waits for
# communication from master
-					# in milliseconds; 0 disables
+					# in milliseconds if no unit specified;
+					# 0 disables
#wal_retrieve_retry_interval = 5s	# time to wait before retrying to
# retrieve WAL after a failed attempt
#recovery_min_apply_delay = 0		# minimum delay for applying changes during recovery

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

On Tue, Nov 05, 2019 at 10:21:38AM -0500, Bruce Momjian wrote:

Ugh, the "if no unit specified" is true of all the settings. Should we
make that clearer in a more central location.

Hmm. Are you thinking about a new single location in the docs? I
would be afraid to have something like that rot easily as users would
most likely only read the description of each individual parameter
rather than travel across multiple sections. Also, if you look at
guc.c the default units of each parameter are a case-by-case.
log_rotation_age uses minutes for example, autovacuum_naptime seconds,
etc.
--
Michael

On Wed, Nov 6, 2019 at 04:41:10PM +0900, Michael Paquier wrote:

On Tue, Nov 05, 2019 at 10:21:38AM -0500, Bruce Momjian wrote:

Ugh, the "if no unit specified" is true of all the settings. Should we
make that clearer in a more central location.

Hmm. Are you thinking about a new single location in the docs? I
would be afraid to have something like that rot easily as users would
most likely only read the description of each individual parameter
rather than travel across multiple sections. Also, if you look at
guc.c the default units of each parameter are a case-by-case.
log_rotation_age uses minutes for example, autovacuum_naptime seconds,
etc.

Right. I am just asking if we need to do this for _every_ case where
the default value doesn't match the default _units_, or if we need to
have some kind of central notification.

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

On Thu, Nov 07, 2019 at 05:36:17PM -0500, Bruce Momjian wrote:

Right. I am just asking if we need to do this for _every_ case where
the default value doesn't match the default _units_, or if we need to
have some kind of central notification.

Doing it for all places where it matters makes sense to me, aka in the
description of the docs for each parameter, and perhaps add a note in
postgresql.conf.sample. I am still not sure what you mean with a
central notification though, like a separate section in the docs?
--
Michael