Re: pg_migrator issue with contrib

I've been kicking the tires on this a bit, and I've found an issue when
dealing with contrib/ (specifically dblink, although I haven't looked
around anymore).

dblink_current_query() is not in the 8.4 version - when I run
pg_migrator on an 8.3 cluster that has dblink installed, I get the
following:

Restoring database schema
psql:/home/postgres/pg_migrator_dump_db.sql:271: ERROR: could not find
function "dblink_current_query" in file
"/opt/dbs/pgsql84-beta2/lib/dblink.so"

There were problems executing "/opt/dbs/pgsql84-beta2/bin/psql" --set
ON_ERROR_STOP=on --port 5432 -f "/home/postgres/pg_migrator_dump_db.sql"
--dbname template1 >> "/dev/null"

pg_migrator exits leaving me with a corrupted 8.3 instance.

At the very least, a mention in the documentation of incompatible
contrib module(s) would be nice. Even better would be a sanity check
added to prevent this.
--
Brad Nicholson 416-673-4106
Database Administrator, Afilias Canada Corp.

Brad Nicholson wrote:
> I've been kicking the tires on this a bit, and I've found an issue when
> dealing with contrib/ (specifically dblink, although I haven't looked
> around anymore).
>
> dblink_current_query() is not in the 8.4 version - when I run
> pg_migrator on an 8.3 cluster that has dblink installed, I get the
> following:
>
> Restoring database schema
> psql:/home/postgres/pg_migrator_dump_db.sql:271: ERROR: could not find
> function "dblink_current_query" in file
> "/opt/dbs/pgsql84-beta2/lib/dblink.so"
>
> There were problems executing "/opt/dbs/pgsql84-beta2/bin/psql" --set
> ON_ERROR_STOP=on --port 5432 -f "/home/postgres/pg_migrator_dump_db.sql"
> --dbname template1 >> "/dev/null"

Yep, pg_migrator will exit on any restore error. This really relates to
the problem of how we handle /contrib schema migration from one
release to the other. Good thing you posted to hackers because that is
really the group that can address this. pg_migrator is really just
restoring the schema that pg_dump is producing.

> pg_migrator exits leaving me with a corrupted 8.3 instance.

When you say "corrupted", I assume you mean you have remove the _old
suffixes to restart your 8.3 instance, right? I hope that is the only
corruption issue --- please confirm.

> At the very least, a mention in the documentation of incompatible
> contrib module(s) would be nice. Even better would be a sanity check
> added to prevent this.

OK, I am looking to the hackers group for recommentations on this. I
wonder if I should recommend uninstalling /contrib modules before the
upgrade, but that is not possible for custom data types that have
columns already defined in the old cluster.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
>> At the very least, a mention in the documentation of incompatible
>> contrib module(s) would be nice. Even better would be a sanity check
>> added to prevent this.
>>
>
> OK, I am looking to the hackers group for recommentations on this. I
> wonder if I should recommend uninstalling /contrib modules before the
> upgrade, but that is not possible for custom data types that have
> columns already defined in the old cluster.
>
>

There is no nice answer to this. It goes way beyond data types: you
could be using the module stuff in indexes, functions, views etc. You
can't just drop the stuff. The best I have been able to do in similar
cases is to install the updated module in the database before restoring,
and ignore any restoration errors about "foo already exists" or "foo not
found in .so file". Not sure how well that translates to pg_migrator,
though.

cheers

andrew

On Fri, 2009-06-05 at 15:50 -0400, Bruce Momjian wrote:
> Brad Nicholson wrote:
> > I've been kicking the tires on this a bit, and I've found an issue when
> > dealing with contrib/ (specifically dblink, although I haven't looked
> > around anymore).
> >
> > dblink_current_query() is not in the 8.4 version - when I run
> > pg_migrator on an 8.3 cluster that has dblink installed, I get the
> > following:
> >
> > Restoring database schema
> > psql:/home/postgres/pg_migrator_dump_db.sql:271: ERROR: could not find
> > function "dblink_current_query" in file
> > "/opt/dbs/pgsql84-beta2/lib/dblink.so"
> >
> > There were problems executing "/opt/dbs/pgsql84-beta2/bin/psql" --set
> > ON_ERROR_STOP=on --port 5432 -f "/home/postgres/pg_migrator_dump_db.sql"
> > --dbname template1 >> "/dev/null"
>
> Yep, pg_migrator will exit on any restore error. This really relates to
> the problem of how we handle /contrib schema migration from one
> release to the other. Good thing you posted to hackers because that is
> really the group that can address this. pg_migrator is really just
> restoring the schema that pg_dump is producing.
>
> > pg_migrator exits leaving me with a corrupted 8.3 instance.
>
> When you say "corrupted", I assume you mean you have remove the _old
> suffixes to restart your 8.3 instance, right? I hope that is the only
> corruption issue --- please confirm.

Unfortunately no - when I try and start the old version, I get:

pg_ctl -D pgsql83/ start
postgres: could not find the database system
Expected to find it in the directory "/opt/DATA/pgsql83",
but could not open file "/opt/DATA/pgsql83/global/pg_control": No such
file or directory

I am specifying both new and old paths to pg_migrator, as I don't have
my data directories in standard locations.

Test case is pretty straight forward.

Create an 8.3 instance
Add dblink
follow through the pg_migrator upgrade path
try and start your 8.3 instance after the failure

--
Brad Nicholson 416-673-4106
Database Administrator, Afilias Canada Corp.

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Brad Nicholson wrote:
>> At the very least, a mention in the documentation of incompatible
>> contrib module(s) would be nice. Even better would be a sanity check
>> added to prevent this.

> OK, I am looking to the hackers group for recommentations on this.

One thing I was going to suggest is that pg_migrator check that all the
.so's in the old installation also exist in the new one. However that
could be overkill since you don't know for sure if the old .so is
referenced anywhere in the database.

As for the specific problem at hand, it might've been a mistake to
replace dblink_current_query() with a SQL function instead of changing
the internal implementation of the C function. We could still fix that.

regards, tom lane

Brad Nicholson wrote:
> > When you say "corrupted", I assume you mean you have remove the _old
> > suffixes to restart your 8.3 instance, right? I hope that is the only
> > corruption issue --- please confirm.
>
> Unfortunately no - when I try and start the old version, I get:
>
> pg_ctl -D pgsql83/ start
> postgres: could not find the database system
> Expected to find it in the directory "/opt/DATA/pgsql83",
> but could not open file "/opt/DATA/pgsql83/global/pg_control": No such
> file or directory
>
> I am specifying both new and old paths to pg_migrator, as I don't have
> my data directories in standard locations.
>
> Test case is pretty straight forward.
>
> Create an 8.3 instance
> Add dblink
> follow through the pg_migrator upgrade path
> try and start your 8.3 instance after the failure

Uh, I assume you read all of the INSTALL instructions, including the
last item:

10. Reverting to old cluster

...

--> If you ran pg_migrator _without_ --link or did not start the new server,
the old cluster was not modified except that an ".old" suffix was
appended to $PGDATA/global/pg_control and tablespaces directories. To
reuse the old cluster, remove the tablespace directories created by the
new cluster and remove the ".old" suffix from the old cluster tablespace
directory names and $PGDATA/global/pg_control; then you can restart the
old cluster.

I just modified the arrow text to be clearer. The rename of
pg_controldata is done to prevent accidental starting of the old cluster
in case of migration success.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Andrew Dunstan wrote:
>
>
> Bruce Momjian wrote:
> >> At the very least, a mention in the documentation of incompatible
> >> contrib module(s) would be nice. Even better would be a sanity check
> >> added to prevent this.
> >>
> >
> > OK, I am looking to the hackers group for recommentations on this. I
> > wonder if I should recommend uninstalling /contrib modules before the
> > upgrade, but that is not possible for custom data types that have
> > columns already defined in the old cluster.
> >
> >
>
> There is no nice answer to this. It goes way beyond data types: you
> could be using the module stuff in indexes, functions, views etc. You
> can't just drop the stuff. The best I have been able to do in similar
> cases is to install the updated module in the database before restoring,
> and ignore any restoration errors about "foo already exists" or "foo not
> found in .so file". Not sure how well that translates to pg_migrator,
> though.

I suspected this answer but figured I would get a definative answer
rather than guessing.

Based on the way pg_migrator works I am going to suggest that if
/contrib restore generates an error that they uninstall the /contrib
from the old cluster and retry.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Brad Nicholson wrote:
> >> At the very least, a mention in the documentation of incompatible
> >> contrib module(s) would be nice. Even better would be a sanity check
> >> added to prevent this.
>
> > OK, I am looking to the hackers group for recommentations on this.
>
> One thing I was going to suggest is that pg_migrator check that all the
> .so's in the old installation also exist in the new one. However that
> could be overkill since you don't know for sure if the old .so is
> referenced anywhere in the database.

Or in this case that the new *.so has the same functions.

> As for the specific problem at hand, it might've been a mistake to
> replace dblink_current_query() with a SQL function instead of changing
> the internal implementation of the C function. We could still fix that.

I am afraid /contrib is going to be a mine field for this type of
problem so I am going to recommend uninstaling the /contrib module if
possible and retry the migration. That should work in this case.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> > There is no nice answer to this. It goes way beyond data types: you
> > could be using the module stuff in indexes, functions, views etc. You
> > can't just drop the stuff. The best I have been able to do in similar
> > cases is to install the updated module in the database before restoring,
> > and ignore any restoration errors about "foo already exists" or "foo not
> > found in .so file". Not sure how well that translates to pg_migrator,
> > though.
>
> I suspected this answer but figured I would get a definative answer
> rather than guessing.
>
> Based on the way pg_migrator works I am going to suggest that if
> /contrib restore generates an error that they uninstall the /contrib
> from the old cluster and retry.

I have added the following paragraph to the pg_migrator INSTALL docs:

If an error occurs while restoring the database schema, pg_migrator will
exit and you will have to revert to the old cluster as outlined in step
#10 below. To try pg_migrator again, you will need to modify the old
cluster so the pg_migrator schema restore succeeds. If the problem is a
/contrib module, you might need to uninstall the /contrib module from
the old cluster and install it in the new cluster after migration.

The only other idea I have would be to add a switch that allows
schema restore errors to be ignored, but I would like to see if the
above text is enough first.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Hi,

Ah, we need module/extension/package/plugin so badly...

Le 5 juin 09 à 22:19, Bruce Momjian a écrit :
> I am afraid /contrib is going to be a mine field for this type of
> problem so I am going to recommend uninstaling the /contrib module if
> possible and retry the migration. That should work in this case.

You can't seriously recommend that, I'm afraid.
As Andrew (Dunstan) was saying up-thread, the faulty module (from
contrib or pgfoundry) could hold some indexes (btree, gist, gin) and/
or data types in the cluster relations.

So if you uninstall the module (drop type cascade, drop operator
class, ...) you lose data.

Some example modules that I can think of and are wildspread in the
field, as far as I know, are ip4r (data type and indexes), orafce
(functions, views, tables), and some less spread are prefix (data type
and indexes) or temporal (period data type, indexes).

Please do not recommend people to lose their precious data to be able
to upgrade. You could tell them pg_migrator isn't an option in their
case, though. At least we're left with a faster (multi-threaded)
pg_restore :)

Regards,
--
dim

Bruce,

Assuming a contrib module *hasn't* changed its API, does pg_migrator
link against the 8.4 version of the module's .so, or the old 8.3 version?

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

On Fri, Jun 5, 2009 at 6:11 PM, Dimitri Fontaine <dfontaine(at)hi-media(dot)com> wrote:
> Some example modules that I can think of and are wildspread in the field, as
> far as I know, are ip4r (data type and indexes), orafce (functions, views,
> tables), and some less spread are prefix (data type and indexes) or temporal
> (period data type, indexes).

And hstore...

David Blewett

Josh Berkus wrote:
> Bruce,
>
> Assuming a contrib module *hasn't* changed its API, does pg_migrator
> link against the 8.4 version of the module's .so, or the old 8.3 version?

8.4 version, or whatever is in the 8.4 lib, which should be 8.4.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Dimitri Fontaine wrote:
> Hi,
>
> Ah, we need module/extension/package/plugin so badly...
>
> Le 5 juin 09 ? 22:19, Bruce Momjian a ?crit :
> > I am afraid /contrib is going to be a mine field for this type of
> > problem so I am going to recommend uninstaling the /contrib module if
> > possible and retry the migration. That should work in this case.
>
> You can't seriously recommend that, I'm afraid.
> As Andrew (Dunstan) was saying up-thread, the faulty module (from
> contrib or pgfoundry) could hold some indexes (btree, gist, gin) and/
> or data types in the cluster relations.
>
> So if you uninstall the module (drop type cascade, drop operator
> class, ...) you lose data.
>
> Some example modules that I can think of and are wildspread in the
> field, as far as I know, are ip4r (data type and indexes), orafce
> (functions, views, tables), and some less spread are prefix (data type
> and indexes) or temporal (period data type, indexes).
>
> Please do not recommend people to lose their precious data to be able
> to upgrade. You could tell them pg_migrator isn't an option in their
> case, though. At least we're left with a faster (multi-threaded)
> pg_restore :)

