Date: Wed, 18 Jun 2014 21:40:34 -0400
From: Bruce Momjian <bruce@momjian.us>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Andres Freund <andres@2ndquadrant.com>,
 PostgreSQL Bugs <pgsql-bugs@postgresql.org>
Subject: Re: pg_upgrade < 9.3 -> >=9.3 misses a step around
 multixacts
Message-ID: <20140619014034.GC3806@momjian.us>
References: <20140530121631.GE25431@alap3.anarazel.de>
 <20140530132916.GM28490@momjian.us>
 <20140618225131.GN18688@eldon.alvh.no-ip.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="HcAYCG3uE/tztfnV"
Content-Disposition: inline
In-Reply-To: <20140618225131.GN18688@eldon.alvh.no-ip.org>
User-Agent: Mutt/1.5.20 (2009-06-14)
Precedence: bulk
Sender: pgsql-bugs-owner@postgresql.org


--HcAYCG3uE/tztfnV
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

On Wed, Jun 18, 2014 at 06:51:31PM -0400, Alvaro Herrera wrote:
> Bruce Momjian wrote:
> > On Fri, May 30, 2014 at 02:16:31PM +0200, Andres Freund wrote:
> > > Hi,
> > > 
> > > When upgrading a < 9.3 cluster pg_upgrade doesn't bother to keep the old
> > > multixacts around because they won't be read after the upgrade (and
> > > aren't compatible). It just resets the new cluster's nextMulti to the
> > > old + 1.
> > > Unfortunately that means that there'll be a offsets/0000 file created by
> > > initdb around. Sounds harmless enough, but that'll actually cause
> > > problems if the old cluster had a nextMulti that's bigger than that
> > > page.
> 
> I've been playing with this a bit.  Here's a patch that just does
> rmtree() of the problematic file during pg_upgrade, as proposed by
> Andres, which solves the problem.  Note that this patch removes the 0000
> file in both cases: when upgrading from 9.2, and when upgrading from
> 9.3+.  The former case is the bug that Andres reported.  In the second
> case, we overwrite the files with the ones from the old cluster; if
> there's a lingering 0000 file in the new cluster, it would cause
> problems as well.  (Really, I don't see any reason to think that these
> two cases are any different.)

I wasn't happy with having that delete code added there when we do
directory delete in the function above.  I instead broke apart the
delete and copy code and called the delete code where needed, in the
attached patch.

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

  + Everyone has their own god. +

--HcAYCG3uE/tztfnV
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment; filename="pg_upgrade.diff"

diff --git a/contrib/pg_upgrade/pg_upgrade.c b/contrib/pg_upgrade/pg_upgrade.c
new file mode 100644
index 773bb07..d1f302a
*** a/contrib/pg_upgrade/pg_upgrade.c
--- b/contrib/pg_upgrade/pg_upgrade.c
*************** create_new_objects(void)
*** 363,384 ****
  }
  
  /*
!  * Delete the given subdirectory contents from the new cluster, and copy the
!  * files from the old cluster into it.
   */
  static void
! copy_subdir_files(char *subdir)
  {
- 	char		old_path[MAXPGPATH];
  	char		new_path[MAXPGPATH];
  
  	prep_status("Deleting files from new %s", subdir);
  
- 	snprintf(old_path, sizeof(old_path), "%s/%s", old_cluster.pgdata, subdir);
  	snprintf(new_path, sizeof(new_path), "%s/%s", new_cluster.pgdata, subdir);
! 	if (!rmtree(new_path, true))
  		pg_fatal("could not delete directory \"%s\"\n", new_path);
  	check_ok();
  
  	prep_status("Copying old %s to new server", subdir);
  
--- 363,397 ----
  }
  
  /*
!  * Delete the given subdirectory contents from the new cluster
   */
  static void
! rm_new_subdir_files(char *subdir, bool rmtopdir)
  {
  	char		new_path[MAXPGPATH];
  
  	prep_status("Deleting files from new %s", subdir);
  
  	snprintf(new_path, sizeof(new_path), "%s/%s", new_cluster.pgdata, subdir);
! 	if (!rmtree(new_path, rmtopdir))
  		pg_fatal("could not delete directory \"%s\"\n", new_path);
+ 
  	check_ok();
+ }
+ 
+ /*
+  * Copy the files from the old cluster into it
+  */
+ static void
+ copy_subdir_files(char *subdir)
+ {
+ 	char		old_path[MAXPGPATH];
+ 	char		new_path[MAXPGPATH];
+ 
+ 	rm_new_subdir_files(subdir, true);
+ 
+ 	snprintf(old_path, sizeof(old_path), "%s/%s", old_cluster.pgdata, subdir);
+ 	snprintf(new_path, sizeof(new_path), "%s/%s", new_cluster.pgdata, subdir);
  
  	prep_status("Copying old %s to new server", subdir);
  
*************** copy_clog_xlog_xid(void)
*** 419,424 ****
--- 432,438 ----
  	{
  		copy_subdir_files("pg_multixact/offsets");
  		copy_subdir_files("pg_multixact/members");
+ 
  		prep_status("Setting next multixact ID and offset for new cluster");
  
  		/*
*************** copy_clog_xlog_xid(void)
*** 436,441 ****
--- 450,462 ----
  	}
  	else if (new_cluster.controldata.cat_ver >= MULTIXACT_FORMATCHANGE_CAT_VER)
  	{
+ 		/*
+ 		 * Remove files created by initdb that no longer match the
+ 		 * new multi-xid value.
+ 		 */
+ 		rm_new_subdir_files("pg_multixact/offsets", false);
+ 		rm_new_subdir_files("pg_multixact/members", false);
+ 
  		prep_status("Setting oldest multixact ID on new cluster");
  
  		/*

--HcAYCG3uE/tztfnV
Content-Type: text/plain
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0


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

--HcAYCG3uE/tztfnV--