Re: [patch] pg_upgrade script for 8.3->8.4

Hi all,

I attached pg_upgrade.sh script patch which works now for 8.3->8.4. It is
contrib module in contrib/pg_upgrade directory. Just make/make install and it works.

There are two changes from previous 8.1->8.2.

1) pg_largeobject is also upgraded
2) added check for dropped column

And now how to run a test. At first DO NOT test it on production database :-).
Please, do binary copy and start test on binary copy. Binary copy is important
because it contains a lot of updated, dead tuples and other interesting things.

Script is easy to use. You need only setup access to old and new binaries and
old and new data directory. I use following script:

#!/bin/bash

export PG_OLD_DATADIR=/zfs_test/postgres_83/data_83
export PG_OLD_BINDIR=/usr/postgres/8.3/bin
export PG_NEW_DATADIR=/zfs_test/postgres_83/data_84_upg
export PG_NEW_BASEDIR=/var/tmp/pg84_upg/
ksh ${PG_NEW_BASEDIR}/bin/pg_upgrade.sh -m

you can use also switches - try pg_upgrade.sh --help

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

The script contains some magic to handle following issues.

1) Keep relfileid of toast file same. It is important because toast pointer
contains relfileid. Currently script creates fake files with same number to
protect postgresql to create new relation with this refileid. It works but by my
opinion it is not much robust. I suggest to use following syntax:

create table foo (id int) with (relfileid=16544, reltoastid=11655,
reltoastidx=16543)

pg_dump(all) will be extended to dump this information on a request.

2) problem with dropped columns. PostgreSQL do not remove column physically from
the disk. It only marks that column as deleted and the column is ignored in the
future. But pg_dump dumps only valid column. There is idea from greg&greg to
extend create table syntax with padding column:

CREATE TABLE foo (
col1 integer,
NULL COLUMN(2,0),
col2 integer
);

3) tablespace and database oid mapping. It is similar with relations. Another
problem with tablespace location is that CREATE TABLESPACE checks if directory
is empty and it fails when it contains any file/directory. Unfortunately, it is
no much good for upgrade because usually tablespace is mountpoint and any
copy/move outside a mountpoint is not wanted.

Suggested sugar syntax is:

CREATE DATABASE foobar WITH ID=17012;
CREATE TABLESPACE bar LOCATION '/dev/null/' ID=15543 IGNORECONTENT;

4) script is written in ksh. It has several problems. First is that it does not
work on win, second is that it needs extra magic to escape any stupid object
names. Bruce has suggested to rewrite it to PERL. It is maybe good idea but I'm
not really PERL guru - any volunteers?

By the way why we accept whole ASCII in name datatype (including 0-31)?

Comments, thoughts?

Thanks Zdenek

On Thu, Dec 04, 2008 at 05:57:01PM +0100, Zdenek Kotala wrote:
> Hi all,
>
> I attached pg_upgrade.sh script patch which works now for 8.3->8.4. It is
> contrib module in contrib/pg_upgrade directory. Just make/make install
> and it works.

Kudos!

> 4) script is written in ksh. It has several problems. First is that
> it does not work on win, second is that it needs extra magic to
> escape any stupid object names. Bruce has suggested to rewrite it
> to PERL. It is maybe good idea but I'm not really PERL guru - any
> volunteers?

I'll give it a whirl today :)

Cheers,
David (oh, and it's Perl ;)
--
David Fetter <david(at)fetter(dot)org> http://fetter.org/
Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter
Skype: davidfetter XMPP: david(dot)fetter(at)gmail(dot)com

Remember to vote!
Consider donating to Postgres: http://www.postgresql.org/about/donate

Zdenek Kotala napsal(a):
> Hi all,
>
> I attached pg_upgrade.sh script patch which works now for 8.3->8.4. It
> is contrib module in contrib/pg_upgrade directory. Just make/make
> install and it works.

I forget to mention that default datetime format is different now. Please, use
same datetime format for 8.3 and 8.4. I will add check.

Zdenek

On Thu, 4 Dec 2008, Zdenek Kotala wrote:

> 1) Keep relfileid of toast file same. It is important because toast pointer
> contains relfileid. Currently script creates fake files with same number to
> protect postgresql to create new relation with this refileid. It works but by
> my opinion it is not much robust. I suggest to use following syntax:
>
> create table foo (id int) with (relfileid=16544, reltoastid=11655,
> reltoastidx=16543)