Very good point, and something I had not considered. I tried
uninstalling hstore and it gladly dropped any hstore columns!

I have updated the INSTALL instructions:

If an error occurs while restoring the database schema, pg_migrator will
exit and you will have to revert to the old cluster as outlined in step
#10 below. To try pg_migrator again, you will need to modify the old
cluster so the pg_migrator schema restore succeeds. If the problem is a
/contrib module, you might need to uninstall the /contrib module from
the old cluster and install it in the new cluster after the migration,
assuming the module is not being used to store user data.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On 6/5/09 6:27 PM, Bruce Momjian wrote:
> Josh Berkus wrote:
>> Bruce,
>>
>> Assuming a contrib module *hasn't* changed its API, does pg_migrator
>> link against the 8.4 version of the module's .so, or the old 8.3 version?
>
> 8.4 version, or whatever is in the 8.4 lib, which should be 8.4.

So, here's what we need for 8.3 --> 8.4 for contrib modules:

1) make a list of contrib modules which do not convert cleanly (testers?)
2) document these.
3) give pg_migrator some hackish way to install 8.4 contrib modules from
source before copying over the database.
4) set pg_migrator to ignore duplicate object warnings if it does the above.

Note that we expect NOT to have this issue for 8.4-->8.5, since we'll
have a full module infrastructure by then. Really!

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

Josh Berkus <josh(at)agliodbs(dot)com> writes:
> So, here's what we need for 8.3 --> 8.4 for contrib modules:

> 1) make a list of contrib modules which do not convert cleanly (testers?)
> 2) document these.
> 3) give pg_migrator some hackish way to install 8.4 contrib modules from
> source before copying over the database.
> 4) set pg_migrator to ignore duplicate object warnings if it does the above.

1A) consider whether we can reduce/eliminate the conversion problems.
We have five days.

I don't think we need testing, per se. The first step should be to diff
the 8.3 and 8.4 versions of the various contrib .sql.in files and
determine what changed. Any module whose .sql.in file hasn't changed
is definitely safe.

regards, tom lane

On 6 Jun 2009, at 19:50, Tom Lane wrote:
> We have five days.
>
> I don't think we need testing, per se. The first step should be to
> diff
> the 8.3 and 8.4 versions of the various contrib .sql.in files and
> determine what changed. Any module whose .sql.in file hasn't changed
> is definitely safe.

I can tell you already that dblink has changed, and I had to drop it
before migration, otherwise everything went fine. Migration of 57GB
data in place took about 1 minute.

Le 6 juin 09 à 20:45, Josh Berkus a écrit :
> So, here's what we need for 8.3 --> 8.4 for contrib modules:

That does nothing for external modules whose code isn't in PostgreSQL
control. I'm thinking of those examples I cited up-thread --- and some
more. (ip4r, temporal, prefix, hstore-new, oracfe, etc).

Could pg_migrator detect usage of "objects" oids (data types in
relation, index opclass, ...) that are unknown to be in the standard -
core + contrib distribution, and quit trying to upgrade the cluster in
this case, telling the user his database is not supported?

> Note that we expect NOT to have this issue for 8.4-->8.5, since
> we'll have a full module infrastructure by then. Really!

Note: added in-place upgrade to requirements of the first version of
the feature.
--
dim

Dimitri Fontaine wrote:
> Le 6 juin 09 ? 20:45, Josh Berkus a ?crit :
> > So, here's what we need for 8.3 --> 8.4 for contrib modules:
>
> That does nothing for external modules whose code isn't in PostgreSQL
> control. I'm thinking of those examples I cited up-thread --- and some
> more. (ip4r, temporal, prefix, hstore-new, oracfe, etc).

Agreed, that's why the new INSTALL text addresses this clearly. You
might want to read my blog entry about why the INSTALL file is so
important for pg_migrator:

http://momjian.us/main/blogs/pgblog.html#June_6_2009

> Could pg_migrator detect usage of "objects" oids (data types in
> relation, index opclass, ...) that are unknown to be in the standard -
> core + contrib distribution, and quit trying to upgrade the cluster in
> this case, telling the user his database is not supported?

Well, they will get an error and see the INSTALL file --- I don't see
having pg_migrator go around looking for things as a fruitful effort.

> > Note that we expect NOT to have this issue for 8.4-->8.5, since
> > we'll have a full module infrastructure by then. Really!
>
> Note: added in-place upgrade to requirements of the first version of
> the feature.

Yes, this will certainly spur development of a better /contrib install
system.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Grzegorz Jaskiewicz wrote:
>
> On 6 Jun 2009, at 19:50, Tom Lane wrote:
> > We have five days.
> >
> > I don't think we need testing, per se. The first step should be to
> > diff
> > the 8.3 and 8.4 versions of the various contrib .sql.in files and
> > determine what changed. Any module whose .sql.in file hasn't changed
> > is definitely safe.
>
> I can tell you already that dblink has changed, and I had to drop it
> before migration, otherwise everything went fine. Migration of 57GB
> data in place took about 1 minute.

The good news is that the INSTALL instructions where clear enough for
the tester to understand that uninstalling dblink from the old cluster
and reinstalling it in the new cluster would work.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:
> Josh Berkus <josh(at)agliodbs(dot)com> writes:
> > So, here's what we need for 8.3 --> 8.4 for contrib modules:
>
> > 1) make a list of contrib modules which do not convert cleanly (testers?)
> > 2) document these.
> > 3) give pg_migrator some hackish way to install 8.4 contrib modules from
> > source before copying over the database.
> > 4) set pg_migrator to ignore duplicate object warnings if it does the above.
>
> 1A) consider whether we can reduce/eliminate the conversion problems.
> We have five days.
>
> I don't think we need testing, per se. The first step should be to diff
> the 8.3 and 8.4 versions of the various contrib .sql.in files and
> determine what changed. Any module whose .sql.in file hasn't changed
> is definitely safe.

I am a little hesistant to do one-off adjustments for /contrib changes
between versions. The INSTALL instructions are pretty clear of how to
fix problems like dblink.

A more complex case would be if hstore couldn't be migrated, and there
is no way to drop that type and reinstall on the new system without
dropping your data too, but again, I don't see how we have any way to
fix that for 8.4 anyway.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Josh Berkus wrote:
> On 6/5/09 6:27 PM, Bruce Momjian wrote:
> > Josh Berkus wrote:
> >> Bruce,
> >>
> >> Assuming a contrib module *hasn't* changed its API, does pg_migrator
> >> link against the 8.4 version of the module's .so, or the old 8.3 version?
> >
> > 8.4 version, or whatever is in the 8.4 lib, which should be 8.4.
>
> So, here's what we need for 8.3 --> 8.4 for contrib modules:
>
> 1) make a list of contrib modules which do not convert cleanly (testers?)
> 2) document these.
> 3) give pg_migrator some hackish way to install 8.4 contrib modules from
> source before copying over the database.
> 4) set pg_migrator to ignore duplicate object warnings if it does the above.
>
> Note that we expect NOT to have this issue for 8.4-->8.5, since we'll
> have a full module infrastructure by then. Really!

I think the cleanest solution is to document that these issues might
happen and suggest solutions. This has already worked for our tester
today so I am hopeful people will just figure out how to fix this.

I have added dblink as a specific migration example to the INSTALL file:

---------------------------------------------------------------------------

If an error occurs while restoring the database schema, pg_migrator will
exit and you will have to revert to the old cluster as outlined in step
#10 below. To try pg_migrator again, you will need to modify the old
cluster so the pg_migrator schema restore succeeds. If the problem is a
/contrib module, you might need to uninstall the /contrib module from
the old cluster and install it in the new cluster after the migration,
assuming the module is not being used to store user data.

For example, /contrib/dblink changed its API from Postgres 8.3 to 8.4 so
sites using it must uninstall dblink from their Postgres 8.3, cluster,
do the migration, then install dblink in Postgres 8.4. More complex
cases might require individual objects to be migrated manually.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> I think the cleanest solution is to document that these issues might
> happen and suggest solutions.

No, the cleanest solution is to fix it before people ever see a
problem. This is trivial to do in the case of dblink and I don't
see why you think it would be better to make users work around it.

Also, dblink is one of the easiest cases because (a) it doesn't have
anything but functions, and thus it's possible to drop it from the
old DB without data loss, and (b) the inconsistency that it has is
something that will result in a clean, readily understandable failure
during migration. As opposed to some other cases that will migrate
just fine and then dump core during use.

I've just finished running through a diff of 8.3 vs 8.4 contrib
SQL files. It looks like we have these cases:

* Quite a few modules have differences in the signatures of GIST and
GIN opclass support functions. As was discussed earlier, we don't
really have to fix those, because the SQL-level signatures don't
affect anything at runtime. Most of these modules also have RECHECK
flags that were removed, but we have a solution for that too.

* dblink has an entirely unnecessary change in the definition of
dblink_current_query(), which will cause it to fail to migrate.

* fuzzystrmatch has added a new levenshtein() function. The worst
case if this is migrated without any extra effort is the user
doesn't have access to that function. But it's a functions-only
module, so the user could fix it by just rerunning the SQL file
afterwards.

* intarray has removed its internal @> and <@ operators. As I was
mentioning the other day, it might be the best thing to just revert
that change anyway, until we can get a better handle on the behavior
of GIN for empty arrays.

* isn has got the nastiest problems of the lot: since it piggybacks
on type bigint, a migrated database might work, or might crash
miserably, depending on whether bigint has become pass-by-value
in the new database. I'm not very sure if we can fix this reasonably.

* pageinspect has changed the ABI of get_raw_page() in a way that will
likely make it dump core if the function definition is migrated from
an old DB. This needs to be fixed. It's also added a new function
fsm_page_contents(bytea), which is no big problem, since the SQL
file can be re-executed without harm.

* pg_buffercache has changed the view pg_buffercache, which is
definitely going to be a migration issue. Need to test whether
it represents a crash risk if the old definition is migrated.

* pg_freespacemap has made *major* changes in its ABI. There's
probably no hope of this working either, but we need to be sure
it's not a crash risk.

* pgstattuple has made changes in the output types of its functions.
This is a serious crash risk, and I'm not immediately sure how to
fix it. Note that simply migrating the module will not draw any
errors.

* seg has added a new function seg_center(seg). This can be fixed
by reloading the SQL file, but not by dropping the module beforehand
since that would risk data loss.

* tsearch2 has opclass support function changes, but unlike other
cases of this, it will fail to migrate to 8.4 because the functions
are references to core functions instead of being declared in the
module. Also, "drop it first" isn't a very useful recommendation
since the domains it defines might be used somewhere.

So we've definitely got some work here. If we do nothing, several
of these issues are going to end up with CVE numbers.

regards, tom lane

On 7 Jun 2009, at 03:27, Bruce Momjian wrote:

> Grzegorz Jaskiewicz wrote:
>>
>> On 6 Jun 2009, at 19:50, Tom Lane wrote:
>>> We have five days.
>>>
>>> I don't think we need testing, per se. The first step should be to
>>> diff
>>> the 8.3 and 8.4 versions of the various contrib .sql.in files and
>>> determine what changed. Any module whose .sql.in file hasn't
>>> changed
>>> is definitely safe.
>>
>> I can tell you already that dblink has changed, and I had to drop it
>> before migration, otherwise everything went fine. Migration of 57GB
>> data in place took about 1 minute.
>
> The good news is that the INSTALL instructions where clear enough for
> the tester to understand that uninstalling dblink from the old cluster
> and reinstalling it in the new cluster would work.
Yes, but I forgot about one database, and had to do it all over again,
luckily I first tested it without --link...

On Sun, Jun 7, 2009 at 12:11 AM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> * intarray has removed its internal @> and <@ operators.  As I was
> mentioning the other day, it might be the best thing to just revert
> that change anyway, until we can get a better handle on the behavior
> of GIN for empty arrays.

+1.

> So we've definitely got some work here.  If we do nothing, several
> of these issues are going to end up with CVE numbers.

I think it's becoming increasingly clear that pg_migrator is not for
the faint of heart; in fact, I think we should hedge references to it
with words like "experimental". If we set an expectation that this
tool has the same level of maturity and reliability that people
associate with PostgreSQL in general, we are going to have a lot of
disappointed users. Just to recall the history, the first pgfoundry
commit messages for this tool were on February 9th, three months after
the start of the final CommitFest and feature freeze for 8.4. Since
then, development has proceeded at an amazingly rapid pace, but
there's only so much you can do in four months, especially when it's
too late to rearchitect previously-committed 8.4 features to play more
nicely with the new feature being added.

