On Jun20, 2011, at 18:16 , Tom Lane wrote:
> Florian Pflug <fgp@phlo.org> writes:
>> The code the actually implements the "check settings first, apply later" logic
>> isn't easy to read. Now, assume that this code has a bug. Then, with your
>> patch applied, we might end up with the postmaster applying a setting (because
>> it didn't abort early) but the backend ignoring it (because they did abort early).
>> This is obviously bad. Depending on the setting, the consequences may range
>> from slightly confusing behaviour to outright crashes I guess...
>
> This is already known to happen: there are cases where the postmaster
> and a backend can come to different conclusions about whether a setting
> is valid (eg, because it depends on database encoding). Whether that's
> a bug or not isn't completely clear, but if this patch is critically
> dependent on the situation never happening, I don't think we can accept
> it.
Does that mean that some backends might currently choose to ignore an
updated config file wholesale on SIGUP (because some settings are invalid)
while others happily apply it? Meaning that they'll afterwards disagree
even on modified settings which *would* be valid for both backends?
Or do these kinds of setting rejections happen late enough to fall out
of the all-or-nothing logic in ProcessConfigFile?
Anyway, the patch *doesn't* depend on all backends's setting being in sync.
The issue we were discussion was whether it's OK to add another small risk
of them getting out of sync by aborting early on errors in backends but
not in the postmaster. I was arguing against that, while Alexey was in favour
of it, on the grounds that it reduces log traffic (but only at DEBUG2 or
beyond).
best regards,
Florian Pflug