[PostgreSQL-Hackers] Re: [BUGS] BUG #5650: Postgres service showing as stopped when in fact it is running

Magnus Hagander writes:
I took a quick look at the code, and from what I can tell this is
because PQconnectionNeedsPassword() always returns false if a
pgpass.conf has been used. There is no handling the case where pgpass
is used, but has an incorrect password.

On Fri, Sep 24, 2010 at 16:04, Tom Lane wrote:
Magnus Hagander <magnus@hagander.net> writes:
Why should it?  That code is complicated enough, I don't think it needs
to have a behavior of pretending that a wrong entry isn't there.

On 09/24/2010 10:15 AM, Magnus Hagander wrote:
On Fri, Sep 24, 2010 at 16:04, Tom Lanewrote:
In that case, we should probably teach pg_ctl about this case, no?
Since it clearly gives an incorrect message to the user now...

Andrew Dunstan writes:
pg_ctl decides that the server is running iff it can connect to it. Do
you intend to provide for a different test?

On Fri, Sep 24, 2010 at 4:11 PM, Tom Lane wrote:

(Of course, a "pg_ping" utility would be a better answer, but nobody's
gotten around to that in more than ten years, so I'm not holding my
breath.)

Dave Page writes:
Hmm, that sounded like it could be my 9.1 mini project - then Google
showed me that SeanC wrote something already.
http://archives.postgresql.org/pgsql-patches/2003-07/msg00053.php

On 09/24/2010 11:11 AM, Tom Lane wrote:
Andrew Dunstan<andrew@dunslane.net> writes:
Seems like getting a password challenge from the server is sufficient
evidence that the server is running, whether we are able to meet the
challenge or not. Perhaps we could just twiddle pg_ctl's "is it up"
test a bit to notice whether the connect failure was of this sort.

Magnus Hagander wrote:
On Fri, Sep 17, 2010 at 05:51, Ashesh Vashi
wrote:
I took a quick look at the code, and from what I can tell this is
because PQconnectionNeedsPassword() always returns false if a
pgpass.conf has been used. There is no handling the case where pgpass
is used, but has an incorrect password.

Does anybody recall the specific reason for this? Do we need a way for
pg_ctl to figure this out, or do we need to change it in
PQconnecitonNeedsPassword()?

On Fri, Nov 12, 2010 at 03:49, Bruce Momjian wrote:
Magnus Hagander wrote:
I was not able to reproduce this failure on my BSD system using GIT
head:

$ psql test
psql: FATAL:  password authentication failed for user "postgres"
password retrieved from file "/u/postgres/.pgpass"

$ pg_ctl status
pg_ctl: server is running (PID: 710)
/usr/var/local/pgsql/bin/postgres "-i"

Magnus Hagander wrote:
The problem is not in pg_ctl status, it's in pg_ctl start. They're
different codepaths - status never tries to actually connect, it just
checks if the process is alive.

Bruce Momjian writes:
Uh, I still cannot reproduce the failure:

Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
I would imagine you need -w option on the start. The whole issue
here is whether start's wait-for-server-start code works.

On Fri, Nov 12, 2010 at 17:47, Bruce Momjian wrote:
Tom Lane wrote:
Thanks, I am now able to reproduce this.  I was able to get this to
report the .pgpass problem:

$ psql postgres
psql: FATAL:  password authentication failed for user "postgres"
password retrieved from file "/u/postgres/.pgpass"

$ pg_ctl stop
waiting for server to shut down.... done
server stopped

$ pg_ctl -w -l /dev/null start
waiting for server to start....FATAL:  password authentication failed
for user "postgres"
password retrieved from file "/u/postgres/.pgpass"
.FATAL:  password authentication failed for user "postgres"
password retrieved from file "/u/postgres/.pgpass"
.FATAL:  password authentication failed for user "postgres"
password retrieved from file "/u/postgres/.pgpass"
.^C

I basically report the connection error string if it starts with "FATAL:".

I originally tried to check for an ERRCODE_INVALID_PASSWORD error field
(see // comments), but it seems there is no way to access this, i.e.
PQgetResult(conn) on a connection failure is always NULL.

Anyway, perhaps FATAL is a better test because it will report any major
failure, not just a .pgpass one.

Patch attached.

Bad Bruce, using C++ comments like that :P And non-context diff ;)

On Wed, Nov 17, 2010 at 19:50, Bruce Momjian wrote:
Magnus Hagander wrote:
I was just going to post on that.  :-)  Right now, it prints the FATAL
and keeps printing 60 times, then says not running.  Should we just exit
on FATAL and output a special exit string, or say running?

Magnus Hagander wrote:
running. That might break other scenarios though, but i'm not sure - I
think we can safely say the server is running when we try to log in
and get a password failure.

On Wed, Nov 17, 2010 at 19:57, Bruce Momjian wrote:
Magnus Hagander wrote:
That was another part of the discussion.  Right now we report any FATAL,
so it might be a password problem, or something else, and it seems doing
all FATALs is the best idea because it will catch any other cases like
this.

Is FATAL, in general, enough to conclude the server is running?

