Incorrect GUC descriptions in docs and postgresql.conf.sample
Hi all,
I got curious with what Justin just told here with
max_logical_replication_workers:
/messages/by-id/20210526001359.GE3676@telsasoft.com
And while looking at the full set of GUCs, I noticed much more than
one parameter that needed adjustments in the documentation when these
are PGC_SIGHUP or PGC_POSTMASTER, leading me to the attached patch.
Any comments or objections?
Thanks,
--
Michael
Attachments:
guc-docfixes.patchtext/x-diff; charset=us-asciiDownload+40-4
Your patch adds documentation about GUCs that can only be set at server
start/config/commandline.
But it's not true for any of these, which are all HUP/SUSET.
Please double check your logic :)
src/backend/utils/misc/guc.c: {"autovacuum_work_mem", PGC_SIGHUP, RESOURCES_MEM,
src/backend/utils/misc/guc.c: {"remove_temp_files_after_crash", PGC_SIGHUP, ERROR_HANDLING_OPTIONS,
src/backend/utils/misc/guc.c: {"restart_after_crash", PGC_SIGHUP, ERROR_HANDLING_OPTIONS,
src/backend/utils/misc/guc.c: {"log_lock_waits", PGC_SUSET, LOGGING_WHAT,
src/backend/utils/misc/guc.c: {"autovacuum_work_mem", PGC_SIGHUP, RESOURCES_MEM,
src/backend/utils/misc/guc.c: {"ssl_max_protocol_version", PGC_SIGHUP, CONN_AUTH_SSL,
src/backend/utils/misc/guc.c: {"ssl_min_protocol_version", PGC_SIGHUP, CONN_AUTH_SSL,
--
Justin
On Tue, May 25, 2021 at 08:43:14PM -0500, Justin Pryzby wrote:
Your patch adds documentation about GUCs that can only be set at server
start/config/commandline.
Oh: I realized that I read too quickly and misinterpretted what "only be set in
the config" means (I know I'm not the only one). Oops.
In some cases it sounds strange to say that a parameter can "only" be set in
the config file, since it's dynamically changed at runtime. Which is more
flexible than restrictive.
of a restriction.
But it's not true for any of these, which are all HUP/SUSET.
Please double check your logic :)src/backend/utils/misc/guc.c: {"autovacuum_work_mem", PGC_SIGHUP, RESOURCES_MEM,
src/backend/utils/misc/guc.c: {"remove_temp_files_after_crash", PGC_SIGHUP, ERROR_HANDLING_OPTIONS,
src/backend/utils/misc/guc.c: {"restart_after_crash", PGC_SIGHUP, ERROR_HANDLING_OPTIONS,
src/backend/utils/misc/guc.c: {"log_lock_waits", PGC_SUSET, LOGGING_WHAT,
src/backend/utils/misc/guc.c: {"autovacuum_work_mem", PGC_SIGHUP, RESOURCES_MEM,
src/backend/utils/misc/guc.c: {"ssl_max_protocol_version", PGC_SIGHUP, CONN_AUTH_SSL,
src/backend/utils/misc/guc.c: {"ssl_min_protocol_version", PGC_SIGHUP, CONN_AUTH_SSL,
--
Justin
On Tue, May 25, 2021 at 09:01:30PM -0500, Justin Pryzby wrote:
On Tue, May 25, 2021 at 08:43:14PM -0500, Justin Pryzby wrote:
Your patch adds documentation about GUCs that can only be set at server
start/config/commandline.Oh: I realized that I read too quickly and misinterpretted what "only be set in
the config" means (I know I'm not the only one). Oops.In some cases it sounds strange to say that a parameter can "only" be set in
the config file, since it's dynamically changed at runtime. Which is more
flexible than restrictive.
That's the wording used for ages in the documentation, so I would
stick with that.
But it's not true for any of these, which are all HUP/SUSET.
Please double check your logic :)src/backend/utils/misc/guc.c: {"autovacuum_work_mem", PGC_SIGHUP, RESOURCES_MEM,
src/backend/utils/misc/guc.c: {"remove_temp_files_after_crash", PGC_SIGHUP, ERROR_HANDLING_OPTIONS,
src/backend/utils/misc/guc.c: {"restart_after_crash", PGC_SIGHUP, ERROR_HANDLING_OPTIONS,
src/backend/utils/misc/guc.c: {"log_lock_waits", PGC_SUSET, LOGGING_WHAT,
src/backend/utils/misc/guc.c: {"autovacuum_work_mem", PGC_SIGHUP, RESOURCES_MEM,
src/backend/utils/misc/guc.c: {"ssl_max_protocol_version", PGC_SIGHUP, CONN_AUTH_SSL,
src/backend/utils/misc/guc.c: {"ssl_min_protocol_version", PGC_SIGHUP, CONN_AUTH_SSL,
There is one point where you are right here: log_lock_waits has no
need to be changed. Looks like I checked too many things at once.
--
Michael
On Wed, May 26, 2021 at 11:10:22AM +0900, Michael Paquier wrote:
There is one point where you are right here: log_lock_waits has no
need to be changed. Looks like I checked too many things at once.
Fixed that, did one extra round of review, and applied.
--
Michael