Re: pg15b2: large objects lost on upgrade

Andres Freund <andres@anarazel.de>

From: Andres Freund <andres@anarazel.de>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, "Jonathan S. Katz" <jkatz@postgresql.org>, Noah Misch <noah@leadboat.com>, Andrew Dunstan <andrew@dunslane.net>, Bruce Momjian <bruce@momjian.us>, Michael Paquier <michael@paquier.xyz>, Justin Pryzby <pryzby@telsasoft.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>, Shruthi Gowda <gowdashru@gmail.com>
Date: 2022-08-03T20:20:14Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →
  1. Revert recent changes to 002_pg_upgrade.pl.

  2. Revise test case added in 43746996399541ecb5c7b188725a5f097c15ceae.

  3. Use TRUNCATE to preserve relfilenode for pg_largeobject + index.

  4. Preserve relfilenode of pg_largeobject and its index across pg_upgrade.

  5. Have VACUUM warn on relfrozenxid "in the future".

  6. Set relfrozenxid to oldest extant XID seen by VACUUM.

  7. pg_upgrade: Preserve relfilenodes and tablespace OIDs.

  8. Perform a lot more sanity checks when freezing tuples.

Hi,

On 2022-08-03 09:59:40 -0400, Robert Haas wrote:
> On Tue, Aug 2, 2022 at 3:51 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> > > The test does look helpful and it would catch regressions. Loosely
> > > quoting Robert on a different point upthread, we don't want to turn off
> > > the alarm just because it's spuriously going off.
> > > I think the weakened check is OK (and possibly mimics the real-world
> > > where autovacuum runs), unless you see a major drawback to it?
> >
> > I also think that ">=" is a sufficient requirement.  It'd be a
> > bit painful to test if we had to cope with potential XID wraparound,
> > but we know that these installations haven't been around nearly
> > long enough for that, so a plain ">=" test ought to be good enough.
> > (Replacing the simple "eq" code with something that can handle that
> > doesn't look like much fun, though.)
> 
> I don't really like this approach. Imagine that the code got broken in
> such a way that relfrozenxid and relminmxid were set to a value chosen
> at random - say, the contents of 4 bytes of unallocated memory that
> contained random garbage. Well, right now, the chances that this would
> cause a test failure are nearly 100%. With this change, they'd be
> nearly 0%.

Can't that pretty easily be addressed by subsequently querying txid_current(),
and checking that the value isn't newer than that?

Greetings,

Andres Freund