Re: pgsql: Trial fix for old cross-version upgrades. - Mailing list pgsql-committers

From Tom Lane
Subject Re: pgsql: Trial fix for old cross-version upgrades.
Date
Msg-id 301492.1740249291@sss.pgh.pa.us
Whole thread Raw
In response to Re: pgsql: Trial fix for old cross-version upgrades.  (Andrew Dunstan <andrew@dunslane.net>)
Responses Re: pgsql: Trial fix for old cross-version upgrades.
Re: pgsql: Trial fix for old cross-version upgrades.
List pgsql-committers
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

--- PGBuild/Modules/TestUpgradeXversion.pm.orig    2024-11-02 01:47:21.000000000 -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 = $tmpdir;
         $tdir =~ s!\\!/!g;
@@ -431,7 +431,9 @@ sub test_upgrade    ## no critic (Subrou
         $param = "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 = '$tdir'\n";
+        print $opgconf "$param = '$tdir'\n"
+            if (!$using_localhost);
+        print $opgconf "autovacuum = 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 = "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 = ''\n";
-        print $pgconf "$tmp_param = '$tmpdir'\n";
+        unless ($using_localhost)
+        {
+            print $pgconf "listen_addresses = ''\n";
+            print $pgconf "$tmp_param = '$tmpdir'\n";
+        }
+        print $pgconf "autovacuum = 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")
--- /home/buildfarm/bf-data/upgrade.tester/HEAD/origin-REL9_2_STABLE.sql.fixed    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,

pgsql-committers by date:

Previous
From: Andrew Dunstan
Date:
Subject: Re: pgsql: Trial fix for old cross-version upgrades.
Next
From: Jeff Davis
Date:
Subject: pgsql: Documentation fixups for dumping statistics.