Thread: Slightly better testing for pg_ctl(1)'s -w...

Slightly better testing for pg_ctl(1)'s -w...

From
Sean Chittenden
Date:
pg_ctl(1)'s -w option works well if the default user can automatically
authenticate without any user intervention.  The attached patch checks
the error message to see if it's asking for a password.  The theory
being that if it's asking for a password, the backend is up.  I'm not
entirely happy with the fact that I'm dependent on the error message
text, but I couldn't easily figure out a better way to test this via
libpq(3), so I'm not too unhappy... it's just not elegant.  This patch
does not encompass all possible scenarios for the backend being up, but
CONNECTION_BAD being set in libpq(3).  Regardless, it's a start and
hopefully someone can apply this.  I also cleaned up a small memory
leak when a connection is bad (PGconn not being free(3)'ed).  -sc

% pg_ctl -w start && psql
waiting for postmaster to start....done
postmaster started
test=>



--
Sean Chittenden


Attachment

Re: Slightly better testing for pg_ctl(1)'s -w...

From
Tom Lane
Date:
Sean Chittenden <chitt@speakeasy.net> writes:
> pg_ctl(1)'s -w option works well if the default user can automatically
> authenticate without any user intervention.  The attached patch checks
> the error message to see if it's asking for a password.  The theory
> being that if it's asking for a password, the backend is up.  I'm not
> entirely happy with the fact that I'm dependent on the error message
> text, but I couldn't easily figure out a better way to test this via
> libpq(3), so I'm not too unhappy... it's just not elegant.

psql and pg_dump test for this same error string, so you're in good
company on that front, but password prompting is not the only or even
the most likely misleading failure.  I believe both the Red Hat and
Debian distributions set the default auth method to IDENT, meaning that
the message you'd likely get is going to be a bleat about IDENT auth
failing, not a password request.  Unfortunately that message is going to
be localized, but it should have a SQLSTATE assigned, so you could
check for ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION ...

            regards, tom lane

Re: Slightly better testing for pg_ctl(1)'s -w...

From
Sean Chittenden
Date:
>> pg_ctl(1)'s -w option works well if the default user can automatically
>> authenticate without any user intervention.  The attached patch checks
>> the error message to see if it's asking for a password.  The theory
>> being that if it's asking for a password, the backend is up.  I'm not
>> entirely happy with the fact that I'm dependent on the error message
>> text, but I couldn't easily figure out a better way to test this via
>> libpq(3), so I'm not too unhappy... it's just not elegant.
>
> psql and pg_dump test for this same error string, so you're in good
> company on that front, but password prompting is not the only or even
> the most likely misleading failure.  I believe both the Red Hat and
> Debian distributions set the default auth method to IDENT, meaning that
> the message you'd likely get is going to be a bleat about IDENT auth
> failing, not a password request.  Unfortunately that message is going
> to
> be localized, but it should have a SQLSTATE assigned, so you could
> check for ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION ...

Ok, I've read over the code a little bit... it doesn't seem like
there's an obvious way to get the error code via libpq(3).
CopyErrorData() looks promising, but I'm running out of time to find a
better way to do this.  Were you hinting at extending libpq(3) to
having the backend send the errcode to the frontend?  -sc

--
Sean Chittenden


Re: Slightly better testing for pg_ctl(1)'s -w...

From
Tom Lane
Date:
Sean Chittenden <chitt@speakeasy.net> writes:
>> ... it should have a SQLSTATE assigned, so you could
>> check for ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION ...

> Ok, I've read over the code a little bit... it doesn't seem like
> there's an obvious way to get the error code via libpq(3).

Hmmm ... I was thinking of PQresultErrorField, but you don't actually
get a PGresult from a connection failure, so that's no good :-(.

I suppose we need to think about extending libpq so that a SQLSTATE
can be retrieved for connection-level failures.  That kinda moves
it out of the realm of bug-fix-for-beta though.

            regards, tom lane

Re: Slightly better testing for pg_ctl(1)'s -w...

From
Sean Chittenden
Date:
>>> ... it should have a SQLSTATE assigned, so you could
>>> check for ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION ...
>
>> Ok, I've read over the code a little bit... it doesn't seem like
>> there's an obvious way to get the error code via libpq(3).
>
> Hmmm ... I was thinking of PQresultErrorField, but you don't actually
> get a PGresult from a connection failure, so that's no good :-(.
>
> I suppose we need to think about extending libpq so that a SQLSTATE
> can be retrieved for connection-level failures.  That kinda moves
> it out of the realm of bug-fix-for-beta though.

Whew, glad it wasn't me.  Could we (ie Bruce) add getting the raw
errcode as an 8.1 TODO item?  In the mean time, are you going to commit
the pg_ctl patch?  -sc

--
Sean Chittenden


Re: Slightly better testing for pg_ctl(1)'s -w...

From
Bruce Momjian
Date:
Added to TODO:

* Allow libpq to access SQLSTATE so pg_ctl can test for connection failure

  This would be used for checking if the server is up.



---------------------------------------------------------------------------

Sean Chittenden wrote:
> >>> ... it should have a SQLSTATE assigned, so you could
> >>> check for ERRCODE_INVALID_AUTHORIZATION_SPECIFICATION ...
> >
> >> Ok, I've read over the code a little bit... it doesn't seem like
> >> there's an obvious way to get the error code via libpq(3).
> >
> > Hmmm ... I was thinking of PQresultErrorField, but you don't actually
> > get a PGresult from a connection failure, so that's no good :-(.
> >
> > I suppose we need to think about extending libpq so that a SQLSTATE
> > can be retrieved for connection-level failures.  That kinda moves
> > it out of the realm of bug-fix-for-beta though.
>
> Whew, glad it wasn't me.  Could we (ie Bruce) add getting the raw
> errcode as an 8.1 TODO item?  In the mean time, are you going to commit
> the pg_ctl patch?  -sc
>
> --
> Sean Chittenden
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 5: Have you checked our extensive FAQ?
>
>                http://www.postgresql.org/docs/faqs/FAQ.html
>

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: Slightly better testing for pg_ctl(1)'s -w...

From
Bruce Momjian
Date:
Patch applied.  Thanks.

I simplified your test and made the error message a #define that is now
used consistently in all places.  Patch attached.

---------------------------------------------------------------------------


Sean Chittenden wrote:
> pg_ctl(1)'s -w option works well if the default user can automatically
> authenticate without any user intervention.  The attached patch checks
> the error message to see if it's asking for a password.  The theory
> being that if it's asking for a password, the backend is up.  I'm not
> entirely happy with the fact that I'm dependent on the error message
> text, but I couldn't easily figure out a better way to test this via
> libpq(3), so I'm not too unhappy... it's just not elegant.  This patch
> does not encompass all possible scenarios for the backend being up, but
> CONNECTION_BAD being set in libpq(3).  Regardless, it's a start and
> hopefully someone can apply this.  I also cleaned up a small memory
> leak when a connection is bad (PGconn not being free(3)'ed).  -sc
>
> % pg_ctl -w start && psql
> waiting for postmaster to start....done
> postmaster started
> test=>
>

[ Attachment, skipping... ]

>
>
> --
> Sean Chittenden
>

>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
>                http://archives.postgresql.org

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073
Index: src/bin/pg_ctl/pg_ctl.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/pg_ctl/pg_ctl.c,v
retrieving revision 1.38
diff -c -c -r1.38 pg_ctl.c
*** src/bin/pg_ctl/pg_ctl.c    15 Oct 2004 04:54:33 -0000    1.38
--- src/bin/pg_ctl/pg_ctl.c    16 Oct 2004 03:00:05 -0000
***************
*** 364,370 ****
      char        portstr[32];
      char       *p;

-
      *portstr = '\0';

      /* post_opts */
--- 364,369 ----
***************
*** 432,438 ****
      {
          if ((conn = PQsetdbLogin(NULL, portstr, NULL, NULL,
                                   "template1", NULL, NULL)) != NULL &&
!             PQstatus(conn) == CONNECTION_OK)
          {
              PQfinish(conn);
              success = true;
--- 431,439 ----
      {
          if ((conn = PQsetdbLogin(NULL, portstr, NULL, NULL,
                                   "template1", NULL, NULL)) != NULL &&
!             (PQstatus(conn) == CONNECTION_OK ||
!              (strcmp(PQerrorMessage(conn),
!                      PQnoPasswordSupplied) == 0)))
          {
              PQfinish(conn);
              success = true;
Index: src/bin/pg_dump/pg_backup_db.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_backup_db.c,v
retrieving revision 1.59
diff -c -c -r1.59 pg_backup_db.c
*** src/bin/pg_dump/pg_backup_db.c    1 Oct 2004 17:25:55 -0000    1.59
--- src/bin/pg_dump/pg_backup_db.c    16 Oct 2004 03:00:07 -0000
***************
*** 168,174 ****
          if (PQstatus(newConn) == CONNECTION_BAD)
          {
              noPwd = (strcmp(PQerrorMessage(newConn),
!                             "fe_sendauth: no password supplied\n") == 0);
              badPwd = (strncmp(PQerrorMessage(newConn),
                      "Password authentication failed for user", 39) == 0);

--- 168,174 ----
          if (PQstatus(newConn) == CONNECTION_BAD)
          {
              noPwd = (strcmp(PQerrorMessage(newConn),
!                             PQnoPasswordSupplied) == 0);
              badPwd = (strncmp(PQerrorMessage(newConn),
                      "Password authentication failed for user", 39) == 0);

***************
*** 249,255 ****
              die_horribly(AH, modulename, "failed to connect to database\n");

          if (PQstatus(AH->connection) == CONNECTION_BAD &&
!             strcmp(PQerrorMessage(AH->connection), "fe_sendauth: no password supplied\n") == 0 &&
              !feof(stdin))
          {
              PQfinish(AH->connection);
--- 249,255 ----
              die_horribly(AH, modulename, "failed to connect to database\n");

          if (PQstatus(AH->connection) == CONNECTION_BAD &&
!             strcmp(PQerrorMessage(AH->connection), PQnoPasswordSupplied) == 0 &&
              !feof(stdin))
          {
              PQfinish(AH->connection);
Index: src/bin/pg_dump/pg_dumpall.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/pg_dump/pg_dumpall.c,v
retrieving revision 1.53
diff -c -c -r1.53 pg_dumpall.c
*** src/bin/pg_dump/pg_dumpall.c    15 Oct 2004 04:32:28 -0000    1.53
--- src/bin/pg_dump/pg_dumpall.c    16 Oct 2004 03:00:10 -0000
***************
*** 957,963 ****
          }

          if (PQstatus(conn) == CONNECTION_BAD &&
!             strcmp(PQerrorMessage(conn), "fe_sendauth: no password supplied\n") == 0 &&
              !feof(stdin))
          {
              PQfinish(conn);
--- 957,963 ----
          }

          if (PQstatus(conn) == CONNECTION_BAD &&
!             strcmp(PQerrorMessage(conn), PQnoPasswordSupplied) == 0 &&
              !feof(stdin))
          {
              PQfinish(conn);
Index: src/bin/psql/command.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/command.c,v
retrieving revision 1.128
diff -c -c -r1.128 command.c
*** src/bin/psql/command.c    14 Oct 2004 20:23:46 -0000    1.128
--- src/bin/psql/command.c    16 Oct 2004 03:00:13 -0000
***************
*** 929,935 ****
                                 NULL, NULL, dbparam, userparam, pwparam);

          if (PQstatus(pset.db) == CONNECTION_BAD &&
!             strcmp(PQerrorMessage(pset.db), "fe_sendauth: no password supplied\n") == 0 &&
              !feof(stdin))
          {
              PQfinish(pset.db);
--- 929,935 ----
                                 NULL, NULL, dbparam, userparam, pwparam);

          if (PQstatus(pset.db) == CONNECTION_BAD &&
!             strcmp(PQerrorMessage(pset.db), PQnoPasswordSupplied) == 0 &&
              !feof(stdin))
          {
              PQfinish(pset.db);
Index: src/bin/psql/startup.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/psql/startup.c,v
retrieving revision 1.103
diff -c -c -r1.103 startup.c
*** src/bin/psql/startup.c    8 Oct 2004 11:24:19 -0000    1.103
--- src/bin/psql/startup.c    16 Oct 2004 03:00:14 -0000
***************
*** 195,201 ****
                                 username, password);

          if (PQstatus(pset.db) == CONNECTION_BAD &&
!             strcmp(PQerrorMessage(pset.db), "fe_sendauth: no password supplied\n") == 0 &&
              !feof(stdin))
          {
              PQfinish(pset.db);
--- 195,201 ----
                                 username, password);

          if (PQstatus(pset.db) == CONNECTION_BAD &&
!             strcmp(PQerrorMessage(pset.db), PQnoPasswordSupplied) == 0 &&
              !feof(stdin))
          {
              PQfinish(pset.db);
Index: src/bin/scripts/common.c
===================================================================
RCS file: /cvsroot/pgsql/src/bin/scripts/common.c,v
retrieving revision 1.11
diff -c -c -r1.11 common.c
*** src/bin/scripts/common.c    29 Aug 2004 05:06:54 -0000    1.11
--- src/bin/scripts/common.c    16 Oct 2004 03:00:14 -0000
***************
*** 12,17 ****
--- 12,18 ----

  #include "postgres_fe.h"
  #include "common.h"
+ #include "libpq-fe.h"

  #include <pwd.h>
  #include <unistd.h>
***************
*** 102,108 ****
          }

          if (PQstatus(conn) == CONNECTION_BAD &&
!             strcmp(PQerrorMessage(conn), "fe_sendauth: no password supplied\n") == 0 &&
              !feof(stdin))
          {
              PQfinish(conn);
--- 103,109 ----
          }

          if (PQstatus(conn) == CONNECTION_BAD &&
!             strcmp(PQerrorMessage(conn), PQnoPasswordSupplied) == 0 &&
              !feof(stdin))
          {
              PQfinish(conn);
Index: src/interfaces/libpq/fe-auth.c
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/fe-auth.c,v
retrieving revision 1.93
diff -c -c -r1.93 fe-auth.c
*** src/interfaces/libpq/fe-auth.c    28 Sep 2004 00:06:02 -0000    1.93
--- src/interfaces/libpq/fe-auth.c    16 Oct 2004 03:00:18 -0000
***************
*** 634,640 ****
              if (password == NULL || *password == '\0')
              {
                  (void) snprintf(PQerrormsg, PQERRORMSG_LENGTH,
!                                 "fe_sendauth: no password supplied\n");
                  return STATUS_ERROR;
              }
              if (pg_password_sendauth(conn, password, areq) != STATUS_OK)
--- 634,640 ----
              if (password == NULL || *password == '\0')
              {
                  (void) snprintf(PQerrormsg, PQERRORMSG_LENGTH,
!                                 PQnoPasswordSupplied);
                  return STATUS_ERROR;
              }
              if (pg_password_sendauth(conn, password, areq) != STATUS_OK)
Index: src/interfaces/libpq/libpq-fe.h
===================================================================
RCS file: /cvsroot/pgsql/src/interfaces/libpq/libpq-fe.h,v
retrieving revision 1.108
diff -c -c -r1.108 libpq-fe.h
*** src/interfaces/libpq/libpq-fe.h    29 Aug 2004 05:07:00 -0000    1.108
--- src/interfaces/libpq/libpq-fe.h    16 Oct 2004 03:00:19 -0000
***************
*** 398,403 ****
--- 398,406 ----
  /* Exists for backward compatibility.  bjm 2003-03-24 */
  #define PQfreeNotify(ptr) PQfreemem(ptr)

+ /* Define the string so all uses are consistent. */
+ #define PQnoPasswordSupplied    "fe_sendauth: no password supplied\n"
+
  /*
   * Make an empty PGresult with given status (some apps find this
   * useful). If conn is not NULL and status indicates an error, the