On Sat, Jan 19, 2013 at 11:27:28AM -0500, Tom Lane wrote:
> Bruce Momjian <[email protected]> writes:
> > Why is a clean shutdown important? If the server crashed, we would have
> > committed transactions in the WAL files which are not transfered to the
> > new server, and would be lost.
>
> > I am hesistant to even start such an old server because pg_upgrade never
> > modifies the old server. Even starting it in that case would be
> > modifying it.
>
> I'm not really following this logic. If the old cluster was in a
> crashed state, why would we not expect that starting a postmaster would
> be the best (only) way to repair the damage and make everything good
> again? Isn't that exactly what the user would have to do anyway? What
> other action would you expect him to take instead?
>
> (But, at least with the type of packaging I'm using in Fedora, he would
> first have to go through a package downgrade/reinstallation process,
> because the packaging provides no simple scripted way of manually
> starting the old postgres executable, only the new one. Moreover, what
> pg_upgrade is printing provides no help in figuring out whether that's
> the next step.)
>
> I do sympathize with taking a paranoid attitude here, but I'm failing
> to see what advantage there is in not attempting to start the old
> postmaster. In the *only* case that pg_upgrade is successfully
> protecting against with this logic, namely there's-an-active-postmaster-
> already, the postmaster is equally able to protect itself. In other
> cases it would be more helpful not less to let the postmaster analyze
> the situation.
>
> > The other problem is that if the server start fails, how do we know if
> > the failure was due to a running postmaster?
>
> Because we read the postmaster's log file, or at least tell the user to
> do so. That report would be unambiguous, unlike pg_upgrade's.
Attached is a WIP patch to give you an idea of how I am going to solve
this problem. This comment says it all:
! /*
! * If we have a postmaster.pid file, try to start the server. If
! * it starts, the pid file was stale, so stop the server. If it
! * doesn't start, assume the server is running.
! */
--
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/check.c b/contrib/pg_upgrade/check.c
new file mode 100644
index 1780788..25cf23c
*** a/contrib/pg_upgrade/check.c
--- b/contrib/pg_upgrade/check.c
*************** check_and_dump_old_cluster(bool live_che
*** 78,84 ****
/* -- OLD -- */
if (!live_check)
! start_postmaster(&old_cluster);
set_locale_and_encoding(&old_cluster);
--- 78,84 ----
/* -- OLD -- */
if (!live_check)
! start_postmaster(&old_cluster, true);
set_locale_and_encoding(&old_cluster);
*************** issue_warnings(char *sequence_script_fil
*** 201,207 ****
/* old = PG 8.3 warnings? */
if (GET_MAJOR_VERSION(old_cluster.major_version) <= 803)
{
! start_postmaster(&new_cluster);
/* restore proper sequence values using file created from old server */
if (sequence_script_file_name)
--- 201,207 ----
/* old = PG 8.3 warnings? */
if (GET_MAJOR_VERSION(old_cluster.major_version) <= 803)
{
! start_postmaster(&new_cluster, true);
/* restore proper sequence values using file created from old server */
if (sequence_script_file_name)
*************** issue_warnings(char *sequence_script_fil
*** 224,230 ****
/* Create dummy large object permissions for old < PG 9.0? */
if (GET_MAJOR_VERSION(old_cluster.major_version) <= 804)
{
! start_postmaster(&new_cluster);
new_9_0_populate_pg_largeobject_metadata(&new_cluster, false);
stop_postmaster(false);
}
--- 224,230 ----
/* Create dummy large object permissions for old < PG 9.0? */
if (GET_MAJOR_VERSION(old_cluster.major_version) <= 804)
{
! start_postmaster(&new_cluster, true);
new_9_0_populate_pg_largeobject_metadata(&new_cluster, false);
stop_postmaster(false);
}
diff --git a/contrib/pg_upgrade/exec.c b/contrib/pg_upgrade/exec.c
new file mode 100644
index e326a10..2b3c203
*** a/contrib/pg_upgrade/exec.c
--- b/contrib/pg_upgrade/exec.c
*************** exec_prog(const char *log_file, const ch
*** 99,104 ****
--- 99,106 ----
fclose(log);
result = system(cmd);
+ if (result != -1)
+ result = WEXITSTATUS(result);
umask(old_umask);
diff --git a/contrib/pg_upgrade/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c
new file mode 100644
index 85997e5..e329cb5
*** a/contrib/pg_upgrade/pg_upgrade.c
--- b/contrib/pg_upgrade/pg_upgrade.c
*************** main(int argc, char **argv)
*** 95,101 ****
/* -- NEW -- */
! start_postmaster(&new_cluster);
check_new_cluster();
report_clusters_compatible();
--- 95,101 ----
/* -- NEW -- */
! start_postmaster(&new_cluster, true);
check_new_cluster();
report_clusters_compatible();
*************** main(int argc, char **argv)
*** 116,122 ****
/* New now using xids of the old system */
/* -- NEW -- */
! start_postmaster(&new_cluster);
prepare_new_databases();
--- 116,122 ----
/* New now using xids of the old system */
/* -- NEW -- */
! start_postmaster(&new_cluster, true);
prepare_new_databases();
*************** setup(char *argv0, bool live_check)
*** 191,203 ****
/* no postmasters should be running */
if (!live_check && is_server_running(old_cluster.pgdata))
! pg_log(PG_FATAL, "There seems to be a postmaster servicing the old cluster.\n"
! "Please shutdown that postmaster and try again.\n");
/* same goes for the new postmaster */
if (is_server_running(new_cluster.pgdata))
! pg_log(PG_FATAL, "There seems to be a postmaster servicing the new cluster.\n"
"Please shutdown that postmaster and try again.\n");
/* get path to pg_upgrade executable */
if (find_my_exec(argv0, exec_path) < 0)
--- 191,218 ----
/* no postmasters should be running */
if (!live_check && is_server_running(old_cluster.pgdata))
! {
! /*
! * If we have a postmaster.pid file, try to start the server. If
! * it starts, the pid file was stale, so stop the server. If it
! * doesn't start, assume the server is running.
! */
! if (start_postmaster(&old_cluster, false))
! stop_postmaster(false);
! else
! pg_log(PG_FATAL, "There seems to be a postmaster servicing the old cluster.\n"
! "Please shutdown that postmaster and try again.\n");
! }
/* same goes for the new postmaster */
if (is_server_running(new_cluster.pgdata))
! {
! if (start_postmaster(&new_cluster, false))
! stop_postmaster(false);
! else
! pg_log(PG_FATAL, "There seems to be a postmaster servicing the new cluster.\n"
"Please shutdown that postmaster and try again.\n");
+ }
/* get path to pg_upgrade executable */
if (find_my_exec(argv0, exec_path) < 0)
diff --git a/contrib/pg_upgrade/pg_upgrade.h b/contrib/pg_upgrade/pg_upgrade.h
new file mode 100644
index d5c3fa9..768c5cb
*** a/contrib/pg_upgrade/pg_upgrade.h
--- b/contrib/pg_upgrade/pg_upgrade.h
*************** __attribute__((format(PG_PRINTF_ATTRIBUT
*** 422,428 ****
char *cluster_conn_opts(ClusterInfo *cluster);
! void start_postmaster(ClusterInfo *cluster);
void stop_postmaster(bool fast);
uint32 get_major_server_version(ClusterInfo *cluster);
void check_pghost_envvar(void);
--- 422,428 ----
char *cluster_conn_opts(ClusterInfo *cluster);
! bool start_postmaster(ClusterInfo *cluster, bool throw_error);
void stop_postmaster(bool fast);
uint32 get_major_server_version(ClusterInfo *cluster);
void check_pghost_envvar(void);
diff --git a/contrib/pg_upgrade/server.c b/contrib/pg_upgrade/server.c
new file mode 100644
index 0b48251..ddad4d4
*** a/contrib/pg_upgrade/server.c
--- b/contrib/pg_upgrade/server.c
*************** stop_postmaster_atexit(void)
*** 170,177 ****
}
! void
! start_postmaster(ClusterInfo *cluster)
{
char cmd[MAXPGPATH * 4 + 1000];
PGconn *conn;
--- 170,177 ----
}
! bool
! start_postmaster(ClusterInfo *cluster, bool throw_error)
{
char cmd[MAXPGPATH * 4 + 1000];
PGconn *conn;
*************** start_postmaster(ClusterInfo *cluster)
*** 236,241 ****
--- 236,244 ----
false,
"%s", cmd);
+ if (!throw_error && !pg_ctl_return)
+ 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)
*************** start_postmaster(ClusterInfo *cluster)
*** 256,261 ****
--- 259,266 ----
CLUSTER_NAME(cluster));
os_info.running_cluster = cluster;
+
+ return true;
}
--
Sent via pgsql-bugs mailing list ([email protected])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs
