Re: pg_upgrade bug found! - Mailing list pgsql-hackers

Bruce Momjian wrote:
> OK, thanks to RhodiumToad on IRC, I was able to determine the cause of
> the two reported pg_upgrade problems he saw via IRC.  It seems toast
> tables have xids and pg_dump is not preserving the toast relfrozenxids
> as it should.  Heap tables have preserved relfrozenxids, but if you
> update a heap row but don't change the toast value, and the old heap row
> is later removed, the toast table can have an older relfrozenxids than
> the heap table.
>
> The fix for this is to have pg_dump preserve toast relfrozenxids, which
> can be easily added and backpatched.  We might want to push a 9.0.4 for
> this.  Second, we need to find a way for people to detect and fix
> existing systems that have this problem, perhaps looming when the
> pg_class relfrozenxid passes the toast relfrozenxid, and thirdly, we
> need to figure out how to get this information to users.  Perhaps the
> communication comes through the 9.0.4 release announcement.

I am not sure how to interpret the lack of replies to this email.
Either it is confidence, shock, or we told you so.  ;-)

Anyway, the attached patch fixes the problem.  The fix is for pg_dump's
binary upgrade mode.  This would need to be backpatched back to 8.4
because pg_migrator needs this too.

I have added a personal regression test to show which
pg_class.relfrozenxid values are not preserved, and with this patch the
only ones not preserved are toast tables used by system tables, which
are not copied from the old cluster (FirstNormalObjectId = 16384).  I am
attaching that old/new pg_class.relfrozenxid diff as well.

Any idea how to correct existing systems?  Would VACUUM FREEZE of just
the toast tables work?  I perhaps could create a short DO block that
would vacuum freeze just toast tables;  it would have to be run in every
database.

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

  + It's impossible for everything to be true. +
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
new file mode 100644
index 3f6e77b..1ccdb4d
*** a/src/bin/pg_dump/pg_dump.c
--- b/src/bin/pg_dump/pg_dump.c
*************** getTables(int *numTables)
*** 3812,3817 ****
--- 3812,3819 ----
      int            i_relhasrules;
      int            i_relhasoids;
      int            i_relfrozenxid;
+     int            i_toastoid;
+     int            i_toastfrozenxid;
      int            i_relpersistence;
      int            i_owning_tab;
      int            i_owning_col;
*************** getTables(int *numTables)
*** 3855,3861 ****
                            "(%s c.relowner) AS rolname, "
                            "c.relchecks, c.relhastriggers, "
                            "c.relhasindex, c.relhasrules, c.relhasoids, "
!                           "c.relfrozenxid, c.relpersistence, "
                            "CASE WHEN c.reloftype <> 0 THEN c.reloftype::pg_catalog.regtype ELSE NULL END AS
reloftype," 
                            "d.refobjid AS owning_tab, "
                            "d.refobjsubid AS owning_col, "
--- 3857,3865 ----
                            "(%s c.relowner) AS rolname, "
                            "c.relchecks, c.relhastriggers, "
                            "c.relhasindex, c.relhasrules, c.relhasoids, "
!                           "c.relfrozenxid, tc.oid AS toid, "
!                           "tc.relfrozenxid AS tfrozenxid, "
!                           "c.relpersistence, "
                            "CASE WHEN c.reloftype <> 0 THEN c.reloftype::pg_catalog.regtype ELSE NULL END AS
reloftype," 
                            "d.refobjid AS owning_tab, "
                            "d.refobjsubid AS owning_col, "
*************** getTables(int *numTables)
*** 3889,3895 ****
                            "(%s c.relowner) AS rolname, "
                            "c.relchecks, c.relhastriggers, "
                            "c.relhasindex, c.relhasrules, c.relhasoids, "
!                           "c.relfrozenxid, 'p' AS relpersistence, "
                            "CASE WHEN c.reloftype <> 0 THEN c.reloftype::pg_catalog.regtype ELSE NULL END AS
reloftype," 
                            "d.refobjid AS owning_tab, "
                            "d.refobjsubid AS owning_col, "
--- 3893,3901 ----
                            "(%s c.relowner) AS rolname, "
                            "c.relchecks, c.relhastriggers, "
                            "c.relhasindex, c.relhasrules, c.relhasoids, "
