On 6/7/22 7:58 PM, Tom Lane wrote:
> I wrote:
>> The attached draft patch makes the following changes:
>
> Here's a v2 that polishes the loose ends:
Thanks! I reviewed and did some basic testing locally. I did not see any
of the generated defaults.
>> (I didn't do anything about in_hot_standby, which is set through
>> a hack rather than via set_config_option; not sure whether we want
>> to do anything there, or what it should be if we do.)
The comment diff showed that it went from "hack" to "hack" :)
> I concluded that directly assigning to in_hot_standby was a fairly
> horrid idea and we should just change it with SetConfigOption.
> With this coding, as long as in_hot_standby is TRUE it will show
> as having a non-default setting in \dconfig. I had to remove the
> assertion I'd added about PGC_INTERNAL variables only receiving
> "default" values, but this just shows that was too inflexible anyway.
I tested this and the server correctly rendered "in_hot_standby" in
\dconfig. I also tested setting "hot_standby to "on" while the server
was not in recovery, and \dconfig correctly did not render "in_hot_standby".
>> * The rlimit-derived value of max_stack_depth is likewise relabeled
>> as PGC_S_DYNAMIC_DEFAULT, resolving the complaint Jonathan had upthread.
>> But now that we have a way to hide this, I'm having second thoughts
>> about whether we should. If you are on a platform that's forcing an
>> unreasonably small stack size, it'd be good if \dconfig told you so.
>> Could it be sane to label that value as PGC_S_DYNAMIC_DEFAULT only when
>> it's the limit value (2MB) and PGC_S_ENV_VAR when it's smaller?
>
> I concluded that was just fine and did it.
Reading the docs, I think this is OK to do. We already say that "2MB" is
a very conservative setting. And even if the value can be computed to be
larger, we don't allow the server to set it higher than "2MB".
I don't know how frequently issues around "max_stack_depth" being too
small are reported -- I'd be curious to know that -- but I don't have
any strong arguments against allowing the behavior you describe based on
our current docs.
Jonathan