It seems to me that if we keep plugging at this problem, 8.4->8.5
migration has the potential to be considerably smoother than 8.3 to
8.4 migration (and, yes, a good module facility will help), but it
wouldn't be surprising to me if we're well into 9.x territory before
we really get all of the issues hammered out. I don't think even a
major feature like Hot Standby has the far-reaching implications on
how the system needs to be designed that upgrade-in-place does.

So while I agree with Tom that we should fix as many of these issues
as we reasonably well can for 8.4, I also agree with Bruce that a lot
of this comes down to setting appropriate expectations for our users:
this is a potentially useful tool, especially for users with huge
databases, but it's new, and it's not perfect, and use with caution.

...Robert

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I think it's becoming increasingly clear that pg_migrator is not for
> the faint of heart; in fact, I think we should hedge references to it
> with words like "experimental".

Probably ...

> Just to recall the history, the first pgfoundry
> commit messages for this tool were on February 9th, three months after
> the start of the final CommitFest and feature freeze for 8.4. Since
> then, development has proceeded at an amazingly rapid pace, but
> there's only so much you can do in four months,

... but the above is a *complete* misrepresentation of the thing's
history (apparently you failed to look in the Attic?). EDB have been
using previous versions of this tool for some years, and the basic
premise is the same as contrib/pg_upgrade that existed as far back as
7.1/7.2 timeframe.

regards, tom lane

On Jun 7, 2009, at 10:04 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> I think it's becoming increasingly clear that pg_migrator is not for
>> the faint of heart; in fact, I think we should hedge references to it
>> with words like "experimental".
>
> Probably ...
>
>> Just to recall the history, the first pgfoundry
>> commit messages for this tool were on February 9th, three months
>> after
>> the start of the final CommitFest and feature freeze for 8.4. Since
>> then, development has proceeded at an amazingly rapid pace, but
>> there's only so much you can do in four months,
>
> ... but the above is a *complete* misrepresentation of the thing's
> history (apparently you failed to look in the Attic?). EDB have been
> using previous versions of this tool for some years, and the basic
> premise is the same as contrib/pg_upgrade that existed as far back as
> 7.1/7.2 timeframe.

I did know that EDB had been using the tool for a while, but I admit
I'm not familiar with the whole history. I had the impression that
we'd gotten a lot more serious about really making this rock solid
since Bruce took it in February. But maybe that's not the case?

...Robert

Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> I think it's becoming increasingly clear that pg_migrator is not for
>> the faint of heart; in fact, I think we should hedge references to it
>> with words like "experimental".
>
> Probably ...

I'm with Robert on that one - while pg_migrator is extremely inmportant
for us to go forward. I really think we need to tag it "experimental"
for this release at least. pg_migrator is complex and we are still
discovering new issues every day I don't think rushing it as _THE_
solution will do any good for our reputation. A lot of our code(as
software in general) took years to mature and pg_migrator is likely not
an exception.

>
>> Just to recall the history, the first pgfoundry
>> commit messages for this tool were on February 9th, three months after
>> the start of the final CommitFest and feature freeze for 8.4. Since
>> then, development has proceeded at an amazingly rapid pace, but
>> there's only so much you can do in four months,
>
> ... but the above is a *complete* misrepresentation of the thing's
> history (apparently you failed to look in the Attic?). EDB have been
> using previous versions of this tool for some years, and the basic
> premise is the same as contrib/pg_upgrade that existed as far back as
> 7.1/7.2 timeframe.

well - how much field exposure has pg_migrator/edb_migrator seen
actually? given the large number of "breaks your database" bugs and
issues that got found during the last few weeks I have a hard time
imaging that edb really used the current code for their customers.

Stefan

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> I did know that EDB had been using the tool for a while, but I admit
> I'm not familiar with the whole history. I had the impression that
> we'd gotten a lot more serious about really making this rock solid
> since Bruce took it in February. But maybe that's not the case?

I don't actually know the EDB end of the history either; maybe someone
can educate us about that. But it's true that the core developers,
at least, weren't taking it seriously until this year. That's because
it really can only handle catalog changes, not changes to the contents
of user tables; and it's been quite a long time since we've had a
release where we didn't change tuple header layout or packing rules or
something that made it a nonstarter. It wasn't clear till early this
year that 8.3->8.4 would be a cycle where pg_migrator had a chance of
being useful in production ... so we got serious about it.

(I do not know whether EDB ever really used it in production. If they
did, it must have been for private updates that changed catalogs and
not user data.)

regards, tom lane

I wrote:
> * pageinspect has changed the ABI of get_raw_page() in a way that will
> likely make it dump core if the function definition is migrated from
> an old DB. This needs to be fixed.
> [ and similarly for some other contrib modules ]

After thinking about this some more, I think that there is a fairly
simple coding rule we can adopt to prevent post-migration crashes
of the sort I'm worrying about above. That is:

* If you change the ABI of a C-language function, change its C name.

This will ensure that if someone tries to migrate the old function
definition from an old database, they will get a pg_migrator failure,
or at worst a clean runtime failure when they attempt to use the old
definition. They won't get a core dump or some worse form of security
problem.

As an example, the problem in pageinspect is this diff:

***************
*** 6,16 ****
--
-- get_raw_page()
--
! CREATE OR REPLACE FUNCTION get_raw_page(text, int4)
RETURNS bytea
AS 'MODULE_PATHNAME', 'get_raw_page'
LANGUAGE C STRICT;

--
-- page_header()
--
--- 6,21 ----
--
-- get_raw_page()
--
! CREATE OR REPLACE FUNCTION get_raw_page(text, text, int4)
RETURNS bytea
AS 'MODULE_PATHNAME', 'get_raw_page'
LANGUAGE C STRICT;

+ CREATE OR REPLACE FUNCTION get_raw_page(text, int4)
+ RETURNS bytea
+ AS $$ SELECT get_raw_page($1, 'main', $2); $$
+ LANGUAGE SQL STRICT;
+
--
-- page_header()
--
***************

The underlying C-level get_raw_page function is still there, but
it now expects three arguments not two, and will crash if it's
passed an int4 where it's expecting a text argument. But the old
function definition will migrate without error --- there's no way
for pg_migrator to realize it's installing a security hazard.

The way we should have done this, which I intend to go change it to,
is something like

CREATE OR REPLACE FUNCTION get_raw_page(text, int4)
RETURNS bytea
AS 'MODULE_PATHNAME', 'get_raw_page'
LANGUAGE C STRICT;

CREATE OR REPLACE FUNCTION get_raw_page(text, text, int4)
RETURNS bytea
AS 'MODULE_PATHNAME', 'get_raw_page_3'
LANGUAGE C STRICT;

so that the old function's ABI is preserved. Migration of the old
contrib module will then lead to the 3-argument function not being
immediately available, but the 2-argument one still works. Had we not
wanted to keep the 2-argument form for some reason, we would have
provided only get_raw_page_3 in the .so file, and attempts to use the
old function definition would fail safely.

(We have actually seen similar problems before with people trying
to dump and reload database containing contrib modules. pg_migrator
is not creating a problem that wasn't there before, it's just making
it worse.)

Comments, better ideas?

regards, tom lane

Grzegorz Jaskiewicz wrote:
>
> On 7 Jun 2009, at 03:27, Bruce Momjian wrote:
>
> > Grzegorz Jaskiewicz wrote:
> >>
> >> On 6 Jun 2009, at 19:50, Tom Lane wrote:
> >>> We have five days.
> >>>
> >>> I don't think we need testing, per se. The first step should be to
> >>> diff
> >>> the 8.3 and 8.4 versions of the various contrib .sql.in files and
> >>> determine what changed. Any module whose .sql.in file hasn't
> >>> changed
> >>> is definitely safe.
> >>
> >> I can tell you already that dblink has changed, and I had to drop it
> >> before migration, otherwise everything went fine. Migration of 57GB
> >> data in place took about 1 minute.
> >
> > The good news is that the INSTALL instructions where clear enough for
> > the tester to understand that uninstalling dblink from the old cluster
> > and reinstalling it in the new cluster would work.
> Yes, but I forgot about one database, and had to do it all over again,
> luckily I first tested it without --link...

Any failure would have allowed you to revert to the old server. It is
only when you _start_ the new server that the old server cannot be used;
I have clarified the INSTALL file on that point.

If you use linking, the migration will be much faster (no data copying),
but you will no longer be able to access your old cluster once you start
the new cluster after the upgrade.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Jun 7, 2009, at 12:03 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> I did know that EDB had been using the tool for a while, but I admit
>> I'm not familiar with the whole history. I had the impression that
>> we'd gotten a lot more serious about really making this rock solid
>> since Bruce took it in February. But maybe that's not the case?
>
> I don't actually know the EDB end of the history either; maybe someone
> can educate us about that. But it's true that the core developers,
> at least, weren't taking it seriously until this year. That's because
> it really can only handle catalog changes, not changes to the contents
> of user tables; and it's been quite a long time since we've had a
> release where we didn't change tuple header layout or packing rules or
> something that made it a nonstarter. It wasn't clear till early this
> year that 8.3->8.4 would be a cycle where pg_migrator had a chance of
> being useful in production ... so we got serious about it.

OK, that's more or less what I thought, and what I intended to convey
upthread. As far as core Postgres is concerned this is a new feature,
and we haven't worked out all the kinks yet. As long as we set
expectations accordingly, I think that's OK. You mention CVEs for
these contrib issues, but will CVEs still be issued if we make clear
that this is experimental only? I would hope not, since that would
amount to a policy that any half-baked code anywhere on pgfoundry is
just as much our responsibility as core Postgres. Surely we're
allowed to say "good progress has been made here, but we harbor no
illusions that it's bullet-proof."

...Robert

On 6/7/09 10:56 AM, Robert Haas wrote:
> OK, that's more or less what I thought, and what I intended to convey
> upthread. As far as core Postgres is concerned this is a new feature,
> and we haven't worked out all the kinks yet.

Yes, I'm calling it "pg_migrator beta" in any advocacy/PR about it.
AFAIC, until we have these sorts of issues worked out, it's still a beta.

--
Josh Berkus
PostgreSQL Experts Inc.
www.pgexperts.com

Josh Berkus wrote:
> On 6/7/09 10:56 AM, Robert Haas wrote:
>> OK, that's more or less what I thought, and what I intended to convey
>> upthread. As far as core Postgres is concerned this is a new feature,
>> and we haven't worked out all the kinks yet.
>
> Yes, I'm calling it "pg_migrator beta" in any advocacy/PR about it.
> AFAIC, until we have these sorts of issues worked out, it's still a beta.

afaiks bruce stated he is going to remove the BETA tag from pg_migrator
soon so I guess calling it beta in the main project docs will confuse
the hell out of people(or causing them to think that it is not beta any
more).
So maybe calling it experimental(from the POV of the main project) or
something similiar might still be the better solution.

Stefan

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > I think the cleanest solution is to document that these issues might
> > happen and suggest solutions.
>
> No, the cleanest solution is to fix it before people ever see a
> problem. This is trivial to do in the case of dblink and I don't
> see why you think it would be better to make users work around it.
>
> Also, dblink is one of the easiest cases because (a) it doesn't have
> anything but functions, and thus it's possible to drop it from the
> old DB without data loss, and (b) the inconsistency that it has is
> something that will result in a clean, readily understandable failure
> during migration. As opposed to some other cases that will migrate
> just fine and then dump core during use.
>
> I've just finished running through a diff of 8.3 vs 8.4 contrib
> SQL files. It looks like we have these cases:

[ list removed]

Certainly if you can fix /contrib problems at the source, please do so.
I was commenting on the idea of having pg_migrator somehow skip specific
items to try to make it more failure-proof. While I can do that for a
few cases, such as suppress the creation of specific functions by
filtering the schema dump file, I will never be able to get them all,
and doing it extensively could destabilize pg_migrator.

You are suggesting improving /contrib itself, which is better.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:
> The underlying C-level get_raw_page function is still there, but
> it now expects three arguments not two, and will crash if it's
> passed an int4 where it's expecting a text argument. But the old
> function definition will migrate without error --- there's no way
> for pg_migrator to realize it's installing a security hazard.

FYI, there is nothing pg_migrator specific here. Someone doing a
dump/reload from 8.3 to 8.4 would have the same security issue.
pg_migrator is using the same pg_dump output as a dump restore, except
it uses --schema. pg_migrator would actually be more secure because it
will exit on the restore error rather than having the error possibly
ignored by the user.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Robert Haas wrote:
> OK, that's more or less what I thought, and what I intended to convey
> upthread. As far as core Postgres is concerned this is a new feature,
> and we haven't worked out all the kinks yet. As long as we set
> expectations accordingly, I think that's OK. You mention CVEs for
> these contrib issues, but will CVEs still be issued if we make clear
> that this is experimental only? I would hope not, since that would
> amount to a policy that any half-baked code anywhere on pgfoundry is
> just as much our responsibility as core Postgres. Surely we're
> allowed to say "good progress has been made here, but we harbor no
> illusions that it's bullet-proof."