!                           "c.relfrozenxid, tc.oid AS toid, "
!                           "tc.relfrozenxid AS tfrozenxid, "
!                           "'p' AS relpersistence, "
                            "CASE WHEN c.reloftype <> 0 THEN c.reloftype::pg_catalog.regtype ELSE NULL END AS
reloftype," 
                            "d.refobjid AS owning_tab, "
                            "d.refobjsubid AS owning_col, "
*************** getTables(int *numTables)
*** 3922,3928 ****
                            "(%s c.relowner) AS rolname, "
                            "c.relchecks, c.relhastriggers, "
                            "c.relhasindex, c.relhasrules, c.relhasoids, "
!                           "c.relfrozenxid, 'p' AS relpersistence, "
                            "NULL AS reloftype, "
                            "d.refobjid AS owning_tab, "
                            "d.refobjsubid AS owning_col, "
--- 3928,3936 ----
                            "(%s c.relowner) AS rolname, "
                            "c.relchecks, c.relhastriggers, "
                            "c.relhasindex, c.relhasrules, c.relhasoids, "
!                           "c.relfrozenxid, tc.oid AS toid, "
!                           "tc.relfrozenxid AS tfrozenxid, "
!                           "'p' AS relpersistence, "
                            "NULL AS reloftype, "
                            "d.refobjid AS owning_tab, "
                            "d.refobjsubid AS owning_col, "
*************** getTables(int *numTables)
*** 3955,3961 ****
                            "(%s relowner) AS rolname, "
                            "relchecks, (reltriggers <> 0) AS relhastriggers, "
                            "relhasindex, relhasrules, relhasoids, "
!                           "relfrozenxid, 'p' AS relpersistence, "
                            "NULL AS reloftype, "
                            "d.refobjid AS owning_tab, "
                            "d.refobjsubid AS owning_col, "
--- 3963,3972 ----
                            "(%s relowner) AS rolname, "
                            "relchecks, (reltriggers <> 0) AS relhastriggers, "
                            "relhasindex, relhasrules, relhasoids, "
!                           "relfrozenxid, "
!                           "0 AS toid, "
!                           "0 AS tfrozenxid, "
!                           "'p' AS relpersistence, "
                            "NULL AS reloftype, "
                            "d.refobjid AS owning_tab, "
                            "d.refobjsubid AS owning_col, "
*************** getTables(int *numTables)
*** 3987,3993 ****
                            "(%s relowner) AS rolname, "
                            "relchecks, (reltriggers <> 0) AS relhastriggers, "
                            "relhasindex, relhasrules, relhasoids, "
!                           "0 AS relfrozenxid, 'p' AS relpersistence, "
                            "NULL AS reloftype, "
                            "d.refobjid AS owning_tab, "
                            "d.refobjsubid AS owning_col, "
--- 3998,4007 ----
                            "(%s relowner) AS rolname, "
                            "relchecks, (reltriggers <> 0) AS relhastriggers, "
                            "relhasindex, relhasrules, relhasoids, "
!                           "0 AS relfrozenxid, "
!                           "0 AS toid, "
!                           "0 AS tfrozenxid, "
!                           "'p' AS relpersistence, "
                            "NULL AS reloftype, "
                            "d.refobjid AS owning_tab, "
                            "d.refobjsubid AS owning_col, "
*************** getTables(int *numTables)
*** 4019,4025 ****
                            "(%s relowner) AS rolname, "
                            "relchecks, (reltriggers <> 0) AS relhastriggers, "
                            "relhasindex, relhasrules, relhasoids, "
!                           "0 AS relfrozenxid, 'p' AS relpersistence, "
                            "NULL AS reloftype, "
                            "d.refobjid AS owning_tab, "
                            "d.refobjsubid AS owning_col, "
--- 4033,4042 ----
                            "(%s relowner) AS rolname, "
                            "relchecks, (reltriggers <> 0) AS relhastriggers, "
                            "relhasindex, relhasrules, relhasoids, "
!                           "0 AS relfrozenxid, "
!                           "0 AS toid, "
!                           "0 AS tfrozenxid, "
!                           "'p' AS relpersistence, "
                            "NULL AS reloftype, "
                            "d.refobjid AS owning_tab, "
                            "d.refobjsubid AS owning_col, "
