GUC flags

From: Justin Pryzby <pryzby(at)telsasoft(dot)com>
To: pgsql-hackers(at)lists(dot)postgresql(dot)org
Subject: GUC flags
Date: 2021-11-29 03:08:33
Message-ID: 20211129030833.GJ17618@telsasoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

This isn't flagged with GUC_EXPLAIN:
enable_incremental_sort

Here's some more candidates:
shared_buffers - it seems unfortunate this is not included; actually, it seems
like maybe this should be always included - not just if it's set to a
non-default, but especially if it's left at the default..
I suppose it's more important for DML than SELECT.
temp_tablespaces isn't, but temp_buffers is;
autovacuum - if it's off, that can be the cause of the issue (same as force_parallel_mode, which has GUC_EXPLAIN);
max_worker_processes - isn't, but these are: max_parallel_workers, max_parallel_workers_per_gather;
track_io_timing - it can have high overhead;
session_preload_libraries, shared_preload_libraries, local_preload_libraries;
debug_assertions - it's be kind of nice to show this whenever it's true (even thought it's not "changed")
debug_discard_caches
lc_collate and lc_ctype ?
server_version_num - I asked about this in the past, but Tom thought it should
not be included, and I kind of agree, but I'm including it here for
completeness sake

This isn't marked GUC_NOT_IN_SAMPLE, like all other DEVELOPER_OPTIONS:
trace_recovery_messages

I'm not sure jit_tuple_deforming should be marked GUC_NOT_IN_SAMPLE.
I disable this one because it's slow for tables with many attributes.
Same for jit_expressions ?

bgwriter_lru_maxpages should have GUC_UNIT_BLOCKS

max_identifier_length should have BYTES (like log_parameter_max_length and
track_activity_query_size).

block_size and wal_block_size should say BYTES (like wal_segment_size)
And all three descriptions should say ".. in [prefix]bytes" (but see below).

Maybe these should have COMPAT_OPTIONS_PREVIOUS:
integer_datetimes
ssl_renegotiation_limit

autovacuum_freeze_max_age has a comment about pg_resetwal which is obsolete
since 74cf7d46a.

checkpoint_warning refers to "checkpoint segments", which is obsolete since
88e982302.

The attached does the least-disputable, lowest hanging fruit.

More ideas:

Maybe maintenance_io_concurrency should not be GUC_EXPLAIN. But it's used not
only by ANALYZE but also heap_index_delete_tuples.

Should these be GUC_RUNTIME_COMPUTED?
in_hot_standby, data_directory_mode

Since GUC_DISALLOW_IN_FILE effectively implies GUC_NOT_IN_SAMPLE in
src/backend/utils/misc/help_config.c:displayStruct(), many of the redundant
GUC_NOT_IN_SAMPLE could be removed.

I think several things with COMPAT_OPTIONS_PREVIOUS could have
GUC_NO_SHOW_ALL and/or GUC_NOT_IN_SAMPLE.

The GUC descriptions are a hodge podge of full sentences and telegrams.
There's no consistency whether the long description can be read independently
from the short description. For these GUCs, the short description reads more
like a "DETAIL" message:
|trace_recovery_messages, log_min_error_statement, log_min_messages, client_min_messages
|log_transaction_sample_rate, log_statement_sample_rate
|data_directory_mode, log_file_mode, unix_socket_permissions
|log_directory, log_destination, log_line_prefix
|unix_socket_group, default_tablespace, DateStyle, maintenance_work_mem, geqo_generations

For integer/real GUCs, the long description is being used just to describe the
"special" values:
|jit_inline_above_cost, jit_optimize_above_cost, jit_above_cost, log_startup_progress_interval,
|tcp_user_timeout, tcp_keepalives_interval, tcp_keepalives_idle, log_temp_files, old_snapshot_threshold,
|log_parameter_max_length_on_error, log_parameter_max_length, log_autovacuum_min_duration, log_min_duration_sample,
|idle_session_timeout, idle_in_transaction_session_timeout, lock_timeout,
|statement_timeout, shared_memory_size_in_huge_pages

Descriptions of some GUCs describe their default units, but other's don't.
The units are not very important for input, since a non-default unit can be
specified, like SET statement_timeout='1h'. It's important for output, and
SHOW already includes a unit, which may not be the default unit. So I think
the default units should be removed from the descriptions.

This cleanup is similar to GUC categories fixed in a55a98477.
Tom was of the impression that there's more loose ends on that thread.
https://www.postgresql.org/message-id/flat/16997-ff16127f6e0d1390(at)postgresql(dot)org

--
Justin

Attachment Content-Type Size
0001-guc.c-fix-GUC-flags.patch text/x-diff 3.3 KB
0002-guc.c-fix-GUC-descriptions.patch text/x-diff 16.2 KB
0003-guc.c-do-not-mention-units.patch text/x-diff 5.5 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Masahiko Sawada 2021-11-29 03:38:08 Re: parallel vacuum comments
Previous Message Michael Paquier 2021-11-29 02:58:44 Re: [PATCH] ALTER tab completion