Thread: shouldn't we log permission errors when accessing the configured trigger file?
shouldn't we log permission errors when accessing the configured trigger file?
From
Andres Freund
Date:
Hi,
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?
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
Re: shouldn't we log permission errors when accessing the configured trigger file?
From
Robert Haas
Date:
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.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Re: shouldn't we log permission errors when accessing the configured trigger file?
From
Magnus Hagander
Date:
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:Seems reasonable. It could lead to quite a bit of log spam, I
> 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?
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)
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/
Re: shouldn't we log permission errors when accessing the configured trigger file?
From
Bruce Momjian
Date:
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
Re: shouldn't we log permission errors when accessing the configured trigger file?
From
Bruce Momjian
Date:
On Wed, Apr 16, 2014 at 06:55:14PM -0400, Bruce Momjian wrote: > > 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. Patch applied. -- Bruce Momjian <bruce@momjian.us> http://momjian.us EnterpriseDB http://enterprisedb.com + Everyone has their own god. +