*************** getTables(int *numTables)
*** 4047,4053 ****
                            "(%s relowner) AS rolname, "
                            "relchecks, (reltriggers <> 0) AS relhastriggers, "
                            "relhasindex, relhasrules, relhasoids, "
!                           "0 AS relfrozenxid, 'p' AS relpersistence, "
                            "NULL AS reloftype, "
                            "NULL::oid AS owning_tab, "
                            "NULL::int4 AS owning_col, "
--- 4064,4073 ----
                            "(%s relowner) AS rolname, "
                            "relchecks, (reltriggers <> 0) AS relhastriggers, "
                            "relhasindex, relhasrules, relhasoids, "
!                           "0 AS relfrozenxid, "
!                           "0 AS toid, "
!                           "0 AS tfrozenxid, "
!                           "'p' AS relpersistence, "
                            "NULL AS reloftype, "
                            "NULL::oid AS owning_tab, "
                            "NULL::int4 AS owning_col, "
*************** getTables(int *numTables)
*** 4070,4076 ****
                            "relchecks, (reltriggers <> 0) AS relhastriggers, "
                            "relhasindex, relhasrules, "
                            "'t'::bool AS relhasoids, "
!                           "0 AS relfrozenxid, 'p' AS relpersistence, "
                            "NULL AS reloftype, "
                            "NULL::oid AS owning_tab, "
                            "NULL::int4 AS owning_col, "
--- 4090,4099 ----
                            "relchecks, (reltriggers <> 0) AS relhastriggers, "
                            "relhasindex, relhasrules, "
                            "'t'::bool AS relhasoids, "
!                           "0 AS relfrozenxid, "
!                           "0 AS toid, "
!                           "0 AS tfrozenxid, "
!                           "'p' AS relpersistence, "
                            "NULL AS reloftype, "
                            "NULL::oid AS owning_tab, "
                            "NULL::int4 AS owning_col, "
*************** getTables(int *numTables)
*** 4103,4109 ****
                            "relchecks, (reltriggers <> 0) AS relhastriggers, "
                            "relhasindex, relhasrules, "
                            "'t'::bool AS relhasoids, "
!                           "0 as relfrozenxid, 'p' AS relpersistence, "
                            "NULL AS reloftype, "
                            "NULL::oid AS owning_tab, "
                            "NULL::int4 AS owning_col, "
--- 4126,4135 ----
                            "relchecks, (reltriggers <> 0) AS relhastriggers, "
                            "relhasindex, relhasrules, "
                            "'t'::bool AS relhasoids, "
!                           "0 as relfrozenxid, "
!                           "0 AS toid, "
!                           "0 AS tfrozenxid, "
!                           "'p' AS relpersistence, "
                            "NULL AS reloftype, "
                            "NULL::oid AS owning_tab, "
                            "NULL::int4 AS owning_col, "
*************** getTables(int *numTables)
*** 4149,4154 ****
--- 4175,4182 ----
      i_relhasrules = PQfnumber(res, "relhasrules");
      i_relhasoids = PQfnumber(res, "relhasoids");
      i_relfrozenxid = PQfnumber(res, "relfrozenxid");
+     i_toastoid = PQfnumber(res, "toid");
+     i_toastfrozenxid = PQfnumber(res, "tfrozenxid");
      i_relpersistence = PQfnumber(res, "relpersistence");
      i_owning_tab = PQfnumber(res, "owning_tab");
      i_owning_col = PQfnumber(res, "owning_col");
*************** getTables(int *numTables)
*** 4190,4195 ****
--- 4218,4225 ----
          tblinfo[i].hastriggers = (strcmp(PQgetvalue(res, i, i_relhastriggers), "t") == 0);
          tblinfo[i].hasoids = (strcmp(PQgetvalue(res, i, i_relhasoids), "t") == 0);
          tblinfo[i].frozenxid = atooid(PQgetvalue(res, i, i_relfrozenxid));
+         tblinfo[i].toast_oid = atooid(PQgetvalue(res, i, i_toastoid));
+         tblinfo[i].toast_frozenxid = atooid(PQgetvalue(res, i, i_toastfrozenxid));
          if (PQgetisnull(res, i, i_reloftype))
              tblinfo[i].reloftype = NULL;
          else
*************** dumpTableSchema(Archive *fout, TableInfo
*** 12221,12233 ****
                  }
              }

