On Fri, Jun 19, 2015 at 07:10:40PM +0300, Marti Raudsepp wrote:
> Hi list

Sorry I am just getting this report.  Thanks to the people who "stalled"
for me.

> One of my databases failed to upgrade successfully and produced this error in
> the copying phase:
> 
> error while copying relation "pg_catalog.pg_largeobject" ("/srv/ssd/
> PG_9.3_201306121/1/12023" to "/PG_9.4_201409291/1/12130"): No such file or
> directory

As with all good bug reports, there are multiple bugs here.  ;-)  Notice
the second path has an invalid prefix:  "/PG_9.4_201409291/1/12130" ---
obviously something is seriously wrong.  The failure of pg_dumpall to
move 'postgres' and 'template1' databases to the new tablespace cannot
explain that, i.e. I could understand pg_upgrade looking for
pg_largeobject in the default data directory, or in the new one, but not
in a path that doesn't even exist.  pg_upgrade tracks old and new
tablespaces separately, so there is obviously something wrong.

And I found it, patch attached.  The pg_upgrade code was testing for the
tablespace of the _old_ object, then assigning the old and _new_
tablespaces based on that.  The first patch fixes that, and should be
backpatched to all supported pg_upgrade versions.

> Turns out this happens when either the "postgres" or "template1" databases 
> have
> been moved to a non-default tablespace. pg_dumpall does not dump attributes
> (such as tablespace) for these databases; pg_upgrade queries the new cluster
> about the tablespace for these relations and builds a broken destination path
> for the copy/link operation.
> 
> The least bad solution seems to be generating ALTER DATBASE SET TABLESPACE
> commands for these from pg_dumpall. Previously a --globals-only dump didn't
> generate psql \connect commands, but you can't run SET TABLESPACE from within
> the same database you're connected to. So to move "postgres", it needs to
> connect to "template1" and vice versa. That seems fine for the purpose of
> pg_upgrade which can assume a freshly created cluster with both databases
> intact.

Yes, seems like a good solution.

> I have implemented a proof of concept patch for this. Currently I'm only
> tackling the binary upgrade failure and not general pg_dumpall.
> 
> Alternatively, we could allow SET TABLESPACE in the current database, which
> seems less ugly to me. A code comment says "Obviously can't move the tables of
> my own database", but it's not obvious to me why. If I'm the only connected
> backend, it seems that any caches and open files could be invalidated. But I
> don't know how big of an undertaking that would be.

Your submitted patch, attached, also looks good, and should be
backpatched.  My only question is whether this should be for all runs of
pg_dumpall, not just in binary upgrade mode.  Comments?

Once we agree on this, I will apply these to all back branches and run
tests by moving template1 before the upgrade to make sure it works for
all PG versions.

> pg_upgrade) misses:
> * Nothing at all is dumped for the template0 database, although ACLs, settings
> and the tablespace can be changed by the user

Uh, we _assume_ no one is connecting to template0, but you are right
people can change things _about_ template0.  It would be nice to
preserve those.

> * template1 & postgres databases retain ACLs and settings, but not attributes
> like TABLESPACE or CONNECTION LIMIT. Other attributes like LC_COLLATE and
> LC_CTYPE can't be changed without recreating the DB, but those don't  matter
> for the pg_upgrade case anyway.
> 
> It seems those would be good material for another patch?

Agreed.  I am not sure how we have gotten this far with so few
complaints about this problem.

-- 
  Bruce Momjian  <br...@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +
diff --git a/src/bin/pg_upgrade/info.c b/src/bin/pg_upgrade/info.c
new file mode 100644
index e158c9f..390fc62
*** a/src/bin/pg_upgrade/info.c
--- b/src/bin/pg_upgrade/info.c
*************** create_rel_filename_map(const char *old_
*** 140,145 ****
--- 140,146 ----
  						const RelInfo *old_rel, const RelInfo *new_rel,
  						FileNameMap *map)
  {
+ 	/* Someday the old/new tablespaces might not match, so handle it. */
  	if (strlen(old_rel->tablespace) == 0)
  	{
  		/*
*************** create_rel_filename_map(const char *old_
*** 147,162 ****
  		 * exist in the data directories.
  		 */
  		map->old_tablespace = old_data;
- 		map->new_tablespace = new_data;
  		map->old_tablespace_suffix = "/base";
- 		map->new_tablespace_suffix = "/base";
  	}
  	else
  	{
  		/* relation belongs to a tablespace, so use the tablespace location */
  		map->old_tablespace = old_rel->tablespace;
- 		map->new_tablespace = new_rel->tablespace;
  		map->old_tablespace_suffix = old_cluster.tablespace_suffix;
  		map->new_tablespace_suffix = new_cluster.tablespace_suffix;
  	}
  
--- 148,171 ----
  		 * exist in the data directories.
  		 */
  		map->old_tablespace = old_data;
  		map->old_tablespace_suffix = "/base";
  	}
  	else
  	{
  		/* relation belongs to a tablespace, so use the tablespace location */
  		map->old_tablespace = old_rel->tablespace;
  		map->old_tablespace_suffix = old_cluster.tablespace_suffix;
+ 	}
+ 
+ 	/* Do the same for new tablespaces */
+ 	if (strlen(new_rel->tablespace) == 0)
+ 	{
+ 		map->new_tablespace = new_data;
+ 		map->new_tablespace_suffix = "/base";
+ 	}
+ 	else
+ 	{
+ 		map->new_tablespace = new_rel->tablespace;
  		map->new_tablespace_suffix = new_cluster.tablespace_suffix;
  	}
  
>From b10c8f62e436a52b77e3df2ea59f313112e2466a Mon Sep 17 00:00:00 2001
From: Marti Raudsepp <ma...@juffo.org>
Date: Fri, 19 Jun 2015 18:58:17 +0300
Subject: [PATCH] Fix pg_upgrade when postgres/template1 aren't in default
 tablespace

In order to move these databases to the correct tablespace in the new
cluster, pg_dumpall needs to generate \connect commands to switch
databases during restore.

This oversight caused the error:
error while copying relation "pg_catalog.pg_largeobject"
("/tablespace/PG_9.3_201306121/1/12023" to "/PG_9.4_201409291/1/12130"):
No such file or directory
---
 src/bin/pg_dump/pg_dumpall.c | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/src/bin/pg_dump/pg_dumpall.c b/src/bin/pg_dump/pg_dumpall.c
index d98c83e..75226f1 100644
--- a/src/bin/pg_dump/pg_dumpall.c
+++ b/src/bin/pg_dump/pg_dumpall.c
@@ -1412,6 +1412,24 @@ dumpCreateDB(PGconn *conn)
 
 			appendPQExpBufferStr(buf, ";\n");
 		}
+		else if (binary_upgrade)
+		{
+			if (strcmp(dbtablespace, "pg_default") != 0 && !no_tablespaces)
+			{
+				/*
+				 * Cannot change tablespace of the database we're connected to.
+				 * To move "postgres" to another tablespace, we connect to
+				 * "template1" and vice versa.
+				 */
+				if (strcmp(dbname, "postgres") == 0)
+					fprintf(OPF, "\\connect %s\n", fmtId("template1"));
+				else
+					fprintf(OPF, "\\connect %s\n", fmtId("postgres"));
+
+				appendPQExpBuffer(buf, "ALTER DATABASE %s SET TABLESPACE %s;\n",
+								  fdbname, fmtId(dbtablespace));
+			}
+		}
 
 		if (binary_upgrade)
 		{
-- 
2.4.4

-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to