Again, there is nothing pg_migrator-specific about these security
issues.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Stefan Kaltenbrunner wrote:
> Josh Berkus wrote:
> > On 6/7/09 10:56 AM, Robert Haas wrote:
> >> OK, that's more or less what I thought, and what I intended to convey
> >> upthread. As far as core Postgres is concerned this is a new feature,
> >> and we haven't worked out all the kinks yet.
> >
> > Yes, I'm calling it "pg_migrator beta" in any advocacy/PR about it.
> > AFAIC, until we have these sorts of issues worked out, it's still a beta.
>
> afaiks bruce stated he is going to remove the BETA tag from pg_migrator
> soon so I guess calling it beta in the main project docs will confuse
> the hell out of people(or causing them to think that it is not beta any
> more).
> So maybe calling it experimental(from the POV of the main project) or
> something similar might still be the better solution.

This all sounds very discouraging. It is like, "Oh, my, there is a
migration tool and it might have bugs. How do we prevent people from
using it?"

Right now nothing in the project is referring to pg_migrator except in
the press release, and it is marked as beta there. How do you want to
deemphasize it more than that? Why did I bother working on this if the
community reaction is to try to figure out how to make people avoid
using it?

I am now thinking I need to my own PR for pg_migrator because obviously
the community is only worried it might have a bug. Instead of testing
it, looking at the code, submitting bug reports, or anything
constructive, you sit around figuring out how to put a disparaging label
on it!

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> > I did know that EDB had been using the tool for a while, but I admit
> > I'm not familiar with the whole history. I had the impression that
> > we'd gotten a lot more serious about really making this rock solid
> > since Bruce took it in February. But maybe that's not the case?
>
> I don't actually know the EDB end of the history either; maybe someone
> can educate us about that. But it's true that the core developers,
> at least, weren't taking it seriously until this year. That's because
> it really can only handle catalog changes, not changes to the contents
> of user tables; and it's been quite a long time since we've had a
> release where we didn't change tuple header layout or packing rules or
> something that made it a nonstarter. It wasn't clear till early this
> year that 8.3->8.4 would be a cycle where pg_migrator had a chance of
> being useful in production ... so we got serious about it.
>
> (I do not know whether EDB ever really used it in production. If they
> did, it must have been for private updates that changed catalogs and
> not user data.)

pg_migrator verion 0.5 is still on the pg_migrator web site, and that is
the version I started from. It had this line in the intro:

PG_migrator is a tool (not a complete solution) that performs an
in-place upgrade of existing data.

Of course no one wants a toolkit, they want a full solution, so I
modified the code to be easier to use and more robust. I am not sure
how much EDB used it.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Sun, Jun 7, 2009 at 11:50 PM, Bruce Momjian<bruce(at)momjian(dot)us> wrote:
> Stefan Kaltenbrunner wrote:
>> Josh Berkus wrote:
>> > On 6/7/09 10:56 AM, Robert Haas wrote:
>> >> OK, that's more or less what I thought, and what I intended to convey
>> >> upthread.  As far as core Postgres is concerned this is a new feature,
>> >> and we haven't worked out all the kinks yet.
>> >
>> > Yes, I'm calling it "pg_migrator beta" in any advocacy/PR about it.
>> > AFAIC, until we have these sorts of issues worked out, it's still a beta.
>>
>> afaiks bruce stated he is going to remove the BETA tag from pg_migrator
>> soon so I guess calling it beta in the main project docs will confuse
>> the hell out of people(or causing them to think that it is not beta any
>> more).
>> So maybe calling it experimental(from the POV of the main project) or
>> something similar might still be the better solution.
>
> This all sounds very discouraging.  It is like, "Oh, my, there is a
> migration tool and it might have bugs.  How do we prevent people from
> using it?"
>
> Right now nothing in the project is referring to pg_migrator except in
> the press release, and it is marked as beta there.  How do you want to
> deemphasize it more than that?  Why did I bother working on this if the
> community reaction is to try to figure out how to make people avoid
> using it?

Because Rome wasn't built in a day.

It seems to me that you yourself placed a far more disparaging label
on it than anything that anyone has proposed today; this was a week
ago:

http://archives.postgresql.org/pgsql-hackers/2009-05/msg01470.php

I don't think it's anyone's intention to disparage your work on this
tool. It certainly isn't mine. But it seems obvious to me that it
has some pretty severe limitations and warts. The fact that those
limitations and warts are well-documented doesn't negate their
existence. I also don't think calling the software "beta" or
"experimental" is a way of deemphasizing it. I think it's a way of
being clear that this software is not the bullet-proof, rock-solid,
handles-all-cases-and-keeps-on-trucking level of robustness that
people have come to expect from PostgreSQL.

FWIW, I have no problem at all with mentioning pg_migrator in the
release notes or the documentation; my failure to respond to your last
emails on this topic was due to being busy and having already spent
too much time responding to other emails, not due to thinking it was a
bad idea. I actually think it's a good idea. But I also think those
references should describe it as experimental, because I think it is.
I really hope it won't remain experimental forever, but I think that's
an accurate characterization of where it is now.

You, or others, may disagree, of course.

...Robert

Robert Haas wrote:
> On Sun, Jun 7, 2009 at 11:50 PM, Bruce Momjian<bruce(at)momjian(dot)us> wrote:
>> Stefan Kaltenbrunner wrote:
>>> Josh Berkus wrote:
>>>> On 6/7/09 10:56 AM, Robert Haas wrote:
>>>>> OK, that's more or less what I thought, and what I intended to convey
>>>>> upthread. As far as core Postgres is concerned this is a new feature,
>>>>> and we haven't worked out all the kinks yet.
>>>> Yes, I'm calling it "pg_migrator beta" in any advocacy/PR about it.
>>>> AFAIC, until we have these sorts of issues worked out, it's still a beta.
>>> afaiks bruce stated he is going to remove the BETA tag from pg_migrator
>>> soon so I guess calling it beta in the main project docs will confuse
>>> the hell out of people(or causing them to think that it is not beta any
>>> more).
>>> So maybe calling it experimental(from the POV of the main project) or
>>> something similar might still be the better solution.
>> This all sounds very discouraging. It is like, "Oh, my, there is a
>> migration tool and it might have bugs. How do we prevent people from
>> using it?"
>>
>> Right now nothing in the project is referring to pg_migrator except in
>> the press release, and it is marked as beta there. How do you want to
>> deemphasize it more than that? Why did I bother working on this if the
>> community reaction is to try to figure out how to make people avoid
>> using it?
>
> Because Rome wasn't built in a day.

indeed

>
> It seems to me that you yourself placed a far more disparaging label
> on it than anything that anyone has proposed today; this was a week
> ago:
>
> http://archives.postgresql.org/pgsql-hackers/2009-05/msg01470.php

well that is way more discouraging than what I wanted to say :)

>
> I don't think it's anyone's intention to disparage your work on this
> tool. It certainly isn't mine. But it seems obvious to me that it
> has some pretty severe limitations and warts. The fact that those
> limitations and warts are well-documented doesn't negate their
> existence. I also don't think calling the software "beta" or
> "experimental" is a way of deemphasizing it. I think it's a way of
> being clear that this software is not the bullet-proof, rock-solid,
> handles-all-cases-and-keeps-on-trucking level of robustness that
> people have come to expect from PostgreSQL.

Exactly my point. pg_migrator gained a lot of momentum in the last weeks
an months, but imho it has still way to go. I do think that binary
upgrades are extremely important for us(that's why I did a fair amount
of testing on it) but I don't think that we should go too far for this
release.
A lot of the code that makes postgresql what it is now took years to
mature on pgfoundry or in contrib. So some of the questions to ask would be:
* is pg_migrator ready for contrib/? Probably not - it is still a way
too moving target so pgfoundry is good
* is pg_migrator ready for /src/bin?

Realistically I think we need to get at least one full cycle to see what
happens in the field with something as complex as pg_migrator to
really get a grasp on what else comes up.

>
> FWIW, I have no problem at all with mentioning pg_migrator in the
> release notes or the documentation; my failure to respond to your last
> emails on this topic was due to being busy and having already spent
> too much time responding to other emails, not due to thinking it was a
> bad idea. I actually think it's a good idea. But I also think those
> references should describe it as experimental, because I think it is.
> I really hope it won't remain experimental forever, but I think that's
> an accurate characterization of where it is now.

yep - I was not against mentioning it either. We just should do it in a
sane way(ie it is not part of the core project yet but endorsed and
might get added in the future or such) so we don't confuse people(like
we call it beta, the homepage does not) and yet get valuable feedback
which we certainly need to go forward.

Stefan

Dimitri Fontaine wrote:
> Le 6 juin 09 à 20:45, Josh Berkus a écrit :
>> So, here's what we need for 8.3 --> 8.4 for contrib modules:
>
> That does nothing for external modules whose code isn't in PostgreSQL
> control. I'm thinking of those examples I cited up-thread --- and some
> more. (ip4r, temporal, prefix, hstore-new, oracfe, etc).

For me and I know several others, the big question would be PostGIS.
Unfortunately I haven't had the time to run any tests myself, so I'll
join the line of people being worried, but I have a number of customers
with somewhere between pretty and very large PostGIS databases that
could really benefit from pg_migrator.

As long as PostGIS is the same version in both of them, is pg_migrator
is likely to work? (one can always run the PostGIS upgrade as a separate
step)

> Could pg_migrator detect usage of "objects" oids (data types in
> relation, index opclass, ...) that are unknown to be in the standard
> -core + contrib distribution, and quit trying to upgrade the cluster in
> this case, telling the user his database is not supported?

+1 on this.

Or at least, have it exit and say "if you know that these things are
reasonably safe, run pg_migrator again with --force" or something like that.

--
Magnus Hagander
Self: http://www.hagander.net/
Work: http://www.redpill-linpro.com/

On Sun, Jun 7, 2009 at 12:36 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> I wrote:
>> * pageinspect has changed the ABI of get_raw_page() in a way that will
>> likely make it dump core if the function definition is migrated from
>> an old DB.  This needs to be fixed.
>> [ and similarly for some other contrib modules ]
>
> After thinking about this some more, I think that there is a fairly
> simple coding rule we can adopt to prevent post-migration crashes
> of the sort I'm worrying about above.  That is:
>
> * If you change the ABI of a C-language function, change its C name.
>
> This will ensure that if someone tries to migrate the old function
> definition from an old database, they will get a pg_migrator failure,
> or at worst a clean runtime failure when they attempt to use the old
> definition.  They won't get a core dump or some worse form of security
> problem.
>
> As an example, the problem in pageinspect is this diff:
>
> ***************
> *** 6,16 ****
>  --
>  -- get_raw_page()
>  --
> ! CREATE OR REPLACE FUNCTION get_raw_page(text, int4)
>  RETURNS bytea
>  AS 'MODULE_PATHNAME', 'get_raw_page'
>  LANGUAGE C STRICT;
>
>  --
>  -- page_header()
>  --
> --- 6,21 ----
>  --
>  -- get_raw_page()
>  --
> ! CREATE OR REPLACE FUNCTION get_raw_page(text, text, int4)
>  RETURNS bytea
>  AS 'MODULE_PATHNAME', 'get_raw_page'
>  LANGUAGE C STRICT;
>
> + CREATE OR REPLACE FUNCTION get_raw_page(text, int4)
> + RETURNS bytea
> + AS $$ SELECT get_raw_page($1, 'main', $2); $$
> + LANGUAGE SQL STRICT;
> +
>  --
>  -- page_header()
>  --
> ***************
>
> The underlying C-level get_raw_page function is still there, but
> it now expects three arguments not two, and will crash if it's
> passed an int4 where it's expecting a text argument.  But the old
> function definition will migrate without error --- there's no way
> for pg_migrator to realize it's installing a security hazard.
>
> The way we should have done this, which I intend to go change it to,
> is something like
>
> CREATE OR REPLACE FUNCTION get_raw_page(text, int4)
> RETURNS bytea
> AS 'MODULE_PATHNAME', 'get_raw_page'
> LANGUAGE C STRICT;
>
> CREATE OR REPLACE FUNCTION get_raw_page(text, text, int4)
> RETURNS bytea
> AS 'MODULE_PATHNAME', 'get_raw_page_3'
> LANGUAGE C STRICT;
>
> so that the old function's ABI is preserved.  Migration of the old
> contrib module will then lead to the 3-argument function not being
> immediately available, but the 2-argument one still works.  Had we not
> wanted to keep the 2-argument form for some reason, we would have
> provided only get_raw_page_3 in the .so file, and attempts to use the
> old function definition would fail safely.
>
> (We have actually seen similar problems before with people trying
> to dump and reload database containing contrib modules.  pg_migrator
> is not creating a problem that wasn't there before, it's just making
> it worse.)
>
> Comments, better ideas?