Magnus Hagander writes:
No - specifically, we will send FATAL when "the database system is
starting up", which is exactly the one we want to *avoid*.
I think we should only exclude the password case. I guess we could
also do all fatal *except* <list>, but that seems more fragile.

Tom Lane wrote:
Magnus Hagander <magnus@hagander.net> writes:
I believe that the above argument is exactly backwards. What we want
here is to check the result of postmaster.c's canAcceptConnections(),
and there are only a finite number of error codes that can result from
rejections there. If we get past that, there are a large number of
possible failures, but all of them indicate that the postmaster is in
principle willing to accept connections. Checking for password errors
only is utterly wrong: any other type of auth failure would be the same
for this purpose, as would "no such database", "no such user", "too many
connections", etc etc etc.

Bruce Momjian writes:
Agreed. So how do we pass that info to libpq without exceeding the
value of fixing this problem? Should we parse pg_controldata output?
pg_upgrade could use machine-readable output from that too.

Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
pg_controldata seems 100% unrelated to this problem. You cannot even
tell if the postmaster is alive just by inspecting pg_control.

Bruce Momjian writes:
Tom Lane wrote:
OK, so a new libpq function --- got it. Would we just pass the status
from the backend or can it be done without backend modifications?

Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
It would definitely be better to do it without backend mods, so that
the functionality would work against back-branch postmasters.

To my mind, the entire purpose of such a function is to classify the
possible errors so that the caller doesn't have to. So I wouldn't
consider that it ought to "pass back the status from the backend".
I think what we basically want is a function that takes a conninfo
string (or one of the variants of that) and returns an enum defined
more or less like this:

* failed to connect to postmaster
* connected, but postmaster is not accepting sessions
* postmaster is up and accepting sessions

I'm not sure those are exactly the categories we want, but something
close to that. In particular, I don't know if there's any value in
subdividing the "not accepting sessions" status --- pg_ctl doesn't
really care, but other use-cases might want to tell the difference
between the various canAcceptConnections failure states.

BTW, it is annoying that we can't definitively distinguish "postmaster
is not running" from a connectivity problem, but I can't see a way
around that.

Bruce Momjian wrote:
Tom Lane wrote:
Agreed. I will research this.

I have researched this and developed the attached patch. It implements
PGping() and PGpingParams() in libpq, and has pg_ctl use it for pg_ctl
-w server status detection.

The new output for cases where .pgpass is not allowing for a connection
is:

$ pg_ctl -w -l /dev/null start
waiting for server to start.... done
server started
However, could not connect, perhaps due to invalid authentication or
misconfiguration.

The code basically checks the connection status between PQconnectStart()
and connectDBComplete() to see if the server is running but we failed to
connect for some reason.

On Fri, Nov 26, 2010 at 3:11 AM, Bruce Momjian wrote:
I have applied this patch, with modified wording of the "cannot connect"
case:

$ pg_ctl -w -l /dev/null start
waiting for server to start.... done
server started
warning:  could not connect, perhaps due to invalid authentication or
misconfiguration.

Fujii Masao wrote:
This patch breaks the behavior that "pg_ctl -w start" waits until the standby
has been ready to accept read-only queries. IOW, pg_ctl without this patch
continues to check the connection even if the connection is rejected because
the database has not been consistent yet. But pg_ctl with this patch treats
that rejection as success of the standby starting and prints the above
messages.

I agree to treat the receipt of password request from the server as success
of the server starting. But I don't think that we should treat other rejection
cases that way and change the existing behavior.

Fujii Masao wrote:
connect"
authentication or
OK, that is easy to fix. The only downside is that if you misconfigured
.pgpass (which is what I used for testing), you have to wait 60 seconds
to get the "cannot connect" error message. Is that OK?

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

+ It's impossible for everything to be true. +

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Bruce Momjian writes:
Fujii Masao wrote:
OK, that is easy to fix.

Bruce Momjian writes:
Fujii Masao wrote:

I wrote:
The reason this is a problem is that somebody, in a fit of inappropriate
optimization, took out the code that allowed canAcceptConnections to
distinguish the "not consistent yet" state.

On Fri, Nov 26, 2010 at 6:30 PM, Tom Lane wrote:
Speaking of classifying errors, should we have a fourth result value to
cover "obviously bogus parameters"?

Tom Lane wrote:
I wrote:
Oh, no, that's not the case --- the PM_RECOVERY postmaster state does
still distinguish not-ready from ready. The real problem is that what
Bruce implemented has practically nothing to do with what was discussed
last week. PQping is supposed to be smarter about classifying errors
than this.

Bruce Momjian writes:
Tom Lane wrote:
I was not aware this was discussed last week because I am behind on
email. I was fixing a report from a month ago. I did explain how I was
doing the tests.

Tom Lane wrote:
Bruce Momjian <bruce@momjian.us> writes:
Um, you did respond in that thread, several times even:
http://archives.postgresql.org/pgsql-hackers/2010-11/msg01102.php
so I kind of assumed that the patch you presented this week did
what was agreed to last week.