Re: shouldn't we log permission errors when accessing the configured trigger file? - Mailing list pgsql-hackers

From Bruce Momjian
Subject Re: shouldn't we log permission errors when accessing the configured trigger file?
Date
Msg-id 20140416225514.GL7443@momjian.us
Whole thread Raw
In response to Re: shouldn't we log permission errors when accessing the configured trigger file?  (Magnus Hagander <magnus@hagander.net>)
Responses Re: shouldn't we log permission errors when accessing the configured trigger file?
List pgsql-hackers
On Mon, Jan 27, 2014 at 03:45:38PM +0100, Magnus Hagander wrote:
> On Mon, Jan 27, 2014 at 3:43 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>
>     On Sun, Jan 26, 2014 at 1:03 PM, Andres Freund <andres@2ndquadrant.com>
>     wrote:
>     > For some reason CheckForStandbyTrigger() doesn't report permission
>     > errors when stat()int the trigger file. Shouldn't we fix that?
>     >
>     > static bool
>     > CheckForStandbyTrigger(void)
>     > {
>     > ...
>     >         if (stat(TriggerFile, &stat_buf) == 0)
>     >         {
>     >                 ereport(LOG,
>     >                                 (errmsg("trigger file found: %s",
>     TriggerFile)));
>     >                 unlink(TriggerFile);
>     >                 triggered = true;
>     >                 fast_promote = true;
>     >                 return true;
>     >         }
>     >
>     > Imo the stat() should warn about all errors but ENOENT?
>
>     Seems reasonable.  It could lead to quite a bit of log spam, I
>     suppose, but the way things are now could be pretty mystifying if
>     you've located your trigger file somewhere outside $PGDATA, and a
>     parent directory is lacking permissions.
>
>
> +1. Since it actually indicates something that's quite broken (since with that
> you can never make the trigger work until you fix it), the log spam seems like
> it would be appropriate. (Logspam is never nice, but a single log line is also
> very easy to miss - this should log enough that you wouldn't)

I have developed the attached patch to address this issue.

--
  Bruce Momjian  <bruce@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +

Attachment

pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: pg_get_viewdefs() indentation considered harmful
Next
From: Bruce Momjian
Date:
Subject: Re: test script, was Re: [COMMITTERS] pgsql: psql: conditionally display oids and replication identity