Обсуждение: 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 <bruce@momjian.us> 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 +
Вложения
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 <bruce@momjian.us> 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 +
On Fri, Jan 5, 2018 at 9:11 PM, Bruce Momjian <bruce@momjian.us> 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
On Mon, Jan 8, 2018 at 10:43:00AM -0500, Robert Haas wrote: > On Fri, Jan 5, 2018 at 9:11 PM, Bruce Momjian <bruce@momjian.us> 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 <bruce@momjian.us> 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 +
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 <bruce@momjian.us> 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 <bruce@momjian.us> 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 +