maybe, get_raw_page_v2, etc? I suppose you could run into situation
with multiple versions of the function w/same # of arguments?

merlin

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> As long as PostGIS is the same version in both of them, is pg_migrator
> is likely to work? (one can always run the PostGIS upgrade as a separate
> step)

There was just some discussion about that on postgis-devel. I think the
conclusion was that you would have to do the PostGIS update as a
separate step. They intend to support both 1.3.x and 1.4.x on current
versions of Postgres for some time, so in principle you could do it in
either order.

>> Could pg_migrator detect usage of "objects" oids (data types in
>> relation, index opclass, ...) that are unknown to be in the standard
>> -core + contrib distribution, and quit trying to upgrade the cluster in
>> this case, telling the user his database is not supported?

> +1 on this.

> Or at least, have it exit and say "if you know that these things are
> reasonably safe, run pg_migrator again with --force" or something like that.

I don't think that anything in that line is going to be helpful.
What it will lead to is people mindlessly using --force (cf our
bad experiences with -i for pg_dump). If you can't give a *useful*
ie trustworthy warning/error, issuing a useless one is not a good
substitute.

regards, tom lane

Tom Lane wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>> As long as PostGIS is the same version in both of them, is pg_migrator
>> is likely to work? (one can always run the PostGIS upgrade as a separate
>> step)
>
> There was just some discussion about that on postgis-devel. I think the
> conclusion was that you would have to do the PostGIS update as a
> separate step. They intend to support both 1.3.x and 1.4.x on current
> versions of Postgres for some time, so in principle you could do it in
> either order.

Doing them as two steps is totally fine with me, because IIRC the
PostGIS upgrades generally don't require hours and hours of downtime.

>>> Could pg_migrator detect usage of "objects" oids (data types in
>>> relation, index opclass, ...) that are unknown to be in the standard
>>> -core + contrib distribution, and quit trying to upgrade the cluster in
>>> this case, telling the user his database is not supported?
>
>> +1 on this.
>
>> Or at least, have it exit and say "if you know that these things are
>> reasonably safe, run pg_migrator again with --force" or something like that.
>
> I don't think that anything in that line is going to be helpful.
> What it will lead to is people mindlessly using --force (cf our
> bad experiences with -i for pg_dump). If you can't give a *useful*
> ie trustworthy warning/error, issuing a useless one is not a good
> substitute.

Well, in that case, error out would be a better option than doing it and
probably fail later. And have a --force option available, but don't
suggest it.

//Magnus

Magnus Hagander <magnus(at)hagander(dot)net> writes:
> Tom Lane wrote:
>> Magnus Hagander <magnus(at)hagander(dot)net> writes:
>>>> Could pg_migrator detect usage of "objects" oids (data types in
>>>> relation, index opclass, ...) that are unknown to be in the standard
>>>> -core + contrib distribution, and quit trying to upgrade the cluster in
>>>> this case, telling the user his database is not supported?
>>
>>> +1 on this.
>>
>>> Or at least, have it exit and say "if you know that these things are
>>> reasonably safe, run pg_migrator again with --force" or something like that.
>>
>> I don't think that anything in that line is going to be helpful.
>> What it will lead to is people mindlessly using --force (cf our
>> bad experiences with -i for pg_dump). If you can't give a *useful*
>> ie trustworthy warning/error, issuing a useless one is not a good
>> substitute.
>
> Well, in that case, error out would be a better option than doing it and
> probably fail later. And have a --force option available, but don't
> suggest it.

My vote would go to detect and error out without recovering option. If
the tool is not able to handle a situation and knows it, I don't see
what would be good about it leting the user lose data on purpose.

The --force option should be for the user to manually drop his columns
and indexes (etc) and try pg_migrator again, which will stop listing
faulty objects but care about the now compatible cluster.

Restoring the lost data is not the job of pg_migrator, of course.

Regards,
--
dim

Dimitri Fontaine <dfontaine(at)hi-media(dot)com> writes:
> My vote would go to detect and error out without recovering option. If
> the tool is not able to handle a situation and knows it, I don't see
> what would be good about it leting the user lose data on purpose.

No, that's not what's being discussed. The proposal is to have it error
out when it *does not* know whether there is a real problem; and, in
fact, when there's only a rather low probability of there being a real
problem. My view is that that's basically counterproductive. It leads
directly to having to have a --force switch and then to people using
that switch carelessly.

regards, tom lane

Robert Haas wrote:
> > Right now nothing in the project is referring to pg_migrator except in
> > the press release, and it is marked as beta there. ?How do you want to
> > deemphasize it more than that? ?Why did I bother working on this if the
> > community reaction is to try to figure out how to make people avoid
> > using it?
>
> Because Rome wasn't built in a day.
>
> It seems to me that you yourself placed a far more disparaging label
> on it than anything that anyone has proposed today; this was a week
> ago:
>
> http://archives.postgresql.org/pgsql-hackers/2009-05/msg01470.php
>
> I don't think it's anyone's intention to disparage your work on this
> tool. It certainly isn't mine. But it seems obvious to me that it
> has some pretty severe limitations and warts. The fact that those
> limitations and warts are well-documented doesn't negate their
> existence. I also don't think calling the software "beta" or
> "experimental" is a way of deemphasizing it. I think it's a way of
> being clear that this software is not the bullet-proof, rock-solid,
> handles-all-cases-and-keeps-on-trucking level of robustness that
> people have come to expect from PostgreSQL.
>
> FWIW, I have no problem at all with mentioning pg_migrator in the
> release notes or the documentation; my failure to respond to your last
> emails on this topic was due to being busy and having already spent
> too much time responding to other emails, not due to thinking it was a
> bad idea. I actually think it's a good idea. But I also think those
> references should describe it as experimental, because I think it is.
> I really hope it won't remain experimental forever, but I think that's
> an accurate characterization of where it is now.

pg_migrator should be looked at critically here, and I agree we should
avoid letting pg_migrator failures reflect badly on Postgres.

Let me list the problems with pg_migrator:

o /contrib and plugin migration (not unique to pg_migrator)
o you must read/follow the install instructions
o might require post-migration table/index rebuilds
o new so serious bugs might exist

and let me list its benefits:

o first in-place upgrade capability in years
o tested by some users, all successful (since late alpha)
o removes major impediment to adoption
o includes extensive error checking and reporting
o contains detailed installation/usage instructions

So let's look at pg_migrator as an opportunity and a risk. As far as I
know, only Hiroshi Saito and I have have looked at the code. Why don't
others read the pg_migrator source code looking for bugs? Why have more
people not test it?

I think "experimental" is the wrong label. Experimental assumes its
usefulness is uncertain and that it is still being researched ---
neither is true. Once I release pg_migrator 8.4 final at the end of
this week (assuming no bugs are reported), I consider it done, or at
least advanced as far as I can go until I get more feedback from users.

I think we can say: "pg_migrator is designed for experienced users with
large databases, for whom the typical dump/restore required for major
version upgrades is a hardship".

I assume this will be the same adoption pattern we had with the Win32
port, where it was a new platform in 8.0 and we dealt with some issues
as it was deployed, and that people who want it will find it and
hopefully it will be useful for them.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Magnus Hagander wrote:
> Dimitri Fontaine wrote:
> > Le 6 juin 09 ? 20:45, Josh Berkus a ?crit :
> >> So, here's what we need for 8.3 --> 8.4 for contrib modules:
> >
> > That does nothing for external modules whose code isn't in PostgreSQL
> > control. I'm thinking of those examples I cited up-thread --- and some
> > more. (ip4r, temporal, prefix, hstore-new, oracfe, etc).
>
> For me and I know several others, the big question would be PostGIS.
> Unfortunately I haven't had the time to run any tests myself, so I'll
> join the line of people being worried, but I have a number of customers
> with somewhere between pretty and very large PostGIS databases that
> could really benefit from pg_migrator.
>
> As long as PostGIS is the same version in both of them, is pg_migrator
> is likely to work? (one can always run the PostGIS upgrade as a separate
> step)

Yes, it should work with the same version of PostGIS, but I have not
tested it. There is nothing special about PostGIS that would cause it
not work work --- we use the same pg_dump as you would for a major
upgrade --- we just move the files around instead of dumping the data.

> > Could pg_migrator detect usage of "objects" oids (data types in
> > relation, index opclass, ...) that are unknown to be in the standard
> > -core + contrib distribution, and quit trying to upgrade the cluster in
> > this case, telling the user his database is not supported?
>
> +1 on this.
>
> Or at least, have it exit and say "if you know that these things are
> reasonably safe, run pg_migrator again with --force" or something like that.

Right now pg_migrator throws an error if the schema load doesn't work.
Assuming you use the same version on the old and new clusters, it should
work fine. I am unclear what checking oids would do.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:
> Magnus Hagander <magnus(at)hagander(dot)net> writes:
> > As long as PostGIS is the same version in both of them, is pg_migrator
> > is likely to work? (one can always run the PostGIS upgrade as a separate
> > step)
>
> There was just some discussion about that on postgis-devel. I think the
> conclusion was that you would have to do the PostGIS update as a
> separate step. They intend to support both 1.3.x and 1.4.x on current
> versions of Postgres for some time, so in principle you could do it in
> either order.

Oh, yea, you can't go from PostGIS version 1.3 to 1.4 _while_ you do the
pg_migrator upgrade. It has to be done either before or after
pg_migrator is run. I wonder how I could prevent someone from trying
that trick.

> >> Could pg_migrator detect usage of "objects" oids (data types in
> >> relation, index opclass, ...) that are unknown to be in the standard
> >> -core + contrib distribution, and quit trying to upgrade the cluster in
> >> this case, telling the user his database is not supported?
>
> > +1 on this.
>
> > Or at least, have it exit and say "if you know that these things are
> > reasonably safe, run pg_migrator again with --force" or something like that.
>
> I don't think that anything in that line is going to be helpful.
> What it will lead to is people mindlessly using --force (cf our
> bad experiences with -i for pg_dump). If you can't give a *useful*
> ie trustworthy warning/error, issuing a useless one is not a good
> substitute.

Yep. The install instructions explain how you have to get around this,
and if they don't understand it, they shouldn't be using pg_migrator and
should just do the traditional dump/restore. It is too tempting to give
them a force flag.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Magnus Hagander wrote:
> >>> Could pg_migrator detect usage of "objects" oids (data types in
> >>> relation, index opclass, ...) that are unknown to be in the standard
> >>> -core + contrib distribution, and quit trying to upgrade the cluster in
> >>> this case, telling the user his database is not supported?
> >
> >> +1 on this.
> >
> >> Or at least, have it exit and say "if you know that these things are
> >> reasonably safe, run pg_migrator again with --force" or something like that.
> >
> > I don't think that anything in that line is going to be helpful.
> > What it will lead to is people mindlessly using --force (cf our
> > bad experiences with -i for pg_dump). If you can't give a *useful*
> > ie trustworthy warning/error, issuing a useless one is not a good
> > substitute.
>
> Well, in that case, error out would be a better option than doing it and
> probably fail later. And have a --force option available, but don't
> suggest it.

Uh, what doesn't error out now in pg_migrator?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:

> Dimitri Fontaine <dfontaine(at)hi-media(dot)com> writes:
>> My vote would go to detect and error out without recovering option. If
>> the tool is not able to handle a situation and knows it, I don't see
>> what would be good about it leting the user lose data on purpose.
>
> No, that's not what's being discussed. The proposal is to have it error
> out when it *does not* know whether there is a real problem; and, in
> fact, when there's only a rather low probability of there being a real
> problem. My view is that that's basically counterproductive. It leads
> directly to having to have a --force switch and then to people using
> that switch carelessly.

True, it could be that the data type representation has not changed
between 8.3 and 8.4, nor the index content format. In this case
pg_migrator will work fine on the cluster as soon as you installed the
new .so...

So the case where pg_migrator still fails is when the .sql file of the
module has changed in a way that restoring what pg_dump gives no longer
match what the .so exposes, or if the new .so is non backward
compatible?

Ok, maybe there's a way it'll just work. I withdraw my vote.

Thanks for your patience, regards,
--
dim

