I just spent a rather confused half hour while testing my GUC
assign-hook patch, and when I finally figured out what was happening,
it made me wonder whether we should redesign the behavior a little bit.
The current behavior of ProcessConfigFile is that it runs through all
the "name = value" pairs extracted from the file and tries to fully
verify each value (by seeing whether set_config_option with changeVal
false likes it). Only if every one of them checks out does it actually
apply any of the settings. Now this is nice and conservative --- the
aim is to avoid applying settings from a possibly corrupted file, in
case somebody fat-fingered their edits in a big way. But there's a
little problem:
1. It's possible that not all the backends agree on whether a setting
is valid. The case I was testing involved setting client_encoding
from the config file, so whether it succeeds depends on the database
encoding (some conversions might exist and others not). This means
that some backends might apply the postgresql.conf settings and others
not. That's pretty bad in itself, if something that needs to be
consistent system-wide is changing.
2. Only the postmaster reports config file problems at elevel LOG;
backends only complain at DEBUG3, to avoid cluttering the log with
lots of duplicate messages. This means that if you do have a few
backends that fail to adopt a setting, there likely won't be anything
in the log to tell you so. (The reason I was so confused is that I'd
raised log_min_messages to DEBUG5 to try to understand what was
happening ... but my backend-under-test wasn't adopting that setting,
and wasn't logging anything to tell me so either ...)
So I'm thinking we should adopt a strategy that's less likely to result
in divergent behavior among different backends. The idea I have in mind
is to have the first "validation" pass only check that each name is a
legal GUC variable name, and not look at the values at all. If so, try
to apply all the values. Any that fail to apply we log as usual, but
still apply the others. ISTM that verifying the names should be enough
protection against broken files for practical purposes, and it should be
something that all backends will agree on even if there are individual
values that are not valid for all.
Comments?
regards, tom lane
