Re: pg_upgrade: transfer pg_largeobject_metadata's files when possible - Mailing list pgsql-hackers

On Thu, Aug 14, 2025 at 10:22:02AM -0500, Nathan Bossart wrote:
> Here is a patch.  For background, the reason this is limited to upgrades
> from v16 and newer is because the aclitem data type (needed by
> pg_largeobject_metadata.lomacl) changed its storage format in v16 (see
> commit 7b378237aa).  Note that the patch is essentially a revert of commit
> 12a53c732c, but there are enough differences that it should be considered a
> fresh effort.

Noted.

> Something I hadn't anticipated is that we need to take special care to
> transfer the relfilenode of pg_largeobject_metadata and its index, as was
> done for pg_largeobject in commits d498e052b4 and bbe08b8869.  In fact, the
> majority of the patch is dedicated to that.
>
> My testing showed some decent, but not earth-shattering performance
> improvements from this patch.  For upgrades with many large objects with
> NULL lomacl/lomowner columns, pg_upgrade was 50% faster.  With non-NULL
> lomacl/lomowner, that dropped to 25%.  When each large object had a
> comment, there was no change.  I'm assuming that its rare to have lots of
> large objects with comments or security labels, so I don't see any need to
> expend energy trying to optimize that use-case.

I highly doubt that there are a lot of comments assigned to LOs, so
these numbers are pretty cool IMO.  Security labels are a pain to test
in the upgrade path, or test_dummy_label could be extended with a new
TAP test and a pg_upgrade command..  There is some coverage with
comments on LOs in src/bin/pg_dump's 002, so that would be enough for
the comment part, at least.

> I am a bit concerned that we'll forget to add checks for new types of
> dependencies similar to comments and security labels.  If we do, pg_upgrade
> should just fail to restore the schema, and fixing the code should be easy
> enough.  Also, we'll need to remember to revisit this code if there's
> another storage format change for one of pg_largeobject_metadata's columns,
> but that seems unlikely to happen anytime soon.  On the whole, I'm not too
> worried about either of these points.

This part does not worry me much, TBH.  This stuff would require
dump/restore support and the pg_dump test suite would catch that for
commands with --binary-upgrade.  So it should be hard to miss.

-        /*
-         * pg_largeobject
-         */
         if (fout->remoteVersion >= 90300)
             appendPQExpBuffer(loFrozenQry, "SELECT relfrozenxid, relminmxid, relfilenode, oid\n"
                               "FROM pg_catalog.pg_class\n"
-                              "WHERE oid IN (%u, %u);\n",
-                              LargeObjectRelationId, LargeObjectLOidPNIndexId);
+                              "WHERE oid IN (%u, %u, %u, %u);\n",
+                              LargeObjectRelationId, LargeObjectLOidPNIndexId,
+                              LargeObjectMetadataRelationId, LargeObjectMetadataOidIndexId);
[...]
         appendPQExpBufferStr(loHorizonQry, "\n-- For binary upgrade, set pg_largeobject relfrozenxid and
relminmxid\n");
+        appendPQExpBufferStr(lomHorizonQry, "\n-- For binary upgrade, set pg_largeobject_metadata relfrozenxid and
relminmxid\n");
         appendPQExpBufferStr(loOutQry, "\n-- For binary upgrade, preserve pg_largeobject and index relfilenodes\n");
+        appendPQExpBufferStr(lomOutQry, "\n-- For binary upgrade, preserve pg_largeobject_metadata and index
relfilenodes\n");

Is all that really required when upgrading from a cluster in the
9.3~15 range?
--
Michael