Re: hba load error and silent mode

Lists: pgsql-hackers
From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: hba load error and silent mode
Date: 2009-08-24 12:39:02
Message-ID: 4A9289E6.4050606@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers


Last night I had to deal with a puzzled user of version 8.4 who found
postgres refused to start but didn't log any error. It turned out that
there was an error in the pg_hba.conf file, and the client was running
in silent mode (the SUSE default).

This seems like a bug, and it's certainly not very postgres-like behaviour.

Can we move the call to hba_load() in postmaster.c down a bit so it
occurs after the SysLogger is set up? ISTM we really need an absolute
minimum of code between the call to pmdaemonize() and SysLogger_Start().

(Maybe there's a good case for deprecating silent mode. I'm not sure why
Suse uses it. Other distros don't seem to feel the need.)

cheers

andrew


From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: hba load error and silent mode
Date: 2009-08-24 12:57:02
Message-ID: 9837222c0908240557g346ddcdt7065555ed37b5870@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Aug 24, 2009 at 14:39, Andrew Dunstan<andrew(at)dunslane(dot)net> wrote:
>
> Last night I had to deal with a puzzled user of version 8.4  who found
> postgres refused to start but didn't log any error.  It turned out that
> there was an error in the pg_hba.conf file, and the client was running in
> silent mode (the SUSE default).
>
> This seems like a bug, and it's certainly not very postgres-like behaviour.
>
> Can we move the call to hba_load() in postmaster.c down a bit so it occurs
> after the SysLogger is set up? ISTM we really need an absolute minimum of
> code between the call to pmdaemonize() and SysLogger_Start().

I can see other reasons that this would be good, so +1 from me unless
there is any specific reason we can't start it earlier.

> (Maybe there's a good case for deprecating silent mode. I'm not sure why
> Suse uses it. Other distros don't seem to feel the need.)

Could be, but even with silent_mode=off that would be a problem, no?
as in, the log wouldn't go where you'd expect it to go.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: hba load error and silent mode
Date: 2009-08-24 14:11:02
Message-ID: 4A929F76.1070803@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Magnus Hagander wrote:
>> (Maybe there's a good case for deprecating silent mode. I'm not sure why
>> Suse uses it. Other distros don't seem to feel the need.)
>>
>
> Could be, but even with silent_mode=off that would be a problem, no?
> as in, the log wouldn't go where you'd expect it to go.
>
>

It would go to stderr, and then I would have seen it. Silent mode
apparently sends stderr to the bit bucket until the syslogger is set up.
I found where the error was by running postmaster under strace, but
that's really rather icky.

cheers

andrew


From: Joshua Tolley <eggyknap(at)gmail(dot)com>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: hba load error and silent mode
Date: 2009-08-24 14:21:38
Message-ID: 20090824142138.GZ31216@eddie
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Aug 24, 2009 at 02:57:02PM +0200, Magnus Hagander wrote:
> On Mon, Aug 24, 2009 at 14:39, Andrew Dunstan<andrew(at)dunslane(dot)net> wrote:
> >
> > Last night I had to deal with a puzzled user of version 8.4  who found
> > postgres refused to start but didn't log any error.  It turned out that
> > there was an error in the pg_hba.conf file, and the client was running in
> > silent mode (the SUSE default).
> >
> > This seems like a bug, and it's certainly not very postgres-like behaviour.
> >
> > Can we move the call to hba_load() in postmaster.c down a bit so it occurs
> > after the SysLogger is set up? ISTM we really need an absolute minimum of
> > code between the call to pmdaemonize() and SysLogger_Start().
>
> I can see other reasons that this would be good, so +1 from me unless
> there is any specific reason we can't start it earlier.

+1 from me, too, under the same condition. Since logging in really
interesting ways depends on a separate process, any logging before then will
be "abnormal", and any logs we create will probably show up in a relatively
unexpected place. The Principle of Least Surprise suggests we minimize that
possibility.

--
Joshua Tolley / eggyknap
End Point Corporation
http://www.endpoint.com


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: hba load error and silent mode
Date: 2009-08-24 14:31:12
Message-ID: 11296.1251124272@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> (Maybe there's a good case for deprecating silent mode.

+1. The only reason to use it is that an init-script writer is too
lazy to deal with things properly --- the thing in question here being
exactly to think of a place for early failure messages to go.

You can *not* just move the syslogger start call up; it's dependent
on having done some of the other initialization steps. (chdir and
signal setup being obvious candidates.) In general, there will always
be messages that come out before the syslogger can start, and thus a
robust setup has got to provide some fallback place for them.

It might be that a reasonable solution on our end would be for
pmdaemonize to point stdout/stderr someplace other than /dev/null,
perhaps "$PGDATA/postmaster.log"? Of course, it's not clear what
we're supposed to do if that open() fails ...

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: hba load error and silent mode
Date: 2009-08-24 15:15:24
Message-ID: 4A92AE8C.5070703@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> It might be that a reasonable solution on our end would be for
> pmdaemonize to point stdout/stderr someplace other than /dev/null,
> perhaps "$PGDATA/postmaster.log"? Of course, it's not clear what
> we're supposed to do if that open() fails ...
>
>
>

Well, yes, but that is at least a comparatively low risk, certainly much
much lower than the risk having a faulty hba file, for example.

This sounds like a reasonable approach.

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: hba load error and silent mode
Date: 2009-08-24 15:29:25
Message-ID: 12193.1251127765@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Tom Lane wrote:
>> It might be that a reasonable solution on our end would be for
>> pmdaemonize to point stdout/stderr someplace other than /dev/null,
>> perhaps "$PGDATA/postmaster.log"? Of course, it's not clear what
>> we're supposed to do if that open() fails ...

> Well, yes, but that is at least a comparatively low risk, certainly much
> much lower than the risk having a faulty hba file, for example.

> This sounds like a reasonable approach.

Actually, if people are happy with that basic behavior, I think we could
make it robust: open /dev/null and $PGDATA/postmaster.log *before* we
fork away from the terminal session. On failure, report that and exit(1).
On success, go ahead and fork. Failure of the subsequent dup2() calls
should be just about impossible --- in fact, so far as I can tell from
the SUS documents, if we put in a loop for EINTR then there is no
documented way for them to fail.

If no objections, I'll be happy to make this happen.

regards, tom lane


From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: hba load error and silent mode
Date: 2009-08-24 16:28:38
Message-ID: 9837222c0908240928k69b138eaxe401e5c08ea6bc86@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Aug 24, 2009 at 16:31, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> (Maybe there's a good case for deprecating silent mode.
>
> +1.  The only reason to use it is that an init-script writer is too
> lazy to deal with things properly --- the thing in question here being
> exactly to think of a place for early failure messages to go.
>
> You can *not* just move the syslogger start call up; it's dependent
> on having done some of the other initialization steps.  (chdir and
> signal setup being obvious candidates.)  In general, there will always
> be messages that come out before the syslogger can start, and thus a
> robust setup has got to provide some fallback place for them.

Agreed.

I don't see why we couldn't move the hba call specifically, though.
That's a fairly common error, so it would be good if the output went
to the place that is actually configured in postgresql.conf. It's at
least a lot more likely than most other things that are prior to
syslogger startup.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: hba load error and silent mode
Date: 2009-08-24 16:34:15
Message-ID: 14270.1251131655@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> I don't see why we couldn't move the hba call specifically, though.
> That's a fairly common error, so it would be good if the output went
> to the place that is actually configured in postgresql.conf. It's at
> least a lot more likely than most other things that are prior to
> syslogger startup.

Oh, you mean move load_hba *down*, past the syslogger startup?
Yeah, that would probably be all right.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Magnus Hagander <magnus(at)hagander(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: hba load error and silent mode
Date: 2009-08-24 17:50:07
Message-ID: 4A92D2CF.2030806@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>
>> I don't see why we couldn't move the hba call specifically, though.
>> That's a fairly common error, so it would be good if the output went
>> to the place that is actually configured in postgresql.conf. It's at
>> least a lot more likely than most other things that are prior to
>> syslogger startup.
>>
>
> Oh, you mean move load_hba *down*, past the syslogger startup?
> Yeah, that would probably be all right.
>
>

Well, that's what I originally said, yes ;-)

But I don't think that precludes your more general suggestion regarding
startup errors. In particular, I think moving the hba load down would be
reasonable to backpatch to 8.4, whereas I doubt the general fix would.

cheers

andrew


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Magnus Hagander <magnus(at)hagander(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: hba load error and silent mode
Date: 2009-08-24 18:22:48
Message-ID: 28928.1251138168@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Tom Lane wrote:
>> Oh, you mean move load_hba *down*, past the syslogger startup?
>> Yeah, that would probably be all right.

> Well, that's what I originally said, yes ;-)

> But I don't think that precludes your more general suggestion regarding
> startup errors. In particular, I think moving the hba load down would be
> reasonable to backpatch to 8.4, whereas I doubt the general fix would.

Well, the change I had in mind is only a few lines of code, and is
fixing a behavior that you yourself are arguing is unusably broken.
It seems like a reasonable back-patch candidate to me if we think this
is a serious bug. But I personally wasn't seeing any of this as due for
back-patching. The -S behavior has been like it is since forever, and
nobody's complained before.

regards, tom lane


From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Magnus Hagander <magnus(at)hagander(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: hba load error and silent mode
Date: 2009-08-24 18:51:28
Message-ID: 4A92E130.2040900@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>> Tom Lane wrote:
>>
>>> Oh, you mean move load_hba *down*, past the syslogger startup?
>>> Yeah, that would probably be all right.
>>>
>
>
>> Well, that's what I originally said, yes ;-)
>>
>
>
>> But I don't think that precludes your more general suggestion regarding
>> startup errors. In particular, I think moving the hba load down would be
>> reasonable to backpatch to 8.4, whereas I doubt the general fix would.
>>
>
> Well, the change I had in mind is only a few lines of code, and is
> fixing a behavior that you yourself are arguing is unusably broken.
> It seems like a reasonable back-patch candidate to me if we think this
> is a serious bug. But I personally wasn't seeing any of this as due for
> back-patching. The -S behavior has been like it is since forever, and
> nobody's complained before.
>
>
>

We didn't check HBA validity at startup time before, did we? I would not
be surprised to get more complaints now.

cheers

andrew


From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Andrew Dunstan <andrew(at)dunslane(dot)net>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: hba load error and silent mode
Date: 2009-08-24 19:23:30
Message-ID: 9837222c0908241223q4b73d35ma8c1b8179c1e2c21@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On Mon, Aug 24, 2009 at 20:51, Andrew Dunstan<andrew(at)dunslane(dot)net> wrote:
>
>
> Tom Lane wrote:
>>
>> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>>
>>>
>>> Tom Lane wrote:
>>>
>>>>
>>>> Oh, you mean move load_hba *down*, past the syslogger startup?
>>>> Yeah, that would probably be all right.
>>>>
>>
>>
>>>
>>> Well, that's what I originally said, yes ;-)
>>>
>>
>>
>>>
>>> But I don't think that precludes your more general suggestion regarding
>>> startup errors. In particular, I think moving the hba load down would be
>>> reasonable to backpatch to 8.4, whereas I doubt the general fix would.
>>>
>>
>> Well, the change I had in mind is only a few lines of code, and is
>> fixing a behavior that you yourself are arguing is unusably broken.
>> It seems like a reasonable back-patch candidate to me if we think this
>> is a serious bug.  But I personally wasn't seeing any of this as due for
>> back-patching.  The -S behavior has been like it is since forever, and
>> nobody's complained before.
>>
>>
>>
>
> We didn't check HBA validity at startup time before, did we? I would not be
> surprised to get more complaints now.

We checked some of it, but we check it a whole lot more now.

+1 for backpatching at least the move of the load_hba call.

--
Magnus Hagander
Me: http://www.hagander.net/
Work: http://www.redpill-linpro.com/


From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Magnus Hagander <magnus(at)hagander(dot)net>
Cc: Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: hba load error and silent mode
Date: 2009-08-24 19:27:54
Message-ID: 3318.1251142074@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> On Mon, Aug 24, 2009 at 20:51, Andrew Dunstan<andrew(at)dunslane(dot)net> wrote:
>> We didn't check HBA validity at startup time before, did we? I would not be
>> surprised to get more complaints now.

Good point.

> We checked some of it, but we check it a whole lot more now.
> +1 for backpatching at least the move of the load_hba call.

OK, in that case I think it's reasonable to backpatch into 8.4.
Attached is my work-in-progress changes to pmdaemonize --- I think
the code is good now, but I need to go say something in the docs.
I haven't moved the load_hba call yet.

regards, tom lane

Attachment Content-Type Size
unknown_filename text/plain 4.0 KB