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

Attachment: OpenPGP_signature
Description: OpenPGP digital signature

Reply via email to