I wrote:
> The argument for "sending too little" comes from the race condition
> that's described in the function's comments: a variable that has
> source PGC_S_DEFAULT (ie, has never moved off its compiled-in default)
> in the leader could have just been updated in the postmaster, due to
> re-reading postgresql.conf after SIGHUP. In that case, when the
> postmaster forks the worker it will inherit the new setting from
> postgresql.conf, and will run with that because the leader didn't send
> its value. So we risk having a situation where parallel workers are
> using a setting that the leader won't adopt until it next goes idle.
After further study I've realized that the above can't happen, because
the existing code is considerably more magical, delicate, and badly
commented than I'd realized.
Basically it divides the GUCs into two categories: those that will
never be shipped based on their context or name (for which we assume
the worker will obtain correct values via other mechanisms), and all
others, which are shipped if they don't have their compiled-in
default values. On the receiving side, the first loop in
RestoreGUCOptions acts to ensure that all GUCs in the second category
are at their compiled-in defaults, essentially throwing away whatever
the worker might've obtained from pg_db_role_setting or other places.
Then, after receiving and applying the shipped GUCs, we have an exact
match to the leader's state (given the assumption that the compiled-in
values are identical, anyway), without any race conditions.
The magical/fragile part of this is that the same can_skip_guc test
works for both sides of the operation; it's not really obvious that
that must be so.
Forcing all the potentially-shipped GUCs into PGC_S_DEFAULT state has
another critical and undocumented property, which is that it ensures
that set_config_option won't refuse to apply any of the incoming
settings on the basis of their source priority being lower than what
the worker already has.
So we do need RestoreGUCOptions to be doing something equivalent to
InitializeOneGUCOption, although preferably without the leaks.
That doesn't look too awful though, since we should be able to just
Assert that the stack is empty; the only thing that may need to be
freed is the current values of string variables. I'll see about
fixing that and improving the comments while this is all swapped in.
regards, tom lane
