Re: [PATCH] Re: [BUGS] BUG #7815: Upgrading PostgreSQL from 9.1 to 9.2 with pg_upgrade/postgreql-setup fails - invalid status retrieve

[ fixed hackers CC address ]

On Mon, Aug 12, 2013 at 12:31:10PM +0200, Pavel Raiskup wrote:
> The latest update on original bug report at Red Hat bugzilla shows that the
> reproducer is just disable the peer access for 'postgres' user in
> pg_hba.conf.  So — the old server was most probably still running for OP
> (not shut down properly as was originally said).
>
> But basically, this fix is relevant to this thread so posting here.
>
> 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8< 8<
>
> >From f2df7fb281b6346f3feeb5f0f8d2d0ee7fb13f6c Mon Sep 17 00:00:00 2001
> From: Pavel Raiskup <praiskup@redhat.com>
> Date: Mon, 12 Aug 2013 10:45:08 +0200
> Subject: [PATCH] pg_upgrade: stop postmaster properly
>
> Setup the os_info.running_cluster little bit earlier just to allow the
> stop_postmaster_atexit callback success when error occurs during the
> authentication checks.
>
> https://bugzilla.redhat.com/show_bug.cgi?id=896161
> http://www.postgresql.org/message-id/E1TwKHs-0005yp-39@wrigleys.postgresql.org
> ---
>  contrib/pg_upgrade/server.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/contrib/pg_upgrade/server.c b/contrib/pg_upgrade/server.c
> index c1d459d..c082d0e 100644
> --- a/contrib/pg_upgrade/server.c
> +++ b/contrib/pg_upgrade/server.c
> @@ -239,6 +239,8 @@ start_postmaster(ClusterInfo *cluster, bool throw_error)
>      if (!pg_ctl_return && !throw_error)
>          return false;
>
> +    os_info.running_cluster = cluster;
> +
>      /* Check to see if we can connect to the server; if not, report it. */
>      if ((conn = get_db_conn(cluster, "template1")) == NULL ||
>          PQstatus(conn) != CONNECTION_OK)
> @@ -258,8 +260,6 @@ start_postmaster(ClusterInfo *cluster, bool throw_error)
>          pg_log(PG_FATAL, "pg_ctl failed to start the %s server, or connection failed\n",
>                 CLUSTER_NAME(cluster));
>
> -    os_info.running_cluster = cluster;
> -
>      return true;
>  }

Wow, this is an interesting bug report, and it certainly is a bug.  What
is complex is the solution.

The existing code is written so we only shutdown the server on exit when
we know we have started the server _and_ can connect to it.  As you
stated, this leaves the server running in cases where we _did_ start the
server, but can't connect to it, e.g. used MD5 authentication.

The patch moves the atexit setting up, as you suggested, but only does
that when pg_ctl succeeds (we know we started the server), because we
don't want to stop the server on exit if we didn't start it.  PG 9.1+
will allow pg_ctl -w start to succeed even if there are permissions
problems;  earlier versions will not and will keep the server running
--- the user will have to stop the server after pg_upgrade says it is
running.

I am not going to backpatch this beyond 9.3 as it is risky code.  I have
improved the comments in this area.

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

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