Dimitri Fontaine wrote:
> >> I don't think that anything in that line is going to be helpful.
> >> What it will lead to is people mindlessly using --force (cf our
> >> bad experiences with -i for pg_dump). If you can't give a *useful*
> >> ie trustworthy warning/error, issuing a useless one is not a good
> >> substitute.
> >
> > Well, in that case, error out would be a better option than doing it and
> > probably fail later. And have a --force option available, but don't
> > suggest it.
>
> My vote would go to detect and error out without recovering option. If
> the tool is not able to handle a situation and knows it, I don't see
> what would be good about it leting the user lose data on purpose.
>
> The --force option should be for the user to manually drop his columns
> and indexes (etc) and try pg_migrator again, which will stop listing
> faulty objects but care about the now compatible cluster.
>
> Restoring the lost data is not the job of pg_migrator, of course.

Agreed. Right now pg_migrator never modifies the old cluster except for
renaming pg_control (documented) so the old cluster is not accidentally
restarted. I don't want to change that behavior.

I would filter the dump --schema file, but again, it is best to let the
administrator do it, and if they can't, they should just do
dump/restore.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Let me list the problems with pg_migrator:

> o /contrib and plugin migration (not unique to pg_migrator)
> o you must read/follow the install instructions
> o might require post-migration table/index rebuilds
> o new so serious bugs might exist

I think that #1 and #4 could be substantially alleviated if the
instructions recommended doing a trial run with a schema-only dump
of the database. That is,

* pg_dumpall -s
* load that into a test installation (of the *old* PG version)
* migrate the test installation to new PG version
* do the same sorts of applications compatibility checks you'd want to
do anyway before a major version upgrade

This would certainly catch migration-time failures caused by plugins,
and the followup testing would probably catch any large post-migration
issue.

Somebody who is not willing to do this type of testing should not be
using pg_migrator (yet), and probably has not got a database large
enough to need it anyway.

regards, tom lane

Dimitri Fontaine wrote:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
>
> > Dimitri Fontaine <dfontaine(at)hi-media(dot)com> writes:
> >> My vote would go to detect and error out without recovering option. If
> >> the tool is not able to handle a situation and knows it, I don't see
> >> what would be good about it leting the user lose data on purpose.
> >
> > No, that's not what's being discussed. The proposal is to have it error
> > out when it *does not* know whether there is a real problem; and, in
> > fact, when there's only a rather low probability of there being a real
> > problem. My view is that that's basically counterproductive. It leads
> > directly to having to have a --force switch and then to people using
> > that switch carelessly.
>
> True, it could be that the data type representation has not changed
> between 8.3 and 8.4, nor the index content format. In this case
> pg_migrator will work fine on the cluster as soon as you installed the
> new .so...

Yes.

> So the case where pg_migrator still fails is when the .sql file of the
> module has changed in a way that restoring what pg_dump gives no longer
> match what the .so exposes, or if the new .so is non backward
> compatible?

Yes, that is a problem. It is not a pg_migrator-specific problem
because people traditionally bring the /contrib schema over from the old
install (I think). The only pg_migrator-specific failure is when the
data format changed and dump/restore would fix it, but pg_migrator would
migrate corrupt data. :-(

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Tom Lane wrote:
>> There was just some discussion about that on postgis-devel. I think the
>> conclusion was that you would have to do the PostGIS update as a
>> separate step. They intend to support both 1.3.x and 1.4.x on current
>> versions of Postgres for some time, so in principle you could do it in
>> either order.

> Oh, yea, you can't go from PostGIS version 1.3 to 1.4 _while_ you do the
> pg_migrator upgrade. It has to be done either before or after
> pg_migrator is run. I wonder how I could prevent someone from trying
> that trick.

You don't need to, because it will fail automatically. They are using
version-numbered .so files, so C-language functions referencing the 1.3
.so will fail to load if only the 1.4 .so is in the new installation.

regards, tom lane

Dimitri Fontaine <dfontaine(at)hi-media(dot)com> writes:
> So the case where pg_migrator still fails is when the .sql file of the
> module has changed in a way that restoring what pg_dump gives no longer
> match what the .so exposes, or if the new .so is non backward
> compatible?

Exactly. And note that this is not pg_migrator's fault: a pg_dump
dump and reload of the database exposes the user to the same risks,
if the module author has not been careful about compatibility.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Let me list the problems with pg_migrator:
>
> > o /contrib and plugin migration (not unique to pg_migrator)
> > o you must read/follow the install instructions
> > o might require post-migration table/index rebuilds
> > o new so serious bugs might exist
>
> I think that #1 and #4 could be substantially alleviated if the
> instructions recommended doing a trial run with a schema-only dump
> of the database. That is,
>
> * pg_dumpall -s
> * load that into a test installation (of the *old* PG version)
> * migrate the test installation to new PG version
> * do the same sorts of applications compatibility checks you'd want to
> do anyway before a major version upgrade

But you have no data in the database --- can any meaningful testing be
done?

FYI, pg_migrator will do the schema load pretty early (even before the
file copy) and fail on errors. Retrying pg_migrator is pretty easy and
is now well documented in the INSTALL file.

> This would certainly catch migration-time failures caused by plugins,
> and the followup testing would probably catch any large post-migration
> issue.
>
> Somebody who is not willing to do this type of testing should not be
> using pg_migrator (yet), and probably has not got a database large
> enough to need it anyway.

Agreed.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Tom Lane wrote:
> >> There was just some discussion about that on postgis-devel. I think the
> >> conclusion was that you would have to do the PostGIS update as a
> >> separate step. They intend to support both 1.3.x and 1.4.x on current
> >> versions of Postgres for some time, so in principle you could do it in
> >> either order.
>
> > Oh, yea, you can't go from PostGIS version 1.3 to 1.4 _while_ you do the
> > pg_migrator upgrade. It has to be done either before or after
> > pg_migrator is run. I wonder how I could prevent someone from trying
> > that trick.
>
> You don't need to, because it will fail automatically. They are using
> version-numbered .so files, so C-language functions referencing the 1.3
> .so will fail to load if only the 1.4 .so is in the new installation.

Nice. Something to be learned there. ;-)

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:
> Dimitri Fontaine <dfontaine(at)hi-media(dot)com> writes:
> > So the case where pg_migrator still fails is when the .sql file of the
> > module has changed in a way that restoring what pg_dump gives no longer
> > match what the .so exposes, or if the new .so is non backward
> > compatible?
>
> Exactly. And note that this is not pg_migrator's fault: a pg_dump
> dump and reload of the database exposes the user to the same risks,
> if the module author has not been careful about compatibility.

Agreed. In many ways pg_migrator failures will be caused by failures of
the many tools it relies upon, as I mentioned a few days ago.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Dimitri Fontaine wrote:
>> So the case where pg_migrator still fails is when the .sql file of the
>> module has changed in a way that restoring what pg_dump gives no longer
>> match what the .so exposes, or if the new .so is non backward
>> compatible?

