From: Tom Lane <tgl@sss.pgh.pa.us>
To: Andrew Dunstan <andrew@dunslane.net>
cc: Jeff Davis <pgsql@j-davis.com>, Jeff Davis <jdavis@postgresql.org>,
        pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Trial fix for old cross-version upgrades.
In-reply-to: <976dcc37-b629-490e-a052-a057477d062f@dunslane.net>
References: <E1tlBJt-000LUO-16@gemulon.postgresql.org>
 <003dc9936317ab987faa0242f8e33e1cd2fcaf57.camel@j-davis.com>
 <40c1e76334f2baa747334c8fc513d12b8cb297bf.camel@j-davis.com>
 <3815127.1740189601@sss.pgh.pa.us>
 <5ebc188e731ac2b98d68459ce1a9ef3066981774.camel@j-davis.com>
 <3892121.1740193866@sss.pgh.pa.us>
 <976dcc37-b629-490e-a052-a057477d062f@dunslane.net>
Comments: In-reply-to Andrew Dunstan <andrew@dunslane.net>
	message dated "Sat, 22 Feb 2025 10:16:43 -0500"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <296356.1740249201.0@sss.pgh.pa.us>
Date: Sat, 22 Feb 2025 13:34:51 -0500
Message-ID: <301492.1740249291@sss.pgh.pa.us>
Archived-At: 
 <https://www.postgresql.org/message-id/301492.1740249291%40sss.pgh.pa.us>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <296356.1740249201.1@sss.pgh.pa.us>

Andrew Dunstan <andrew@dunslane.net> writes:
> On 2025-02-21 Fr 10:11 PM, Tom Lane wrote:
>> ... It seems there is
>> something different between what TestUpgradeXversion.pm is doing
>> and what 002_pg_upgrade.pl is doing.  No clue what, although it
>> does look like an additional round of analyze'ing has added more
>> stats than were there before.

Hah!  Looking at the script with less bleary eyes, I see it does
this after pg_upgrade:

    if (-e "$installdir/analyze_new_cluster.sh")
    {
        system( "cd $installdir && sh ./analyze_new_cluster.sh "
              . qq{> "$upgrade_loc/$oversion-analyse.log" 2>&1 });
        return if $?;
    }
    else
    {
        system( qq{"$installdir/bin/vacuumdb" --all --analyze-only }
              . qq{> "$upgrade_loc/$oversion-analyse.log" 2>&1 });
        return if $?;
    }

So there's our extra round of ANALYZE right there.  I diked out the
vacuumdb call and things are working much better.  It seems to pass
for branches back through 9.3, and upgrade from 9.2 has only some
diffs for relallvisible (see attached).  We still need to figure out
why that is, but a quick-n-dirty patch could just be to make the dump
comparison logic ignore relallvisible diffs.

We might want to make this vacuumdb invocation dependent on version,
but I could also see just removing it entirely.

> Here's what I have so far:

> . for HEAD/18 disable running the analyze script / vacuumdb --analyze.
> . turn off autovacuum on the old and upgraded database.
> . reverse the order of testing so we do newest first

Check.

> What I'm thinking of doing is running all the eligible upgrades rather 
> than stopping on the first failure.

Seems like possibly wasted work --- I'd be content with
newest-to-oldest.

			regards, tom lane


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="hasty-xversion.patch"; charset="us-ascii"
Content-ID: <296356.1740249201.2@sss.pgh.pa.us>
Content-Description: hasty-xversion.patch
Content-Transfer-Encoding: quoted-printable

--- PGBuild/Modules/TestUpgradeXversion.pm.orig	2024-11-02 01:47:21.000000=
000 -0400
+++ PGBuild/Modules/TestUpgradeXversion.pm	2025-02-22 12:11:31.924183704 -=
0500
@@ -419,7 +419,7 @@ sub test_upgrade    ## no critic (Subrou
 	# run in which it was set up, which will be gone by now, so we repoint
 	# it to the current run's tmpdir.
 	# listen_addresses will be set correctly and requires no adjustment.
-	if (!$using_localhost)
+	# In any case, disable autovacuum to prevent stats changing under us.
 	{
 		my $tdir =3D $tmpdir;
 		$tdir =3D~ s!\\!/!g;
@@ -431,7 +431,9 @@ sub test_upgrade    ## no critic (Subrou
 		$param =3D "unix_socket_directory"
 		  if $oversion ne 'HEAD' && $oversion lt 'REL9_3_STABLE';
 		print $opgconf "\n# Configuration added by buildfarm client\n\n";
-		print $opgconf "$param =3D '$tdir'\n";
+		print $opgconf "$param =3D '$tdir'\n"
+		    if (!$using_localhost);
+		print $opgconf "autovacuum =3D off\n";
 		close($opgconf);
 	}
 =

@@ -507,7 +509,7 @@ sub test_upgrade    ## no critic (Subrou
 		  . qq{> "$upgrade_loc/$oversion-initdb.log" 2>&1});
 	return if $?;
 =

-	unless ($using_localhost)
+	# Again, adjust connection location and disable autovacuum.
 	{
 		open(my $pgconf, ">>", "$installdir/$oversion-upgrade/postgresql.conf")
 		  || die "opening $installdir/$oversion-upgrade/postgresql.conf: $!";
@@ -515,8 +517,12 @@ sub test_upgrade    ## no critic (Subrou
 		$tmp_param =3D "unix_socket_directory"
 		  if $this_branch ne 'HEAD' && $this_branch lt 'REL9_3_STABLE';
 		print $pgconf "\n# Configuration added by buildfarm client\n\n";
-		print $pgconf "listen_addresses =3D ''\n";
-		print $pgconf "$tmp_param =3D '$tmpdir'\n";
+		unless ($using_localhost)
+		{
+		    print $pgconf "listen_addresses =3D ''\n";
+		    print $pgconf "$tmp_param =3D '$tmpdir'\n";
+		}
+		print $pgconf "autovacuum =3D off\n";
 		close($pgconf);
 	}
 =

@@ -573,9 +579,9 @@ sub test_upgrade    ## no critic (Subrou
 	}
 	else
 	{
-		system( qq{"$installdir/bin/vacuumdb" --all --analyze-only }
-			  . qq{> "$upgrade_loc/$oversion-analyse.log" 2>&1 });
-		return if $?;
+#		system( qq{"$installdir/bin/vacuumdb" --all --analyze-only }
+#			  . qq{> "$upgrade_loc/$oversion-analyse.log" 2>&1 });
+#		return if $?;
 	}
 =

 	if (-e "$installdir/reindex_hash.sql")

------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="dumpdiff-REL9_2_STABLE";
	charset="us-ascii"
Content-ID: <296356.1740249201.3@sss.pgh.pa.us>
Content-Description: dumpdiff-REL9_2_STABLE
Content-Transfer-Encoding: quoted-printable

--- /home/buildfarm/bf-data/upgrade.tester/HEAD/origin-REL9_2_STABLE.sql.f=
ixed	2025-02-22 12:55:24.073548904 -0500
+++ /home/buildfarm/bf-data/upgrade.tester/HEAD/converted-REL9_2_STABLE-to=
-HEAD.sql.fixed	2025-02-22 12:55:24.074548905 -0500
@@ -208450,7 +208450,7 @@
 	'version', '000000'::integer,
 	'relpages', '55'::integer,
 	'reltuples', '10000'::real,
-	'relallvisible', '55'::integer
+	'relallvisible', '53'::integer
 );
 SELECT * FROM pg_catalog.pg_restore_attribute_stats(
 	'relation', 'public.hash_f8_heap'::regclass,
@@ -208482,7 +208482,7 @@
 	'version', '000000'::integer,
 	'relpages', '45'::integer,
 	'reltuples', '10000'::real,
-	'relallvisible', '45'::integer
+	'relallvisible', '42'::integer
 );
 SELECT * FROM pg_catalog.pg_restore_attribute_stats(
 	'relation', 'public.hash_i4_heap'::regclass,
@@ -208514,7 +208514,7 @@
 	'version', '000000'::integer,
 	'relpages', '124'::integer,
 	'reltuples', '10000'::real,
-	'relallvisible', '124'::integer
+	'relallvisible', '122'::integer
 );
 SELECT * FROM pg_catalog.pg_restore_attribute_stats(
 	'relation', 'public.hash_name_heap'::regclass,
@@ -208546,7 +208546,7 @@
 	'version', '000000'::integer,
 	'relpages', '55'::integer,
 	'reltuples', '10000'::real,
-	'relallvisible', '55'::integer
+	'relallvisible', '52'::integer
 );
 SELECT * FROM pg_catalog.pg_restore_attribute_stats(
 	'relation', 'public.hash_txt_heap'::regclass,

------- =_aaaaaaaaaa0--