On Sat, Apr 03, 2021 at 08:25:46PM -0500, Justin Pryzby wrote:
> Forking this thread
> https://www.postgresql.org/message-id/20210403154336.GG29125%40momjian.us

Didn't see this one, thanks for forking.

> I understood "developer" to mean someone who's debugging postgres itself, not
> (say) a function written using pl/pgsql.  Like backtrace_functions,
> post_auth_delay, jit_profiling_support.
> 
> But I see that some "dev" options are more user-facing (for a sufficiently
> advanced user):
> ignore_checksum_failure, ignore_invalid_pages, zero_damaged_pages.
> 
> Also, I understood this to mean the "category" in pg_settings, but I guess
> what's important here is the absense of the GUC in the sample/template config
> file.  pg_settings.category and the sample headings it appears are intended to
> be synchronized, but a few of them are out of sync.  See attached.
> 
> +1 to move this to "developer" options and remove it from the sample config:
> 
> # - Other Planner Options -
> #force_parallel_mode = off

0001 has some changes to pg_config_manual.h related to valgrind and
memory randomization.  You may want to remove that before posting a
patch.

-       {"track_commit_timestamp", PGC_POSTMASTER, REPLICATION,
+       {"track_commit_timestamp", PGC_POSTMASTER, REPLICATION_SENDING,
I can get behind this change for clarity where it gets actively used.

-       {"track_activity_query_size", PGC_POSTMASTER, RESOURCES_MEM,
+       {"track_activity_query_size", PGC_POSTMASTER, STATS_COLLECTOR,
But not this one, because it is a memory setting.

-       {"force_parallel_mode", PGC_USERSET, QUERY_TUNING_OTHER,
+       {"force_parallel_mode", PGC_USERSET, DEVELOPER_OPTIONS,
And not this one either, as it is mainly a planner thing, like the
other parameters in the same area.

The last change is related to log_autovacuum_min_duration, and I can
get behind the argument you are making to group all log activity
parameters together.  Now, about this part:
+#log_autovacuum_min_duration = -1  # -1 disables, 0 logs all actions and
+                   # their durations, > 0 logs only
+                   # actions running at least this number
+                   # of milliseconds.
I think that we should clarify in the description that this is an
autovacuum-only thing, say by appending a small sentence about the
fact that it logs autovacuum activities, in a similar fashion to
log_temp_files.  Moving the parameter out of the autovacuum section
makes it lose a bit of context.

@@ -6903,6 +6903,7 @@ fetch_more_data_begin(AsyncRequest *areq)
    char        sql[64];

    Assert(!fsstate->conn_state->pendingAreq);
+   Assert(fsstate->conn);
What's this diff doing here? 
--
Michaelx

Attachment: signature.asc
Description: PGP signature

Reply via email to