> Yes, that is a problem. It is not a pg_migrator-specific problem
> because people traditionally bring the /contrib schema over from the old
> install (I think). The only pg_migrator-specific failure is when the
> data format changed and dump/restore would fix it, but pg_migrator would
> migrate corrupt data. :-(

There is a different problem though: sometimes the recommended fix for
contrib-module incompatibilities is to load the new contrib module into
the target database before trying to load your old dump file. (We told
people to do that for 8.2->8.3 tsearch2, for example.) In the
pg_migrator context there is no way to insert the new contrib module
first, and also no way to ignore the duplicate-object errors that you
typically get while loading the old dump.

It would probably be a relatively simple feature addition to teach
pg_migrator to load such-and-such modules into the new databases before
loading the old dump. But I'm still scared to death by the idea of
letting it ignore errors, so there doesn't seem to be any good solution
to this type of migration scenario.

regards, tom lane

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Tom Lane wrote:
>> I think that #1 and #4 could be substantially alleviated if the
>> instructions recommended doing a trial run with a schema-only dump
>> of the database. That is,
>>
>> * pg_dumpall -s
>> * load that into a test installation (of the *old* PG version)
>> * migrate the test installation to new PG version
>> * do the same sorts of applications compatibility checks you'd want to
>> do anyway before a major version upgrade

> But you have no data in the database --- can any meaningful testing be
> done?

Well, you'd have to insert some. But this is something you'd have to do
*anyway*, unless you are willing to just pray that your apps don't need
any changes. The only new thing I'm suggesting here is incorporating
use of pg_migrator into your normal compatibility testing.

regards, tom lane

Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
> Exactly. And note that this is not pg_migrator's fault: a pg_dump
> dump and reload of the database exposes the user to the same risks,
> if the module author has not been careful about compatibility.

It seems to me the dump will contain text string representation of data
and pg_restore will run the input function of the type on this, so that
maintaining backward compatibility of the data type doesn't sound
hard. As far as the specific index support goes, pg_restore creates the
index from scratch.

So, from my point of view, supporting backward compatibility by means of
dump and restore is the easy way. Introducing pg_migrator in the game,
the data type and index internals upgrade are now faced to the same
problem as the -core in-place upgrade project.

Maybe we'll be able to provide the extension authors (not only contrib)
a specialized API to trigger in case of in-place upgrade of PG version
or the extension itself, ala Erlang code upgrade facility e.g.:
http://erlang.org/doc/reference_manual/code_loading.html#12.3

This part of the extension design will need help from C dynamic module
experts around, because it's terra incognita as far as I'm concerned.

Regards,
--
dim

I wrote:
> * pg_buffercache has changed the view pg_buffercache, which is
> definitely going to be a migration issue. Need to test whether
> it represents a crash risk if the old definition is migrated.

I checked this, and there is not a crash risk: the function successfully
creates its result tuplestore, and then the main executor notices it's
not compatible with the old view. So you get

regression=# select * from pg_buffercache;
ERROR: function return row and query-specified return row do not match
DETAIL: Returned row contains 8 attributes, but query expects 7.

You can fix it by dropping and recreating the view (eg, run the module's
uninstall and then install scripts). I suppose that might be a bit
annoying if you've built additional views atop this one, but overall
it doesn't sound too bad.

So I don't plan to do anything about this module. Still working on
the others.

regards, tom lane

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Dimitri Fontaine wrote:
> >> So the case where pg_migrator still fails is when the .sql file of the
> >> module has changed in a way that restoring what pg_dump gives no longer
> >> match what the .so exposes, or if the new .so is non backward
> >> compatible?
>
> > Yes, that is a problem. It is not a pg_migrator-specific problem
> > because people traditionally bring the /contrib schema over from the old
> > install (I think). The only pg_migrator-specific failure is when the
> > data format changed and dump/restore would fix it, but pg_migrator would
> > migrate corrupt data. :-(
>
> There is a different problem though: sometimes the recommended fix for
> contrib-module incompatibilities is to load the new contrib module into
> the target database before trying to load your old dump file. (We told
> people to do that for 8.2->8.3 tsearch2, for example.) In the
> pg_migrator context there is no way to insert the new contrib module
> first, and also no way to ignore the duplicate-object errors that you
> typically get while loading the old dump.
>
> It would probably be a relatively simple feature addition to teach
> pg_migrator to load such-and-such modules into the new databases before
> loading the old dump. But I'm still scared to death by the idea of
> letting it ignore errors, so there doesn't seem to be any good solution
> to this type of migration scenario.

Ah, OK, interesting. We could have pg_migrator detect this issue and
fail right away with a message indicating pg_migrator cannot be used
unless those objects are dumped manually and the /contrib removed.

As long as pg_migrator is clear, I don't think people will complain.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Tom Lane wrote:
> >> I think that #1 and #4 could be substantially alleviated if the
> >> instructions recommended doing a trial run with a schema-only dump
> >> of the database. That is,
> >>
> >> * pg_dumpall -s
> >> * load that into a test installation (of the *old* PG version)
> >> * migrate the test installation to new PG version
> >> * do the same sorts of applications compatibility checks you'd want to
> >> do anyway before a major version upgrade
>
> > But you have no data in the database --- can any meaningful testing be
> > done?
>
> Well, you'd have to insert some. But this is something you'd have to do
> *anyway*, unless you are willing to just pray that your apps don't need
> any changes. The only new thing I'm suggesting here is incorporating
> use of pg_migrator into your normal compatibility testing.

Ah, I see. Interesting. I have added the second sentence to the
pg_migrator README:

Installation
------------

See the INSTALL file for detailed installation instructions. For
deployment testing, create a schema-only copy of the old cluster, insert
dummy data, and migrate that.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Dimitri Fontaine wrote:
> Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> writes:
> > Exactly. And note that this is not pg_migrator's fault: a pg_dump
> > dump and reload of the database exposes the user to the same risks,
> > if the module author has not been careful about compatibility.
>
> It seems to me the dump will contain text string representation of data
> and pg_restore will run the input function of the type on this, so that
> maintaining backward compatibility of the data type doesn't sound
> hard. As far as the specific index support goes, pg_restore creates the
> index from scratch.
>
> So, from my point of view, supporting backward compatibility by means of
> dump and restore is the easy way. Introducing pg_migrator in the game,
> the data type and index internals upgrade are now faced to the same
> problem as the -core in-place upgrade project.
>
> Maybe we'll be able to provide the extension authors (not only contrib)
> a specialized API to trigger in case of in-place upgrade of PG version
> or the extension itself, ala Erlang code upgrade facility e.g.:
> http://erlang.org/doc/reference_manual/code_loading.html#12.3
>
> This part of the extension design will need help from C dynamic module
> experts around, because it's terra incognita as far as I'm concerned.

At a minimum it would be great if items could mark themselves as
non-binary-upgradable. Perhaps the existence of a symbol in the *.so
file could indicate that, or a function call could be made and you pass
in the old and new major version numbers and it would return true/false
based on binary upgradeability.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Tom Lane wrote:
>> There is a different problem though: sometimes the recommended fix for
>> contrib-module incompatibilities is to load the new contrib module into
>> the target database before trying to load your old dump file. (We told
>> people to do that for 8.2->8.3 tsearch2, for example.) In the
>> pg_migrator context there is no way to insert the new contrib module
>> first, and also no way to ignore the duplicate-object errors that you
>> typically get while loading the old dump.

> Ah, OK, interesting. We could have pg_migrator detect this issue and
> fail right away with a message indicating pg_migrator cannot be used
> unless those objects are dumped manually and the /contrib removed.

How would it detect it? I think the only thing you could do is
hard-wire tests for specific objects, which is klugy and doesn't
extend to third-party modules that you don't know about.

In most cases where there are major incompatibilities, attempting to
load the old dump file would fail anyway, so I don't think pg_migrator
really needs any hard-wired test. It's the minor changes that are
risky. I think ultimately the burden for those has to be on the module
author: he has to either avoid cross-version incompatibilities or make
sure they will fail safely.

regards, tom lane

On Mon, Jun 8, 2009 at 11:08 AM, Bruce Momjian<bruce(at)momjian(dot)us> wrote:
> Robert Haas wrote:
>> > Right now nothing in the project is referring to pg_migrator except in
>> > the press release, and it is marked as beta there. ?How do you want to
>> > deemphasize it more than that? ?Why did I bother working on this if the
>> > community reaction is to try to figure out how to make people avoid
>> > using it?
>>
>> Because Rome wasn't built in a day.
>>
>> It seems to me that you yourself placed a far more disparaging label
>> on it than anything that anyone has proposed today; this was a week
>> ago:
>>
>> http://archives.postgresql.org/pgsql-hackers/2009-05/msg01470.php
>>
>> I don't think it's anyone's intention to disparage your work on this
>> tool.  It certainly isn't mine.  But it seems obvious to me that it
>> has some pretty severe limitations and warts.  The fact that those
>> limitations and warts are well-documented doesn't negate their
>> existence.  I also don't think calling the software "beta" or
>> "experimental" is a way of deemphasizing it.  I think it's a way of
>> being clear that this software is not the bullet-proof, rock-solid,
>> handles-all-cases-and-keeps-on-trucking level of robustness that
>> people have come to expect from PostgreSQL.
>>
>> FWIW, I have no problem at all with mentioning pg_migrator in the
>> release notes or the documentation; my failure to respond to your last
>> emails on this topic was due to being busy and having already spent
>> too much time responding to other emails, not due to thinking it was a
>> bad idea.  I actually think it's a good idea.  But I also think those
>> references should describe it as experimental, because I think it is.
>> I really hope it won't remain experimental forever, but I think that's
>> an accurate characterization of where it is now.
>
> pg_migrator should be looked at critically here, and I agree we should
> avoid letting pg_migrator failures reflect badly on Postgres.
>
> Let me list the problems with pg_migrator:
>
>        o  /contrib and plugin migration (not unique to pg_migrator)
>        o  you must read/follow the install instructions
>        o  might require post-migration table/index rebuilds
>        o  new so serious bugs might exist

I pretty much agree with this list. With respect to #2, I don't think
that it's asking a lot for people to read/follow the install
instructions, so I don't consider that a serious problem.

> and let me list its benefits:
>
>        o  first in-place upgrade capability in years
>        o  tested by some users, all successful (since late alpha)
>        o  removes major impediment to adoption
>        o  includes extensive error checking and reporting
>        o  contains detailed installation/usage instructions
>
> So let's look at pg_migrator as an opportunity and a risk.  As far as I
> know, only Hiroshi Saito and I have have looked at the code.  Why don't
> others read the pg_migrator source code looking for bugs?  Why have more
> people not test it?

No reason at all - I very much hope that happens.

> I think "experimental" is the wrong label.  Experimental assumes its
> usefulness is uncertain and that it is still being researched ---
> neither is true.  Once I release pg_migrator 8.4 final at the end of
> this week (assuming no bugs are reported), I consider it done, or at
> least advanced as far as I can go until I get more feedback from users.

Oh, to me "experimental" does not imply that usefulness is uncertain;
rather, it implies that usefulness has been established but that the
code is new (item #4 above) and may be not be 100% feature-complete
(items #1 and #3 above).

> I think we can say:  "pg_migrator is designed for experienced users with
> large databases, for whom the typical dump/restore required for major
> version upgrades is a hardship".

Precisely. In other words, if you are an INEXPERIENCED user (that is
to say, most of them) or you don't have a particular large database,
dump + reload is probably the safest option. We're not discouraging
you from use pg_migrator, but please be careful and observe that it is
new and has some limitations.

> I assume this will be the same adoption pattern we had with the Win32
> port, where it was a new platform in 8.0 and we dealt with some issues
> as it was deployed, and that people who want it will find it and
> hopefully it will be useful for them.

Completely agree. And like the Windows port, hopefully after a
release or two, we'll figure out what we can improve and do so. I am
interested in this problem but all of my free time lately has been
going into the EXPLAIN patch I'm working on, so I haven't had time to
dig into it much. The problems of being a hobbyist...

...Robert

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Tom Lane wrote:
> >> There is a different problem though: sometimes the recommended fix for
> >> contrib-module incompatibilities is to load the new contrib module into
> >> the target database before trying to load your old dump file. (We told
> >> people to do that for 8.2->8.3 tsearch2, for example.) In the
> >> pg_migrator context there is no way to insert the new contrib module
> >> first, and also no way to ignore the duplicate-object errors that you
> >> typically get while loading the old dump.
>
> > Ah, OK, interesting. We could have pg_migrator detect this issue and
> > fail right away with a message indicating pg_migrator cannot be used
> > unless those objects are dumped manually and the /contrib removed.
>
> How would it detect it? I think the only thing you could do is
> hard-wire tests for specific objects, which is klugy and doesn't
> extend to third-party modules that you don't know about.

Yep, that is the only way. The good news is that we are not modifying
any data so it is just a detection issue, i.e. it would not destabilize
pg_migrator's operation.

> In most cases where there are major incompatibilities, attempting to
> load the old dump file would fail anyway, so I don't think pg_migrator
> really needs any hard-wired test. It's the minor changes that are
> risky. I think ultimately the burden for those has to be on the module
> author: he has to either avoid cross-version incompatibilities or make
> sure they will fail safely.

Yep.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> At a minimum it would be great if items could mark themselves as
> non-binary-upgradable.

It's hardly difficult to make that happen --- just change the C name of
some function, or the name of the whole .so file.

regards, tom lane

Robert Haas wrote:
> > Let me list the problems with pg_migrator:
> >
> > ? ? ? ?o ?/contrib and plugin migration (not unique to pg_migrator)
> > ? ? ? ?o ?you must read/follow the install instructions
> > ? ? ? ?o ?might require post-migration table/index rebuilds
> > ? ? ? ?o ?new so serious bugs might exist
>
> I pretty much agree with this list. With respect to #2, I don't think
> that it's asking a lot for people to read/follow the install
> instructions, so I don't consider that a serious problem.

My point was that I think someday pg_migrator will be point-and-click,
but it is not now.

> Oh, to me "experimental" does not imply that usefulness is uncertain;
> rather, it implies that usefulness has been established but that the
> code is new (item #4 above) and may be not be 100% feature-complete
> (items #1 and #3 above).
>
> > I think we can say: ?"pg_migrator is designed for experienced users with
> > large databases, for whom the typical dump/restore required for major
> > version upgrades is a hardship".
>
> Precisely. In other words, if you are an INEXPERIENCED user (that is
> to say, most of them) or you don't have a particular large database,
> dump + reload is probably the safest option. We're not discouraging
> you from use pg_migrator, but please be careful and observe that it is
> new and has some limitations.

Agreed. There is no reason for most users to need pg_migrator; it is
not worth the risk for them, however small. There are some people who
really need it, and hopefully they are experienced users, while there is
a larger group who want to know such an option _exists_, so if they ever
need it, it is available.

> > I assume this will be the same adoption pattern we had with the Win32
> > port, where it was a new platform in 8.0 and we dealt with some issues
> > as it was deployed, and that people who want it will find it and
> > hopefully it will be useful for them.
>
> Completely agree. And like the Windows port, hopefully after a
> release or two, we'll figure out what we can improve and do so. I am
> interested in this problem but all of my free time lately has been
> going into the EXPLAIN patch I'm working on, so I haven't had time to
> dig into it much. The problems of being a hobbyist...

One difference in risk is that the Windows port usually had _new_ data
meaning you were not risking as much as using pg_migrator on an
estabilished database installation.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > At a minimum it would be great if items could mark themselves as
> > non-binary-upgradable.
>
> It's hardly difficult to make that happen --- just change the C name of
> some function, or the name of the whole .so file.

Yes, but it needs to happen. ;-) PostGIS has done this, which is good.
The problem is that if they don't do it it is out of the control of
pg_migrator.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian wrote:
> > Oh, to me "experimental" does not imply that usefulness is uncertain;
> > rather, it implies that usefulness has been established but that the
> > code is new (item #4 above) and may be not be 100% feature-complete
> > (items #1 and #3 above).
> >
> > > I think we can say: ?"pg_migrator is designed for experienced users with
> > > large databases, for whom the typical dump/restore required for major
> > > version upgrades is a hardship".
> >
> > Precisely. In other words, if you are an INEXPERIENCED user (that is
> > to say, most of them) or you don't have a particular large database,
> > dump + reload is probably the safest option. We're not discouraging
> > you from use pg_migrator, but please be careful and observe that it is
> > new and has some limitations.
>
> Agreed. There is no reason for most users to need pg_migrator; it is
> not worth the risk for them, however small. There are some people who
> really need it, and hopefully they are experienced users, while there is
> a larger group who want to know such an option _exists_, so if they ever
> need it, it is available.

I think this "larger group" is where my problem with the word
"experimental" come in. I think pg_migrator is far enough along that we
know it works, and that it will probably work for future major releases.
By calling it "experimental" we are not conveying confidence in the tool
for people who are making deployment decisions based on the existence of
such tool, even if they aren't going to use it initially. And by not
conveying confidence, we will lose the adoption advantage we can get
from pg_migrator.

Now, we don't want to over-promise, but at the same time we shouldn't
downplay the tool either. For a sufficiently-experienced administrator,
pg_migrator is a useful migration tool, and we need to convey that.
Even if you have to hire a consultant to manage the migration, if it
saves days of downtime, it is worth it. Consultants don't often use
experimental tools, but they do use complex, powerful tools that are
often rough around the edges in terms of usability, e.g. read the
INSTALL file carefully.

For longterm strategy, let me list the challenges for pg_migrator from
any major upgrade (listed in the DEVELOPERS file):

Change Conversion Method
------------------------------------------------------------------------------
clog none
heap page header, including bitmask convert to new page format on read
tuple header, including bitmask convert to new page format on read
data value format create old data type in new cluster
index page format reindex, or recreate old index methods

These are the issue we will have to address for 8.5 and beyond if
pg_migrator is to remain useful.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Mon, Jun 8, 2009 at 1:06 PM, Bruce Momjian<bruce(at)momjian(dot)us> wrote:
> Tom Lane wrote:
>> Bruce Momjian <bruce(at)momjian(dot)us> writes:
>> > At a minimum it would be great if items could mark themselves as
>> > non-binary-upgradable.
>>
>> It's hardly difficult to make that happen --- just change the C name of
>> some function, or the name of the whole .so file.
>
> Yes, but it needs to happen.  ;-)  PostGIS has done this, which is good.
> The problem is that if they don't do it it is out of the control of
> pg_migrator.

I think it might be possible to implement a system that can't be
broken by accident. Firefox (at least AIUI) requires plugin authors
to explicitly flag their modules as compatible with new versions of
Firefox. When you upgrade your firefox installation in place (heh,
heh) it goes off to the web site and checks whether all of your
extensions have been so flagged. Any that have not been get disabled
automatically.

Obviously we don't want to get into connecting to a web site, but we
could probably come up with some other API for .so files to indicate
which versions of PG they're compatible with. If they don't implement
that API, we assume the predate its introduction and are not
upgradeable. I'm fuzzy on the details here but the point is that if
you implement an opt-in system rather than an opt-out system then
people have to deliberately circumvent it to break things, rather than
just needing to be lazy.

...Robert

On Mon, Jun 8, 2009 at 1:20 PM, Bruce Momjian<bruce(at)momjian(dot)us> wrote:
> Bruce Momjian wrote:
>> > Oh, to me "experimental" does not imply that usefulness is uncertain;
>> > rather, it implies that usefulness has been established but that the
>> > code is new (item #4 above) and may be not be 100% feature-complete
>> > (items #1 and #3 above).
>> >
>> > > I think we can say: ?"pg_migrator is designed for experienced users with
>> > > large databases, for whom the typical dump/restore required for major
>> > > version upgrades is a hardship".
>> >
>> > Precisely.  In other words, if you are an INEXPERIENCED user (that is
>> > to say, most of them) or you don't have a particular large database,
>> > dump + reload is probably the safest option.  We're not discouraging
>> > you from use pg_migrator, but please be careful and observe that it is
>> > new and has some limitations.
>>
>> Agreed.  There is no reason for most users to need pg_migrator;  it is
>> not worth the risk for them, however small.  There are some people who
>> really need it, and hopefully they are experienced users, while there is
>> a larger group who want to know such an option _exists_, so if they ever
>> need it, it is available.
>
> I think this "larger group" is where my problem with the word
> "experimental" come in.  I think pg_migrator is far enough along that we
> know it works, and that it will probably work for future major releases.
> By calling it "experimental" we are not conveying confidence in the tool
> for people who are making deployment decisions based on the existence of
> such tool, even if they aren't going to use it initially.  And by not
> conveying confidence, we will lose the adoption advantage we can get
> from pg_migrator.
>
> Now, we don't want to over-promise, but at the same time we shouldn't
> downplay the tool either.  For a sufficiently-experienced administrator,
> pg_migrator is a useful migration tool, and we need to convey that.
> Even if you have to hire a consultant to manage the migration, if it
> saves days of downtime, it is worth it.  Consultants don't often use
> experimental tools, but they do use complex, powerful tools that are
> often rough around the edges in terms of usability, e.g. read the
> INSTALL file carefully.

Fair enough. I'm game to use a different word. I spent approximately
30 seconds coming up with that suggestion. :-)

> For longterm strategy, let me list the challenges for pg_migrator from
> any major upgrade (listed in the DEVELOPERS file):
>
>  Change                                  Conversion Method
>  ------------------------------------------------------------------------------
>  clog                                    none
>  heap page header, including bitmask     convert to new page format on read
>  tuple header, including bitmask         convert to new page format on read
>  data value format                       create old data type in new cluster
>  index page format                       reindex, or recreate old index methods
>
> These are the issue we will have to address for 8.5 and beyond if
> pg_migrator is to remain useful.

No arguments here, sounds like interesting stuff.

...Robert

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> Obviously we don't want to get into connecting to a web site, but we
> could probably come up with some other API for .so files to indicate
> which versions of PG they're compatible with.

You mean like PG_MODULE_MAGIC?

regards, tom lane

On Mon, Jun 8, 2009 at 1:32 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> Obviously we don't want to get into connecting to a web site, but we
>> could probably come up with some other API for .so files to indicate
>> which versions of PG they're compatible with.
>
> You mean like PG_MODULE_MAGIC?

Hey, how about that. Why doesn't that solve our problem here?

[ thinks ... ] I guess it's because there's no guarantee that the
function is installed on the SQL level with the signature that is
appropriate on the C level. To fix that, I suppose we'd need to
version the contents of the .sql file that installs the definitions,
which gets back to the problem of building a general-purpose module
facility.

...Robert

Robert Haas wrote:
> On Mon, Jun 8, 2009 at 1:06 PM, Bruce Momjian<bruce(at)momjian(dot)us> wrote:
> > Tom Lane wrote:
> >> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> >> > At a minimum it would be great if items could mark themselves as
> >> > non-binary-upgradable.
> >>
> >> It's hardly difficult to make that happen --- just change the C name of
> >> some function, or the name of the whole .so file.
> >
> > Yes, but it needs to happen. ?;-) ?PostGIS has done this, which is good.
> > The problem is that if they don't do it it is out of the control of
> > pg_migrator.
>
> I think it might be possible to implement a system that can't be
> broken by accident. Firefox (at least AIUI) requires plugin authors
> to explicitly flag their modules as compatible with new versions of
> Firefox. When you upgrade your firefox installation in place (heh,
> heh) it goes off to the web site and checks whether all of your
> extensions have been so flagged. Any that have not been get disabled
> automatically.

Interesting it allows the flagging to happen in real-time, rather than
requiring the system to know at install time whether it is compatible
with future verions (almost an impossibility). I am afraid we would
need some kind of real-time check, or at least have major versions flag
which external stuff cannot be upgraded, which we have discussed here
already.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Robert Haas wrote:
> > Now, we don't want to over-promise, but at the same time we shouldn't
> > downplay the tool either. ?For a sufficiently-experienced administrator,
> > pg_migrator is a useful migration tool, and we need to convey that.
> > Even if you have to hire a consultant to manage the migration, if it
> > saves days of downtime, it is worth it. ?Consultants don't often use
> > experimental tools, but they do use complex, powerful tools that are
> > often rough around the edges in terms of usability, e.g. read the
> > INSTALL file carefully.
>
> Fair enough. I'm game to use a different word. I spent approximately
> 30 seconds coming up with that suggestion. :-)

I think the text I already posted is appropriate:

pg_migrator is designed for experienced users with large databases, for
whom the typical dump/restore required for major version upgrades is a
hardship.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

I wrote:
> [ concerning various migration hazards in contrib/ ]

> * isn has got the nastiest problems of the lot: since it piggybacks
> on type bigint, a migrated database might work, or might crash
> miserably, depending on whether bigint has become pass-by-value
> in the new database. I'm not very sure if we can fix this reasonably.

I've spent some time thinking about possible workarounds for this, and
not really come up with any. The only feasible thing I can think of
to do is teach pg_migrator to refuse to migrate if (a) the old DB
contains contrib/isn, and (b) the new DB has FLOAT8PASSYVAL (which
can be checked in pg_control). One question here is how you decide
if the old DB contains contrib/isn. I don't think looking for the
type name per se is a hot idea. The best plan that has come to mind
is to look through pg_proc to see if there are any C-language functions
that reference "$libdir/isn".

> * pg_freespacemap has made *major* changes in its ABI. There's
> probably no hope of this working either, but we need to be sure
> it's not a crash risk.

This turns out not to be a problem: the set of exposed C function names
changed, so the function definitions will fail to migrate. Dropping
the module before migrating and reinstalling it afterwards is an easy
workaround.

> * pgstattuple has made changes in the output types of its functions.
> This is a serious crash risk, and I'm not immediately sure how to
> fix it. Note that simply migrating the module will not draw any
> errors.

This doesn't seem to create serious problems either. pgstatindex() has
changed some output columns from int4 to int8, but because it creates
the result tuple from text strings, it manages to just work anyway.
(In principle you could get some overflow problems with very large
indexes, but I doubt that's an issue in practice; and it couldn't cause
a crash anyway.) pg_relpages() likewise changed its return type, but
in this particular case you could only get garbage answers not a crash.
So I think we can just tell people to reinstall the SQL file after
migration.

> * tsearch2 has opclass support function changes, but unlike other
> cases of this, it will fail to migrate to 8.4 because the functions
> are references to core functions instead of being declared in the
> module. Also, "drop it first" isn't a very useful recommendation
> since the domains it defines might be used somewhere.

It would be nice if this migrated cleanly, but it doesn't and there's
not much we can do about it. At least it will fail safely.

So, other than the suggested pg_migrator hack for contrib/isn, the only
thing left to do here is fix dblink_current_query(). I'll take care of
that, but not till after Joe commits his remaining patch, so as not to
risk creating merge hazards for him.

regards, tom lane

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Mon, Jun 8, 2009 at 1:32 PM, Tom Lane<tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> You mean like PG_MODULE_MAGIC?

> Hey, how about that. Why doesn't that solve our problem here?

> [ thinks ... ] I guess it's because there's no guarantee that the
> function is installed on the SQL level with the signature that is
> appropriate on the C level.

Yeah. And it's more than just the function itself. For example,
in the contrib/isn mess, the function definitions didn't change.
The problem is the passbyval flag (or lack of it) on the type
definition.

I think we've speculated in the past about having ways of embedding
per-function data into the .so libraries so that these sorts of
things could be caught automatically. But it'd be a lot of work
for rather limited reward.

regards, tom lane

Bruce Momjian escribió:

> For longterm strategy, let me list the challenges for pg_migrator from
> any major upgrade (listed in the DEVELOPERS file):
>
> Change Conversion Method
> ------------------------------------------------------------------------------
> clog none
> heap page header, including bitmask convert to new page format on read
> tuple header, including bitmask convert to new page format on read
> data value format create old data type in new cluster
> index page format reindex, or recreate old index methods

TOAST changes?

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

Tom Lane wrote:
> I wrote:
> > [ concerning various migration hazards in contrib/ ]
>
> > * isn has got the nastiest problems of the lot: since it piggybacks
> > on type bigint, a migrated database might work, or might crash
> > miserably, depending on whether bigint has become pass-by-value
> > in the new database. I'm not very sure if we can fix this reasonably.
>
> I've spent some time thinking about possible workarounds for this, and
> not really come up with any. The only feasible thing I can think of
> to do is teach pg_migrator to refuse to migrate if (a) the old DB
> contains contrib/isn, and (b) the new DB has FLOAT8PASSYVAL (which
> can be checked in pg_control). One question here is how you decide
> if the old DB contains contrib/isn. I don't think looking for the
> type name per se is a hot idea. The best plan that has come to mind
> is to look through pg_proc to see if there are any C-language functions
> that reference "$libdir/isn".

Sure, pg_migrator is good at checking. Please confirm you want this
added to pg_migrator.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Bruce Momjian <bruce(at)momjian(dot)us> writes:
> Tom Lane wrote:
>> I've spent some time thinking about possible workarounds for this, and
>> not really come up with any. The only feasible thing I can think of
>> to do is teach pg_migrator to refuse to migrate if (a) the old DB
>> contains contrib/isn, and (b) the new DB has FLOAT8PASSYVAL (which
>> can be checked in pg_control). One question here is how you decide
>> if the old DB contains contrib/isn. I don't think looking for the
>> type name per se is a hot idea. The best plan that has come to mind
>> is to look through pg_proc to see if there are any C-language functions
>> that reference "$libdir/isn".

> Sure, pg_migrator is good at checking. Please confirm you want this
> added to pg_migrator.

Yeah, I'd suggest it. Even if we later come up with a workaround for
contrib/isn, you're going to want to have the infrastructure in place
for this type of check, because there will surely be cases that need it.

Note that I think the FLOAT8PASSYVAL check is a must. There is no
reason to forbid migrating isn on 32-bit machines, for example.

regards, tom lane

Alvaro Herrera wrote:
> Bruce Momjian escribi?:
>
> > For longterm strategy, let me list the challenges for pg_migrator from
> > any major upgrade (listed in the DEVELOPERS file):
> >
> > Change Conversion Method
> > ------------------------------------------------------------------------------
> > clog none
> > heap page header, including bitmask convert to new page format on read
> > tuple header, including bitmask convert to new page format on read
> > data value format create old data type in new cluster
> > index page format reindex, or recreate old index methods
>
> TOAST changes?

Good point, added.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:
> Bruce Momjian <bruce(at)momjian(dot)us> writes:
> > Tom Lane wrote:
> >> I've spent some time thinking about possible workarounds for this, and
> >> not really come up with any. The only feasible thing I can think of
> >> to do is teach pg_migrator to refuse to migrate if (a) the old DB
> >> contains contrib/isn, and (b) the new DB has FLOAT8PASSYVAL (which
> >> can be checked in pg_control). One question here is how you decide
> >> if the old DB contains contrib/isn. I don't think looking for the
> >> type name per se is a hot idea. The best plan that has come to mind
> >> is to look through pg_proc to see if there are any C-language functions
> >> that reference "$libdir/isn".
>
> > Sure, pg_migrator is good at checking. Please confirm you want this
> > added to pg_migrator.
>
> Yeah, I'd suggest it. Even if we later come up with a workaround for
> contrib/isn, you're going to want to have the infrastructure in place
> for this type of check, because there will surely be cases that need it.
>
> Note that I think the FLOAT8PASSYVAL check is a must. There is no
> reason to forbid migrating isn on 32-bit machines, for example.

Done, with patch attached, and pg_migrator beta6 released.

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +