On Wed, Jan 22, 2014 at 8:56 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Amit Kapila <amit.kapila16@gmail.com> writes:
>> On Tue, Jan 21, 2014 at 8:25 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>>> While reviewing, I noted that the
>>> "skipping missing configuration file" message in ParseConfigFile()
>>> uses an elevel of LOG, while the other messages in the same file use
>>> "elevel". I'm thinking that's a bug.
>
>> It might not be right for all cases, but I think this is saving us in few cases
>> when the caller (directly or indirectly) has sent elevel as ERROR or FATAL,
>> in such cases we just want to LOG it, if strict is not true. Now it might not
>> be appropriate if the caller has sent DEBUG2 and we use LOG, but to
>> fix it (if this sounds an issue), some more analysis is required, so let's
>> keep it as it is for now.
>
> The problem with this coding is that at SIGHUP, *all* the postgres
> processes will bleat about a missing file.
>
> To avoid that kind of log spam, the usual practice is to code the
> elog level as something like "IsUnderPostmaster ? DEBUG2 : LOG";
> see the elevel setting in ProcessConfigFile() for precedent.
> In fact, I'm pretty sure that the elevel passed to ParseConfigFile
> should always have come from that logic.
The reason why I have mentioned that it can receive different elevel than what ProcessConfigFile() sends is that
ParseConfigFile()gets called from ParseConfigFp() which gets called from other paths (recovery, parse_extension.. )
aswell which sends elevel as ERROR or higher. However ParseConfigFp() when called from those paths will not call
ParseConfigFile()as none of them will contain any include directive, but code as written doesn't ensure that it will
alwayscome elevel set similar to ProcessConfigFile().
> In any case, your argument is bogus because we could already have
> thrown an error at the given elevel further up in ParseConfigFile,
> or later in ParseConfigFp.
True, but not for this kind of case (ignore if file not present).
With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com