Subject: wal_sender_timeout / wal_receiver_timeout - seconds or milliseconds?

From: PG Doc comments form <[email-masked]>
Date: Thu, 24 Oct 2019 13:06:45 +0000

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!

With best regards,
Dirk Aumueller

---

From: Bruce Momjian <[email-masked]>
Date: Mon, 4 Nov 2019 21:52:34 -0500

> 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  <[email-masked]>        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 +

---

From: Michael Paquier <[email-masked]>
Date: Tue, 5 Nov 2019 13:27:16 +0900

> units, the integer is in milliseconds.  I am not sure how to improve

I do have a suggestion as per the attached, say:
+#wal_sender_timeout = 60s      # in milliseconds if no unit specified;
+                               # 0 disables
--
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

---

From: Bruce Momjian <[email-masked]>
Date: Tue, 5 Nov 2019 10:21:38 -0500

> +#wal_sender_timeout = 60s      # in milliseconds if no unit specified;

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

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

---

From: Michael Paquier <[email-masked]>
Date: Wed, 6 Nov 2019 16:41:10 +0900

> 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

---

From: Bruce Momjian <[email-masked]>
Date: Thu, 7 Nov 2019 17:36:17 -0500

> log_rotation_age uses minutes for example, autovacuum_naptime seconds,

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.

---

From: Michael Paquier <[email-masked]>
Date: Fri, 8 Nov 2019 09:05:19 +0900

> 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