Invalid pg_upgrade error message during live check
Pg_upgrade is able to run in --check mode when the old server is still running. Unfortunately, all supported versions of pg_upgrade generate an incorrect (but harmless) "failure" message when doing this: $ pg_upgrade -b /u/pgsql.old/bin -B /u/pgsql/bin \ -d /u/pgsql.old/data/ -D /u/pgsql/data --link --check --> *failure* --> Consult the last few lines of "pg_upgrade_server.log" for --> the probable cause of the failure. Performing Consistency Checks on Old Live Server Checking cluster versions ok Checking database user is the install user ok Checking database connection settings ok Checking for prepared transactions ok Checking for reg* data types in user tables ok Checking for contrib/isn with bigint-passing mismatch ok Checking for invalid "unknown" user columns ok Checking for hash indexes ok Checking for roles starting with "pg_" ok Checking for presence of required libraries ok Checking database user is the install user ok Checking for prepared transactions ok *Clusters are compatible* Embarrassingly, I found out about this bug when watching a presentation in Frankfurt: PostgreSQL-Upgrade: Best Practices https://www.ittage.informatik-aktuell.de/programm/2017/postgresql-upgrade-best-practices/ and there is a blog entry about it too: https://blog.dbi-services.com/does-pg_upgrade-in-check-mode-raises-a-failure-when-the-old-cluster-is-running/ (Yeah, it stinks to be me. :-) ) So, I looked into the code and it turns out that start_postmaster() needs to run exec_prog() in a way that allows it to fail and continue _without_ generating an error message. There are other places that need to run exec_prog() and allow it to fail and continue _and_ generate an error message. To fix this, I modified the exec_prog() API to separately control reporting and exiting-on errors. The attached patch does this and generates the proper output: $ pg_upgrade -b /u/pgsql.old/bin -B /u/pgsql/bin \ -d /u/pgsql.old/data/ -D /u/pgsql/data --link --check Performing Consistency Checks on Old Live Server Checking cluster versions ok Checking database user is the install user ok Checking database connection settings ok Checking for prepared transactions ok Checking for reg* data types in user tables ok Checking for contrib/isn with bigint-passing mismatch ok Checking for invalid "unknown" user columns ok Checking for hash indexes ok Checking for roles starting with "pg_" ok Checking for presence of required libraries ok Checking database user is the install user ok Checking for prepared transactions ok *Clusters are compatible* -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription + diff --git a/src/bin/pg_upgrade/dump.c b/src/bin/pg_upgrade/dump.c new file mode 100644 index 5ed6b78..8a662e9 *** a/src/bin/pg_upgrade/dump.c --- b/src/bin/pg_upgrade/dump.c *** generate_old_dump(void) *** 23,29 prep_status("Creating dump of global objects"); /* run new pg_dumpall binary for globals */ ! exec_prog(UTILITY_LOG_FILE, NULL, true, "\"%s/pg_dumpall\" %s --globals-only --quote-all-identifiers " "--binary-upgrade %s -f %s", new_cluster.bindir, cluster_conn_opts(&old_cluster), --- 23,29 prep_status("Creating dump of global objects"); /* run new pg_dumpall binary for globals */ ! exec_prog(UTILITY_LOG_FILE, NULL, true, true, "\"%s/pg_dumpall\" %s --globals-only --quote-all-identifiers " "--binary-upgrade %s -f %s", new_cluster.bindir, cluster_conn_opts(&old_cluster), diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c new file mode 100644 index ea45434..9122e27 *** a/src/bin/pg_upgrade/exec.c --- b/src/bin/pg_upgrade/exec.c *** get_bin_version(ClusterInfo *cluster) *** 71,86 * and attempts to execute that command. If the command executes * successfully, exec_prog() returns true. * ! * If the command fails, an error message i
Invalid pg_upgrade error message during live check
Pg_upgrade is able to run in --check mode when the old server is still running. Unfortunately, all supported versions of pg_upgrade generate an incorrect (but harmless) "failure" message when doing this: $ pg_upgrade -b /u/pgsql.old/bin -B /u/pgsql/bin \ -d /u/pgsql.old/data/ -D /u/pgsql/data --link --check --> *failure* --> Consult the last few lines of "pg_upgrade_server.log" for --> the probable cause of the failure. Performing Consistency Checks on Old Live Server Checking cluster versions ok Checking database user is the install user ok Checking database connection settings ok Checking for prepared transactions ok Checking for reg* data types in user tables ok Checking for contrib/isn with bigint-passing mismatch ok Checking for invalid "unknown" user columns ok Checking for hash indexes ok Checking for roles starting with "pg_" ok Checking for presence of required libraries ok Checking database user is the install user ok Checking for prepared transactions ok *Clusters are compatible* Embarrassingly, I found out about this bug when watching a presentation in Frankfurt: PostgreSQL-Upgrade: Best Practices https://www.ittage.informatik-aktuell.de/programm/2017/postgresql-upgrade-best-practices/ and there is a blog entry about it too: https://blog.dbi-services.com/does-pg_upgrade-in-check-mode-raises-a-failure-when-the-old-cluster-is-running/ (Yeah, it stinks to be me. :-) ) So, I looked into the code and it turns out that start_postmaster() needs to run exec_prog() in a way that allows it to fail and continue _without_ generating an error message. There are other places that need to run exec_prog() and allow it to fail and continue _and_ generate an error message. To fix this, I modified the exec_prog() API to separately control reporting and exiting-on errors. The attached patch does this and generates the proper output: $ pg_upgrade -b /u/pgsql.old/bin -B /u/pgsql/bin \ -d /u/pgsql.old/data/ -D /u/pgsql/data --link --check Performing Consistency Checks on Old Live Server Checking cluster versions ok Checking database user is the install user ok Checking database connection settings ok Checking for prepared transactions ok Checking for reg* data types in user tables ok Checking for contrib/isn with bigint-passing mismatch ok Checking for invalid "unknown" user columns ok Checking for hash indexes ok Checking for roles starting with "pg_" ok Checking for presence of required libraries ok Checking database user is the install user ok Checking for prepared transactions ok *Clusters are compatible* -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription + diff --git a/src/bin/pg_upgrade/dump.c b/src/bin/pg_upgrade/dump.c new file mode 100644 index 5ed6b78..8a662e9 *** a/src/bin/pg_upgrade/dump.c --- b/src/bin/pg_upgrade/dump.c *** generate_old_dump(void) *** 23,29 prep_status("Creating dump of global objects"); /* run new pg_dumpall binary for globals */ ! exec_prog(UTILITY_LOG_FILE, NULL, true, "\"%s/pg_dumpall\" %s --globals-only --quote-all-identifiers " "--binary-upgrade %s -f %s", new_cluster.bindir, cluster_conn_opts(&old_cluster), --- 23,29 prep_status("Creating dump of global objects"); /* run new pg_dumpall binary for globals */ ! exec_prog(UTILITY_LOG_FILE, NULL, true, true, "\"%s/pg_dumpall\" %s --globals-only --quote-all-identifiers " "--binary-upgrade %s -f %s", new_cluster.bindir, cluster_conn_opts(&old_cluster), diff --git a/src/bin/pg_upgrade/exec.c b/src/bin/pg_upgrade/exec.c new file mode 100644 index ea45434..9122e27 *** a/src/bin/pg_upgrade/exec.c --- b/src/bin/pg_upgrade/exec.c *** get_bin_version(ClusterInfo *cluster) *** 71,86 * and attempts to execute that command. If the command executes * successfully, exec_prog() returns true. * ! * If the command fails, an error message i
Re: Invalid pg_upgrade error message during live check
On Fri, Jan 5, 2018 at 04:58:39PM -0500, Bruce Momjian wrote: > Pg_upgrade is able to run in --check mode when the old server is still > running. Unfortunately, all supported versions of pg_upgrade generate > an incorrect (but harmless) "failure" message when doing this: Based on recent discussions, I am thinking we would just fix this in PG 10 and HEAD and leave the harmless error message in the other branches, right? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: Invalid pg_upgrade error message during live check
On Fri, Jan 5, 2018 at 9:11 PM, Bruce Momjian wrote: > On Fri, Jan 5, 2018 at 04:58:39PM -0500, Bruce Momjian wrote: >> Pg_upgrade is able to run in --check mode when the old server is still >> running. Unfortunately, all supported versions of pg_upgrade generate >> an incorrect (but harmless) "failure" message when doing this: > > Based on recent discussions, I am thinking we would just fix this in PG > 10 and HEAD and leave the harmless error message in the other branches, > right? Hmm, not sure why you say that. If the message is buggy and the fix isn't too risky, might as well fix it all the way back. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Invalid pg_upgrade error message during live check
On Mon, Jan 8, 2018 at 10:43:00AM -0500, Robert Haas wrote: > On Fri, Jan 5, 2018 at 9:11 PM, Bruce Momjian wrote: > > On Fri, Jan 5, 2018 at 04:58:39PM -0500, Bruce Momjian wrote: > >> Pg_upgrade is able to run in --check mode when the old server is still > >> running. Unfortunately, all supported versions of pg_upgrade generate > >> an incorrect (but harmless) "failure" message when doing this: > > > > Based on recent discussions, I am thinking we would just fix this in PG > > 10 and HEAD and leave the harmless error message in the other branches, > > right? > > Hmm, not sure why you say that. If the message is buggy and the fix > isn't too risky, might as well fix it all the way back. OK, I will do that then. The message is harmless, but confusing, so I would like to fix it, but it requires changing the API of two functions. I will see how cleanly it applies back to 9.3 and act accordingly. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: Invalid pg_upgrade error message during live check
On Mon, Jan 8, 2018 at 06:26:21PM -0500, Bruce Momjian wrote: > On Mon, Jan 8, 2018 at 10:43:00AM -0500, Robert Haas wrote: > > On Fri, Jan 5, 2018 at 9:11 PM, Bruce Momjian wrote: > > > On Fri, Jan 5, 2018 at 04:58:39PM -0500, Bruce Momjian wrote: > > >> Pg_upgrade is able to run in --check mode when the old server is still > > >> running. Unfortunately, all supported versions of pg_upgrade generate > > >> an incorrect (but harmless) "failure" message when doing this: > > > > > > Based on recent discussions, I am thinking we would just fix this in PG > > > 10 and HEAD and leave the harmless error message in the other branches, > > > right? > > > > Hmm, not sure why you say that. If the message is buggy and the fix > > isn't too risky, might as well fix it all the way back. > > OK, I will do that then. The message is harmless, but confusing, so I > would like to fix it, but it requires changing the API of two functions. > I will see how cleanly it applies back to 9.3 and act accordingly. I ended up just doing HEAD and PG10. When you change a function API, you have to change every call site, and that can cause the patch to be large and make backpatching too far difficult, which was the case for this patch. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: Invalid pg_upgrade error message during live check
On Fri, Jan 5, 2018 at 04:58:39PM -0500, Bruce Momjian wrote: > Pg_upgrade is able to run in --check mode when the old server is still > running. Unfortunately, all supported versions of pg_upgrade generate > an incorrect (but harmless) "failure" message when doing this: Based on recent discussions, I am thinking we would just fix this in PG 10 and HEAD and leave the harmless error message in the other branches, right? -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: Invalid pg_upgrade error message during live check
On Fri, Jan 5, 2018 at 9:11 PM, Bruce Momjian wrote: > On Fri, Jan 5, 2018 at 04:58:39PM -0500, Bruce Momjian wrote: >> Pg_upgrade is able to run in --check mode when the old server is still >> running. Unfortunately, all supported versions of pg_upgrade generate >> an incorrect (but harmless) "failure" message when doing this: > > Based on recent discussions, I am thinking we would just fix this in PG > 10 and HEAD and leave the harmless error message in the other branches, > right? Hmm, not sure why you say that. If the message is buggy and the fix isn't too risky, might as well fix it all the way back. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Re: Invalid pg_upgrade error message during live check
On Mon, Jan 8, 2018 at 10:43:00AM -0500, Robert Haas wrote: > On Fri, Jan 5, 2018 at 9:11 PM, Bruce Momjian wrote: > > On Fri, Jan 5, 2018 at 04:58:39PM -0500, Bruce Momjian wrote: > >> Pg_upgrade is able to run in --check mode when the old server is still > >> running. Unfortunately, all supported versions of pg_upgrade generate > >> an incorrect (but harmless) "failure" message when doing this: > > > > Based on recent discussions, I am thinking we would just fix this in PG > > 10 and HEAD and leave the harmless error message in the other branches, > > right? > > Hmm, not sure why you say that. If the message is buggy and the fix > isn't too risky, might as well fix it all the way back. OK, I will do that then. The message is harmless, but confusing, so I would like to fix it, but it requires changing the API of two functions. I will see how cleanly it applies back to 9.3 and act accordingly. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +
Re: Invalid pg_upgrade error message during live check
On Mon, Jan 8, 2018 at 06:26:21PM -0500, Bruce Momjian wrote: > On Mon, Jan 8, 2018 at 10:43:00AM -0500, Robert Haas wrote: > > On Fri, Jan 5, 2018 at 9:11 PM, Bruce Momjian wrote: > > > On Fri, Jan 5, 2018 at 04:58:39PM -0500, Bruce Momjian wrote: > > >> Pg_upgrade is able to run in --check mode when the old server is still > > >> running. Unfortunately, all supported versions of pg_upgrade generate > > >> an incorrect (but harmless) "failure" message when doing this: > > > > > > Based on recent discussions, I am thinking we would just fix this in PG > > > 10 and HEAD and leave the harmless error message in the other branches, > > > right? > > > > Hmm, not sure why you say that. If the message is buggy and the fix > > isn't too risky, might as well fix it all the way back. > > OK, I will do that then. The message is harmless, but confusing, so I > would like to fix it, but it requires changing the API of two functions. > I will see how cleanly it applies back to 9.3 and act accordingly. I ended up just doing HEAD and PG10. When you change a function API, you have to change every call site, and that can cause the patch to be large and make backpatching too far difficult, which was the case for this patch. -- Bruce Momjian http://momjian.us EnterpriseDB http://enterprisedb.com + As you are, so once was I. As I am, so you will be. + + Ancient Roman grave inscription +