[ 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 <[email protected]>
> 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/[email protected]
> ---
> 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 <[email protected]> http://momjian.us
EnterpriseDB http://enterprisedb.com
+ It's impossible for everything to be true. +
diff --git a/contrib/pg_upgrade/server.c b/contrib/pg_upgrade/server.c
new file mode 100644
index c1d459d..94fc35e
*** a/contrib/pg_upgrade/server.c
--- b/contrib/pg_upgrade/server.c
*************** static void
*** 166,172 ****
stop_postmaster_atexit(void)
{
stop_postmaster(true);
-
}
--- 166,171 ----
*************** start_postmaster(ClusterInfo *cluster, b
*** 236,245 ****
false,
"%s", cmd);
if (!pg_ctl_return && !throw_error)
return false;
! /* 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)
{
--- 235,261 ----
false,
"%s", cmd);
+ /* Did it fail and we are just testing if the server could be started? */
if (!pg_ctl_return && !throw_error)
return false;
! /*
! * We set this here to make sure atexit() shuts down the server,
! * but only if we started the server successfully. We do it
! * before checking for connectivity in case the server started but
! * there is a connectivity failure. If pg_ctl did not return success,
! * we will exit below.
! */
! if (pg_ctl_return)
! os_info.running_cluster = cluster;
!
! /*
! * pg_ctl -w might have failed because the server couldn't be started,
! * or there might have been a connection problem in _checking_ if the
! * server has started. Therefore, even if pg_ctl failed, we continue
! * and test for connectivity in case we get a connection reason for the
! * failure.
! */
if ((conn = get_db_conn(cluster, "template1")) == NULL ||
PQstatus(conn) != CONNECTION_OK)
{
*************** start_postmaster(ClusterInfo *cluster, b
*** 253,265 ****
}
PQfinish(conn);
! /* If the connection didn't fail, fail now */
if (!pg_ctl_return)
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;
}
--- 269,282 ----
}
PQfinish(conn);
! /*
! * If pg_ctl failed, and the connection didn't fail, and throw_error is
! * enabled, fail now. This could happen if the server was already running.
! */
if (!pg_ctl_return)
pg_log(PG_FATAL, "pg_ctl failed to start the %s server, or connection failed\n",
CLUSTER_NAME(cluster));
return true;
}
--
Sent via pgsql-hackers mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