But once there's a more core integrated in-place upgrade, as envisioned
for 8.4->8,5, this syntax wouldn't been useful for anything anymore,
right? I understand your distaste for the hack, but it seems a bit
extreme to start fiddling with CREATE TABLE and pg_dump* just to implement
a work-around for a temporary problem. I think your magic is strong
enough for this, as long as the upgrade script does some sanity checks and
proceeds cautiously I think we can live with it.

> Another problem with tablespace location is that CREATE TABLESPACE
> checks if directory is empty and it fails when it contains any
> file/directory...Suggested sugar syntax is:
> CREATE DATABASE foobar WITH ID=17012;
> CREATE TABLESPACE bar LOCATION '/dev/null/' ID=15543 IGNORECONTENT;

The logic of the above continues here; the "id=xxx" construct seems like
it will be useless clutter in the future, and if the script is capable of
sorting it out with a bit of hacking then go with that. The way I see
things, the sequence of events will go like this:

-Create new cluster
-Restore schema
-Fiddle with IDs, table spaces, etc.
-Then, only if there were no errors or problems with the above, are the
old files moved over.

As long as all the hacking and safety checks happen before any of the
original files are touched at all, it's kind of ugly but it should work
well enough for the target audience: relatively saavy DBAs who just can't
take the time for a dump/reload because their database is too large. The
kind of admins who doesn't know how to test and stage a major version
upgrade with appropriate backups are the group that are still running
PG7.4.[1]

I'm not sure if the same logic applies to the "IGNORECONTENT" suggestion
for tablespaces though. I know I've been vaguely annoyed by that
limitation before, typically because it would have been easier to directly
use a new mount point as a tablespace except that there's a "lost+found"
directory in there. End up adding another directory level for no good
reason. This seems like a relatively small caliber foot-gun to provide;
will have to take a look at the code to see if I still feel that way
afterwards, if nobody jumps up to protest the whole concept first that is.

[1] I hereby propose Greg's Law of DBAs: the larger and more critical a
database is, the more likely it is to attract a clueful DBA to take care
of it.

--
* Greg Smith gsmith(at)gregsmith(dot)com http://www.gregsmith.com Baltimore, MD

Greg Smith napsal(a):
> On Thu, 4 Dec 2008, Zdenek Kotala wrote:
>
>> 1) Keep relfileid of toast file same. It is important because toast
>> pointer contains relfileid. Currently script creates fake files with
>> same number to protect postgresql to create new relation with this
>> refileid. It works but by my opinion it is not much robust. I suggest
>> to use following syntax:
>>
>> create table foo (id int) with (relfileid=16544, reltoastid=11655,
>> reltoastidx=16543)
>
> But once there's a more core integrated in-place upgrade, as envisioned
> for 8.4->8,5, this syntax wouldn't been useful for anything anymore,
> right?

Maybe it could be useful also for user who needs to restore database with same
filename (replication, recovery...). But I don't know about any other real
request for this feature.

> I understand your distaste for the hack, but it seems a bit
> extreme to start fiddling with CREATE TABLE and pg_dump* just to
> implement a work-around for a temporary problem. I think your magic is
> strong enough for this, as long as the upgrade script does some sanity
> checks and proceeds cautiously I think we can live with it.

I agree that it is temporary solution and does not make sense to create any
special temporary hack for it. However, this syntax uses reloption mechanism
which is easy to use and modification should be small and maybe someone could
use for another purpose. But how you mention it is now solved by script magic.

Zdenek