!             appendPQExpBuffer(q, "\n-- For binary upgrade, set relfrozenxid\n");
              appendPQExpBuffer(q, "UPDATE pg_catalog.pg_class\n"
                                "SET relfrozenxid = '%u'\n"
                                "WHERE oid = ",
                                tbinfo->frozenxid);
              appendStringLiteralAH(q, fmtId(tbinfo->dobj.name), fout);
              appendPQExpBuffer(q, "::pg_catalog.regclass;\n");
          }

          /* Loop dumping statistics and storage statements */
--- 12251,12273 ----
                  }
              }

!             appendPQExpBuffer(q, "\n-- For binary upgrade, set heap's relfrozenxid\n");
              appendPQExpBuffer(q, "UPDATE pg_catalog.pg_class\n"
                                "SET relfrozenxid = '%u'\n"
                                "WHERE oid = ",
                                tbinfo->frozenxid);
              appendStringLiteralAH(q, fmtId(tbinfo->dobj.name), fout);
              appendPQExpBuffer(q, "::pg_catalog.regclass;\n");
+
+             if (tbinfo->toast_oid)
+             {
+                 /* We preserve the toast oids, so we can use it during restore */
+                 appendPQExpBuffer(q, "\n-- For binary upgrade, set toast's relfrozenxid\n");
+                 appendPQExpBuffer(q, "UPDATE pg_catalog.pg_class\n"
+                                   "SET relfrozenxid = '%u'\n"
+                                   "WHERE oid = '%u';\n",
+                                   tbinfo->toast_frozenxid, tbinfo->toast_oid);
+             }
          }

          /* Loop dumping statistics and storage statements */
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
new file mode 100644
index 113ecb1..6559e23
*** a/src/bin/pg_dump/pg_dump.h
--- b/src/bin/pg_dump/pg_dump.h
*************** typedef struct _tableInfo
*** 248,253 ****
--- 248,255 ----
      bool        hastriggers;    /* does it have any triggers? */
      bool        hasoids;        /* does it have OIDs? */
      uint32        frozenxid;        /* for restore frozen xid */
+     Oid            toast_oid;        /* for restore toast frozen xid */
+     uint32        toast_frozenxid;/* for restore toast frozen xid */
      int            ncheck;            /* # of CHECK expressions */
      char       *reloftype;        /* underlying type for typed table */
      /* these two are set only if table is a sequence owned by a column: */
3c3
< postgres|654
---
> postgres|1457
6,7c6,7
< template0|654
< template1|654
---
> template0|1457
> template1|1457
11,19c11,19
< pg_toast|pg_toast_11454|654
< pg_toast|pg_toast_11459|654
< pg_toast|pg_toast_11464|654
< pg_toast|pg_toast_11469|654
< pg_toast|pg_toast_11474|654
< pg_toast|pg_toast_11479|654
< pg_toast|pg_toast_11484|654
< pg_toast|pg_toast_1255|654
< pg_toast|pg_toast_1262|654
---
> pg_toast|pg_toast_11511|1457
> pg_toast|pg_toast_11516|1457
> pg_toast|pg_toast_11521|1457
> pg_toast|pg_toast_11526|1457
> pg_toast|pg_toast_11531|1457
> pg_toast|pg_toast_11536|1457
> pg_toast|pg_toast_11541|1457
> pg_toast|pg_toast_1255|1457
> pg_toast|pg_toast_1262|1457
96,103c96,104
< pg_toast|pg_toast_2396|654
< pg_toast|pg_toast_2604|654
< pg_toast|pg_toast_2606|654
< pg_toast|pg_toast_2609|654
< pg_toast|pg_toast_2618|654
< pg_toast|pg_toast_2619|654
< pg_toast|pg_toast_2620|654
< pg_toast|pg_toast_2964|654
---
> pg_toast|pg_toast_2396|1457
> pg_toast|pg_toast_2604|1457
> pg_toast|pg_toast_2606|1457
> pg_toast|pg_toast_2609|1457
> pg_toast|pg_toast_2618|1457
> pg_toast|pg_toast_2619|1457
> pg_toast|pg_toast_2620|1457
> pg_toast|pg_toast_2964|1457
> pg_toast|pg_toast_3596|1457
279c280
< (268 rows)
---
> (269 rows)