Subject: Horribly slow pg_upgrade performance with many Large Objects

From: Hannu Krosing <[email-masked]>
Date: Mon, 7 Apr 2025 22:33:47 +0200

Hi Hackers

## The issue

I have now met a not insignificant number of cases where pg_upgrade
performance is really bad when the database has a large number of
Large Objects.

The average time to `pg_dump --binary-upgrade --format=custom ...` a
database and then `pg_restore ...` it back is 1 minute per 1 million
LOs,

This does not seem to bad until you realize that it is
- 100 minutes, ot 1H40min for 100M LOs
- 16H40 min for 1 Billion large objects .
- 66H40m or 4 2days 18 hours and 40 min for those unfortunate enough
who have to full 4 Billion LOs

My direct tests have 10m5s for 10M LOs and 100m54s for 100M.

Also a smaller server just runs out of shared memory when restoring 100M LOs.

I know that our  semi-official recommendation for using Large Objects
is "Don't!" but still people use them and sometimes they have a lot of
them. One of the reasons seems to be an older version of Hibernate
which converted any column of type CLOB into a Large Object, so we see
cases where average LO size is a few hundred to a few thousand bytes.

I read through the old discussions around the time of release of v12
and the consensus seemed to be at that time that dumping the LOs
without data was ok "because there will not be too many LOs" which
unfortunately has turned out to be wishful thinking, as there are lots
of  Tiny Large Object out there and these really slow down pg_upgrade.

## So what to do about this?

The obvious solution would be to handle the table
`pg_largeobject_metadata` the same way as we currently handle
`pg_largeobject `by not doing anything with it in `pg_dump
--binary-upgrade` and just handle the contents it like we do for user
tables in pg_upgrade itself.

This should work fine for all source database versions starting from PgSQL v12.

For older supported versions 9.2 to 11 where `oid` was system column
we should just dump the three fields of `pg_largeobject_metadata`
directly so that the dump command would be  `COPY (SELECT oid,
lomowner, lomacl) TO stdout` and then restore would happen more or
less automatically.

Or we could just modify `pg_restore` so that it restores LOs using
COPY instead of doing the current dance of 3 separate SELECT's from
lo_xxx() functions.

pg_dump --binary-upgrade format=custome is relatively fast for LOs -
1m58s for 100M object,

though still much slower than `pg_dump --data-only -t
pg_largeobject_metadata` which is 20 sec, or 23 when also run through
gzip

## preferred solution for --binary-upgrade

- link/copy data files when source is v12+
- dump the triplet of pg_largeobject_metadata columns if source is v9.2 - v11

an acceptable faster solution would be to just always dump
pg_largeobject_metadata (oid,  lomowner, lomacl) as this would not
need any changes to `pg_upgrade`, just to `pg_dump --binary-upgrade`
it would not be as fast as linking/copying but would be several orders
of magnitude faster than current behaviour.

Does anyone have any objections to this ?

Would anyone be willing to take up fixing this ?

Or perhaps advise where in code the change should be done ?

I started looking at this and at high level It looks like I need to
create a task to copy out specific data columns  and not the structure
of pg_largeobject metadata but as pg_dump.c is 582,673 it would take
some time to understand where exactly to do this and all the possible
interactions with flags like --binary-upgrade and specific formats.

---
Cheers,
Hannu

---

From: Nathan Bossart <[email-masked]>
Date: Mon, 7 Apr 2025 17:17:03 -0500

> This should work fine for all source database versions starting from PgSQL v12.

Unfortunately, the storage format for aclitem changed in v16, so this would
need to be restricted to upgrades from v16 and newer.  That being said, I
regularly hear about slow upgrades with many LOs, so I think it'd be
worthwhile to try to improve matters in v19.

-- 
nathan

---

From: Michael Paquier <[email-masked]>
Date: Tue, 8 Apr 2025 13:52:12 +0900

On Mon, Apr 07, 2025 at 05:25:32PM -0400, Tom Lane wrote:
> What version are you testing?  We did some work in that area in the
> v17 cycle (a45c78e32).

I am puzzled by the target version used here, as well.  If there is
more that can be improved, v19 would be the version to consider for
future improvements at this stage.
--
Michael

---

From: Hannu Krosing <[email-masked]>
Date: Tue, 8 Apr 2025 09:15:45 +0200

I was testing on version 17

On Tue, Apr 8, 2025 at 6:52 AM Michael Paquier <[email-masked]> wrote:
> I am puzzled by the target version used here, as well.

I was testing on version 17

Here is how you can easily test too (as --binary-upgrade does not dump
the actual data it is ok for the test to not put anything there)

hannuk@db01-c1a:~/work/lo-testing$ createdb -p 5433 lodb
hannuk@db01-c1a:~/work/lo-testing$ psql -p 5433 lodb
psql (17.4 (Ubuntu 17.4-1.pgdg22.04+2))
Type "help" for help.

lodb=# insert into pg_largeobject_metadata(oid, lomowner) SELECT i,
16384 FROM generate_series(1, 100_000_000) g(i);
INSERT 0 100000000
Time: 162414.216 ms (02:42.414)
lodb=#
\q

hannuk@db01-c1a:~/work/lo-testing$ time pg_dump --data-only -t
pg_largeobject_metadata -p 5433 lodb | gzip >
pg_largeobject_metadata.data.gz
real 0m22.094s
user 0m20.741s
sys 0m2.085s

hannuk@db01-c1a:~/work/lo-testing$ time pg_dump --data-only -t
pg_largeobject_metadata --format=custom -p 5433 lodb -f
pg_largeobject_metadata.dump
real 0m20.226s
user 0m18.068s
sys 0m0.824s

>  If there is

If the internal format has changed in 16 the correct way would be to
go through the data-only dump of pg_largeobject_metadata in all cases.
Even for the 100M case where you get the restore in 2 minutes instead
of 100 minutes

hannuk@db01-c1a:~/work/lo-testing$ createdb -p 5434 lodb
hannuk@db01-c1a:~/work/lo-testing$ time pg_restore -p 5434
--exit-on-error --transaction-size=1000 --dbname lodb
pg_largeobject_metadata.dump

real 2m2.277s
user 0m2.594s
sys 0m0.549s

And even in case of the user-visible format change in acl format it is
most likely that changing the visible format using some regexp magic,
or even a dedicated function, would still me much faster than creating
all the LOs though creation commands.

------
The commands I used to do the pg_upgrade-like test were

hannuk@db01-c1a:~/work/lo-testing$ time pg_dump --schema-only
--quote-all-identifiers --binary-upgrade --format=custom
--file=lodb100m.dump -p 5433 lodb
real 1m58.241s
user 0m35.229s
sys 0m17.854s

hannuk@db01-c1a:~/work/lo-testing$ time pg_restore -p 5434
--exit-on-error --transaction-size=1000 --dbname lodb lodb100m.dump
real    100m54.878s
user    3m23.885s
sys     20m33.761s

(I left out the --verbose part that pg_upgrade also sets as I did not
want to get 100M lines of  "large object created " messages )

also the postgres server at -p 5434 needs to be started with -b flag
to accept the loading a dump from --binary-upgrade. In Debian/Ubuntu
this can be directly passed to pg_ctlcluster as follows

sudo pg_ctlcluster 17 target -o -b

----
Hannu

---

From: Hannu Krosing <[email-masked]>
Date: Tue, 8 Apr 2025 09:26:02 +0200

Looked like a bit illogical order on re-reading it so I want to make
clear that the pg_upgrade-like test showing 100min for 100 million LOs
is at the end of last message and the proposed solution is at the
beginning

---

From: Hannu Krosing <[email-masked]>
Date: Tue, 8 Apr 2025 09:35:24 +0200

On Tue, Apr 8, 2025 at 12:17 AM Nathan Bossart <[email-masked]> wrote:
> need to be restricted to upgrades from v16 and newer.

Have we also changed the external format of aclitem any time since v
9.2 or are the changes just to storage ?

If external formats have been stable we can still get reasonable
performance with dumping the data (2 min for 100M rows)
Plus dumping data would work for all the supported source versions.

The worst case would still be quite bad with 80+ min for the full set
of 4 billion LOs but even that would be much better than the 3 days
with current wayd.

> That being said, I

Changing the LO export to dumping pg_largeobject_metadata content
instead of creating the LOs should be a nice small change confined to
pg_dump --binary-upgrade only so perhaps we could squeeze it in v18
still.

--
Hannu

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 8 Apr 2025 10:46:41 -0500

> pg_dump --binary-upgrade only so perhaps we could squeeze it in v18

Feature freeze for v18 was ~4 hours ago, so unfortunately this is v19
material at this point.

-- 
nathan

---

From: Hannu Krosing <[email-masked]>
Date: Tue, 8 Apr 2025 18:13:28 +0200

> Feature freeze for v18 was ~4 hours ago, so unfortunately this is v19

Sure. But I actually think that this is something that should be
back-ported to at least all supported versions at some pon.
Possibly made dependent on some environment flag so only people that
desperately need it will get it.

Btw, who would be the right person(s) to ask questions about internals
of pg_dump ?
I have a few more things in the pipeline to add there and would like
to make sure that I have the right approach.

------
Hannu

---

From: Hannu Krosing <[email-masked]>
Date: Tue, 8 Apr 2025 18:20:06 +0200

This is what the opening comment in pg_upgrade says

I think we do preserve role oids

/*
 * To simplify the upgrade process, we force certain system values to be
 * identical between old and new clusters:
 *
 * We control all assignments of pg_class.oid (and relfilenode) so toast
 * oids are the same between old and new clusters.  This is important
 * because toast oids are stored as toast pointers in user tables.
 *
 * While pg_class.oid and pg_class.relfilenode are initially the same in a
 * cluster, they can diverge due to CLUSTER, REINDEX, or VACUUM FULL. We
 * control assignments of pg_class.relfilenode because we want the filenames
 * to match between the old and new cluster.
 *
 * We control assignment of pg_tablespace.oid because we want the oid to match
 * between the old and new cluster.
 *
 * We control all assignments of pg_type.oid because these oids are stored
 * in user composite type values.
 *
 * We control all assignments of pg_enum.oid because these oids are stored
 * in user tables as enum values.
 *
 * We control all assignments of pg_authid.oid for historical reasons (the
 * oids used to be stored in pg_largeobject_metadata, which is now copied via
 * SQL commands), that might change at some point in the future.
 */

On Tue, Apr 8, 2025 at 6:13 PM Tom Lane <[email-masked]> wrote:
>
> Nathan Bossart <[email-masked]> writes:
> Yeah, even if we had a patch in hand, it's too late for v18.  However
> there are additional problems with this idea:
>
> 1. The idea requires role OIDs to match across the upgrade.
> I don't believe that pg_upgrade tries to preserve role OIDs --- and
> doing so would be problematic, because what if the new cluster's
> bootstrap superuser is named differently in the old and new clusters?
>
> It might be possible to work around that with some casting to/from
> regrole, but I don't think a simple COPY into pg_largeobject_metadata
> will play along with that.
>
> 2. If you just do the equivalent of an INSERT or COPY into
> pg_largeobject_metadata, you could create entries that look right,
> but they are actually not right because there should be pg_shdepend
> entries backing each ownership or permission reference (for non-pinned
> roles) and there won't be.
>
> I guess you could think of also manually inserting rows into
> pg_shdepend, but (a) ugh and (b) the claimed speedup is kind
> of vanishing into the distance at this point.
>
>                         regards, tom lane

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 8 Apr 2025 11:50:01 -0500

On Tue, Apr 08, 2025 at 12:37:43PM -0400, Tom Lane wrote:
> Hannu Krosing <[email-masked]> writes:
> Oh ... I'd been looking for mentions of "role" in
> pg_upgrade_support.c, but what I should have looked for was
> "pg_authid".  So yeah, we do preserve role OIDs, and maybe that's
> enough to make this workable, at least with source versions that
> share the same rules for what goes into pg_largeobject_metadata and
> pg_shdepend.  It's not something I'd risk back-patching though.

I do think it's worth considering going back to copying
pg_largobject_metadata's files for upgrades from v16 and newer.  That
sounds restrictive at the moment, but it'll mean that all but one supported
major version can copy the files during upgrade to v19.  I'll admit I'm a
tad worried about having to go back to copying via SQL commands in the
future and re-regressing things (leading to unpredictable differences in
upgrade downtime), but I'm not sure that's a great reason to withhold this
optimization.

Of course, I wouldn't be opposed to optimizing the SQL command strategy,
too, but I suspect that won't compare to copying the files.

-- 
nathan

---

From: Hannu Krosing <[email-masked]>
Date: Tue, 8 Apr 2025 18:52:45 +0200

> pg_shdepend.  It's not something I'd risk back-patching though.

The risk is why I suggest to have this in backports as something last
resort which is gated on an environment variable

I have been forced to do this half-manually a few times to make
pg_upgrade feasible at all.

Current really UGH workaround is to use pg_repack support for swapping
relfilenodes of pg_largeobject_metadata with a user table before and
after the upgrade and then manually patching up leftovers like
pg_shdepend after pg_upgrade.

These have fortunately been cases where there were no other
dependencies like comments or security labels on LOs, but these to
should be doable;

---

From: Hannu Krosing <[email-masked]>
Date: Tue, 8 Apr 2025 19:18:31 +0200

On Tue, Apr 8, 2025 at 7:07 PM Tom Lane <[email-masked]> wrote:
> > pg_largobject_metadata's files for upgrades from v16 and newer.
>
> (If we do this) I don't see why we'd need to stop at v16.  I'm
> envisioning that we'd use COPY, which will be dealing in the
> text representation of aclitems, and I don't think that's changed
> in a long time.  The sort of thing that would break it is changes
> in the set of available/default privilege bits for large objects.
>
> That is, where the dump currently contains something like
>
> SELECT pg_catalog.lo_create('2121');
> ALTER LARGE OBJECT 2121 OWNER TO postgres;
> GRANT ALL ON LARGE OBJECT 2121 TO joe;

Also note that in my --binary-upgrade tests the 100 min / 100M objects
ratio was in case with no grants. I would expect this to grow to at
least 120 to 150 minutes when grants are also involved.

In copy case I would expect the presence of grants to not make much difference.

> we'd have
>
> COPY pg_largeobject_metadata FROM STDIN;
> ...
> 2121    10      {postgres=rw/postgres,joe=rw/postgres}
> ...

--
Hannu

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 8 Apr 2025 12:22:00 -0500

> in the set of available/default privilege bits for large objects.

I was thinking of actually reverting commit 12a53c7 for upgrades from v16,
which AFAICT is the last release where any relevant storage formats changed
(aclitem changed in v16).  But if COPY gets us pretty close to that and is
less likely to be disrupted by future changes, it could be a better
long-term approach.

> and some appropriate COPY data for pg_shdepend too.

Unless I'm missing something, we don't seem to have had any dependency
handling before commit 12a53c7.  Was that broken before we moved to SQL
commands?

-- 
nathan

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 8 Apr 2025 12:41:26 -0500

On Tue, Apr 08, 2025 at 01:36:58PM -0400, Tom Lane wrote:
> Hmm ... one annoying thing for this project is that AFAICS pg_upgrade
> does *not* preserve database OIDs, which is problematic for using
> COPY to load pg_shdepend rows.

I think it does; see commit aa01051.

-- 
nathan

---

From: Tom Lane <[email-masked]>
Date: Tue, 08 Apr 2025 13:51:22 -0400

> I think it does; see commit aa01051.

Ah --- I thought I remembered something having been done about that,
but I failed to find it because I was looking in pg_upgrade not
pg_dump.  Too bad aa01051 didn't update the comment at the top of
pg_upgrade.c.

			regards, tom lane

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 8 Apr 2025 13:39:45 -0500

On Tue, Apr 08, 2025 at 01:42:20PM -0400, Tom Lane wrote:
> Sounds like it :-(

Huh.  Sure enough, it seems to be lost during an upgrade from 9.6 to 10.

v9.6:

	postgres=# select lo_from_bytea(1234, '1234');
	 lo_from_bytea
	---------------
	          1234
	(1 row)

	postgres=# create role bob;
	CREATE ROLE
	postgres=# grant select on large object 1234 to bob;
	GRANT
	postgres=# drop role bob;
	ERROR:  role "bob" cannot be dropped because some objects depend on it
	DETAIL:  privileges for large object 1234

v10 (upgraded from v9.6):

	postgres=# select lo_get(1234);
	   lo_get
	------------
	 \x31323334
	(1 row)

	postgres=# drop role bob;
	DROP ROLE

If I then try to upgrade that database to v17, it fails like this:

	pg_restore: from TOC entry 2422; 0 0 ACL LARGE OBJECT 1234 nathan
	pg_restore: error: could not execute query: ERROR:  role "16384" does not exist
	Command was: GRANT SELECT ON LARGE OBJECT 1234 TO "16384";

I've also verified that the dependency information is carried over in
upgrades to later versions (AFAICT all the supported ones).

-- 
nathan

---

From: Hannu Krosing <[email-masked]>
Date: Tue, 8 Apr 2025 21:41:06 +0200

On Tue, Apr 8, 2025 at 8:39 PM Nathan Bossart <[email-masked]> wrote:
...
> upgrades to later versions (AFAICT all the supported ones).

If I remember correctly the change to not copying
pg_largeobject_metadata data file but instead moving LOs as part of
schema was done in v12 when oid,, which had been a system column in
v11, became a user column, so upgrade to v11 is likely also missing
the dependencies

---

From: Nathan Bossart <[email-masked]>
Date: Wed, 9 Apr 2025 12:04:21 -0500

On Tue, Apr 08, 2025 at 05:37:50PM -0400, Jan Wieck wrote:
> I remember an incident where large amounts of LOs ran pg_upgrade into a
> transaction-ID wrap around because the restore part would create individual
> single statement transactions per LO to create, then change permissions and
> ownership and finally fill in the data. Could that be related here?

I believe commits 74cf7d4 and 959b38d have largely fixed that problem.

-- 
nathan

---

From: Nathan Bossart <[email-masked]>
Date: Wed, 9 Apr 2025 12:16:44 -0500

> pg_dump.  Too bad aa01051 didn't update the comment at the top of

I'll apply the attached patch to fix the comment shortly.

-- 
nathan

---

From: Nathan Bossart <[email-masked]>
Date: Wed, 9 Apr 2025 12:17:32 -0500

> v11, became a user column, so upgrade to v11 is likely also missing

Right.

-- 
nathan

---

From: Nathan Bossart <[email-masked]>
Date: Fri, 11 Apr 2025 15:11:53 -0500

>> and some appropriate COPY data for pg_shdepend too.

I did some more research here.  For many large objects without ACLs to
dump, I noticed that the vast majority of time is going to restoring the
ALTER OWNER commands.  For 1 million such large objects, restoring took ~73
seconds on my machine.  If I instead invented an lo_create_with_owner()
function and created 100 per SELECT command, the same restore takes ~7
seconds.  Copying the relevant pg_shdepend rows out and back in takes ~2.5
seconds.  I imagine using COPY for pg_largeobject_metadata would also take
a couple of seconds in this case.

For upgrading, I don't think there's any huge benefit to optimizing the
restore commands versus using COPY.  It might make future catalog changes
for large object stuff easier, but I'd expect those to be rare.  However,
the optimized restore commands could be nice for non-pg_upgrade use-cases.

-- 
nathan

---

From: Hannu Krosing <[email-masked]>
Date: Sun, 13 Apr 2025 22:17:59 +0200

And in case there *is* ACL present then each user mentioned in the ACL
adds more overhead

Also the separate GRANT calls cause bloat as the
pg_largeoject_metadata row gets updated for each ALTER USER or GRANT

The following is for 10 million LOs with 1 and 3 users being GRANTed
SELECT on each object (with no grants the pg_restore run was 10
minutes)

Nr of GRANTS  | pg_dump time | pg_restore time
--------------+--------------+----------------
       0      |    0m 10s    |   10m  5s
       1      |    0m 17s    |   15m  3s
       3      |    0m 21s    |   27m 15s

NB! - I left out the --verbose flag from pg_dump as used by
pg_upgrade, as it will emit one line per LO dumped

## 1 GRANT / LO

hannuk@db01-c1a:~/work/lo-testing$ time pg_dump --schema-only
--quote-all-identifiers --binary-upgrade --format=custom
--file=lodb10m.dump -p 5433 lodb10m
real    0m17.022s
user    0m2.956s
sys     0m1.453s

hannuk@db01-c1a:~/work/lo-testing$ time pg_restore -p 5434
--exit-on-error --transaction-size=1000 --dbname lodb10m lodb10m.dump
real    15m3.136s
user    0m28.991s
sys     2m54.164s

## 3 GRANTs / LO

make sample LO with 3 grants

ALTER LARGE OBJECT 1 OWNER TO "hannuk";
GRANT SELECT ON LARGE OBJECT 1 TO "bob";
GRANT SELECT ON LARGE OBJECT 1 TO "joe";
GRANT SELECT ON LARGE OBJECT 1 TO "tom";
lodb10m=# select *  from pg_shdepend where objid = 1;

┌───────┬─────────┬───────┬──────────┬────────────┬──────────┬─────────┐
│ dbid  │ classid │ objid │ objsubid │ refclassid │ refobjid │ deptype │
├───────┼─────────┼───────┼──────────┼────────────┼──────────┼─────────┤
│ 16406 │    2613 │     1 │        0 │       1260 │    16384 │ o       │
│ 16406 │    2613 │     1 │        0 │       1260 │    16393 │ a       │
│ 16406 │    2613 │     1 │        0 │       1260 │    16394 │ a       │
│ 16406 │    2613 │     1 │        0 │       1260 │    16395 │ a       │
└───────┴─────────┴───────┴──────────┴────────────┴──────────┴─────────┘

lodb10m=# select *  from pg_largeobject_metadata ;
┌─────┬──────────┬───────────────────────────────────────────────────────────┐
│ oid │ lomowner │                          lomacl                           │
├─────┼──────────┼───────────────────────────────────────────────────────────┤
│   1 │    16384 │ {hannuk=rw/hannuk,bob=r/hannuk,joe=r/hannuk,tom=r/hannuk} │
└─────┴──────────┴───────────────────────────────────────────────────────────┘

Make the remaining 10M-1 LOs

lodb10m=# insert into pg_largeobject_metadata(oid, lomowner, lomacl)
SELECT i, 16384,
'{hannuk=rw/hannuk,bob=r/hannuk,joe=r/hannuk,tom=r/hannuk}' FROM
generate_series(2, 10_000_000) g(i);
INSERT 0 9999999
Time: 18859.341 ms (00:18.859)

And add their sharedeps

lodb10m=# WITH refdeps (robj, rdeptype)
AS ( VALUES
  (16384, 'o'),
  (16393, 'a'),
  (16394, 'a'),
  (16395, 'a')
)
INSERT INTO pg_shdepend SELECT 16396, 2613, i, 0, 1260, robj, rdeptype
  FROM generate_series(2, 10_000_000) g(i)
     , refdeps
;
INSERT 0 39999996
Time: 116697.342 ms (01:56.697)

Time pg_upgrade's pg_dump and pg_reload

hannuk@db01-c1a:~/work/lo-testing$ time pg_dump --schema-only
--quote-all-identifiers --binary-upgrade --format=custom
--file=lodb10m-3grants.dump -p 5433 lodb10m
real    0m21.519s
user    0m2.951s
sys     0m1.723s

hannuk@db01-c1a:~/work/lo-testing$ time pg_restore -p 5434
--exit-on-error --transaction-size=1000 --dbname lodb10m
lodb10m-3grants.dump
real    27m15.372s
user    0m45.157s
sys     4m57.513s

---

From: Nathan Bossart <[email-masked]>
Date: Mon, 5 May 2025 14:23:25 -0500

> and some appropriate COPY data for pg_shdepend too.

Attached is a proof-of-concept grade patch for using COPY for
pg_largeobject_metadata and the relevant pg_shdepend entries.  On my
laptop, pg_upgrade with 10M LOs (each with a non-bootstrap-superuser owner
and another role with SELECT rights) goes from ~8.5 minutes to ~1 minute
with this patch.

I originally set out to invent a new lo_create_with_owner() function and
teach pg_dump to batch those together in large groups, but as I started the
required pg_dump surgery, I was quickly scared away by the complexity.
Next, I gave COPY a try.  The improvements from using COPY will likely be
limited to the pg_upgrade case, but that's the only case I regularly hear
complaints about for zillions of large objects, so maybe it's good enough
for now.

For the COPY approach, I modified pg_dump to dump the contents of
pg_largeobject_metadata.  This is easy enough, but I ran into problems with
the dependent comments and security labels.  It turns out that even before
v12, we run all the lo_create() commands just so that creating the comments
and security labels works (AFAICT).  So I suspect upgrading with many large
objects has always been slow.  The comment/security label dependency issue
can be fixed (at least well enough for the tests) by moving
PRIO_LARGE_OBJECT below PRIO_TABLE_DATA.  There might be an existing issue
here, because dbObjectTypePriorities has the following comment:

 * NOTE: object-type priorities must match the section assignments made in
 * pg_dump.c; that is, PRE_DATA objects must sort before DO_PRE_DATA_BOUNDARY,
 * POST_DATA objects must sort after DO_POST_DATA_BOUNDARY, and DATA objects
 * must sort between them.

But dumpLO() puts large objects in SECTION_DATA, and PRIO_LARGE_OBJECT is
before PRIO_PRE_DATA_BOUNDARY.  I admittedly haven't spent too much time
investigating this, though.  In any case, it might be a good idea to also
make sure we explicitly mark the large objects and their comments/seclabels
as dependent on the pg_largeobject_metadata data.

That leaves pg_shdepend.  For now, I've just instructed pg_upgrade to COPY
the relevant pg_shdepend rows as an independent step, but perhaps there's a
reasonably straightforward way to put that in pg_dump, too.

-- 
nathan

---

From: Nathan Bossart <[email-masked]>
Date: Wed, 7 May 2025 09:51:19 -0500

> reasonably straightforward way to put that in pg_dump, too.

It turns out there is.  TableDataInfo's filtercond field can be used to
easily add a WHERE clause to the data dumping command.  On my laptop,
upgrading with --jobs=8 with 10M large objects evenly distributed across 10
databases (each with a non-bootstrap-superuser owner and another role with
select rights) takes ~100 seconds without this patch and ~30 seconds with
it.

I've also added dependency tracking, version checks (this only works for
upgrades from >=v12 for now), a hack to ensure the columns for
pg_largeobject_metadata/pg_shdepend are collected, and comments.  I'm sure
there's something I've missed, but this patch has worked well in my tests
thus far.

Taking a step back, I'm a little disappointed in the gains here.  A 3-9x
speedup is nice, but I guess I was hoping to find another order of
magnitude somewhere.  To do any better, I think we'd need to copy the files
for pg_largeobject_metadata directly for upgrades from >= v16, but that
would have to fit somewhere between when pg_restore creates the database
and when it restores any large object comments/seclabels.  I'm not wild
about the amount of hackery required to get that working.

-- 
nathan

---

From: Hannu Krosing <[email-masked]>
Date: Sun, 6 Jul 2025 14:48:08 +0200

Hi Nathan,

Did a quick check of the patch and it seems to work ok.

What do you think of the idea of not dumping pg_shdepend here, but
instead adding the required entries after loading
pg_largeobject_metadata based on the contents of it ?

The query for this would be

WITH db AS (
    SELECT oid FROM pg_database WHERE datname = current_database()
)
INSERT INTO pg_shdepend
SELECT db.oid AS dbid,
       2613 AS classid,
       lm.oid AS objid,
       0 AS objsubid,
       1260 AS refclassid,
       COALESCE(acl.grantee, lm.lomowner) AS refobjid,
       CASE WHEN grantee IS NULL or grantee = lomowner
            THEN 'o'
            ELSE 'a'
       END as deptype
  FROM pg_largeobject_metadata as lm
  LEFT JOIN LATERAL (
    SELECT DISTINCT (aclexplode(lm.lomacl)).grantee
  ) AS acl ON true,
  db
  WHERE (lm.oid, COALESCE(acl.grantee, lm.lomowner)) NOT IN (SELECT
objid, refobjid FROM pg_shdepend WHERE dbid = db.oid)
;

(I had hoped to use ON CONFLICT DO NOTHING but this is not supported
for system tables.)

---|
Hannu

---

From: Nathan Bossart <[email-masked]>
Date: Tue, 8 Jul 2025 16:06:16 -0500

> Did a quick check of the patch and it seems to work ok.

Thanks for taking a look.

> pg_largeobject_metadata based on the contents of it ?

While not dumping it might save a little space during upgrade, the query
seems to be extremely slow.  So, I don't see any strong advantage.

-- 
nathan

---

From: Hannu Krosing <[email-masked]>
Date: Wed, 9 Jul 2025 16:52:16 +0200

> seems to be extremely slow.  So, I don't see any strong advantage.

Yeah, looks like the part that avoids duplicates made it slow.

If you run it without the last WHERE it is reasonably fast. And it
behaves the same as just inserting from the dump which also does not
have any checks against duplicates.

---

From: Nathan Bossart <[email-masked]>
Date: Wed, 9 Jul 2025 12:53:48 -0500

> have any checks against duplicates.

With that change, the query is much faster, but my testing indicates that
it's still measurably slower than dumping and restoring the relevant
contents of pg_shdepend.

-- 
nathan

---

From: Hannu Krosing <[email-masked]>
Date: Wed, 9 Jul 2025 23:13:38 +0200

Ah, I see.

I retried and now it runs for 21 to 29 seconds for 1 million large
objects with two grants each, 3M total rows inserted.

Yesterday it ran 8 seconds. Unfortunately I do not have the plan from
yesterday saved .

Anyway it looks like just dumping pg_shdepend it is better, and likely
also compresses very well for most users

--
Hannu

---

From: Nitin Motiani <[email-masked]>
Date: Thu, 10 Jul 2025 18:05:26 +0530

Hi,

I have a couple of comments/questions.

> There might be an existing issue

> before PRIO_PRE_DATA_BOUNDARY. I admittedly haven't spent too much time investigating
this, though.

I looked through the history of this to see how this happened and if it
could be an existing issue. Prior to a45c78e3284b, dumpLO used to put large
objects in SECTION_PRE_DATA. That commit changed dumpLO and also changed
addBoundaryDependencies to move DO_LARGE_OBJECT from pre-data to data
section. Seems like since then this has been inconsistent with
pg_dump_sort.c. I think the change in pg_dump_sort.c should be backported
to PG17 & 18 independent of the state of the larger patch.

But even with the inconsistency, it doesn't look like there is an existing
issue. As the dependencies were changed in addBoundaryDependencies, that
should take precedence over the order in pg_dump_sort.c. The
dbObjectTypePriorities are used by sortDumpableObjectsByTypeName. But right
after that sortDumpableObjects sorts the objects based on dependencies
therefore the change in boundary dependencies should ensure that this is
working as intended. Still I think dbObjectTypePriorities should be made
consistent with the rest.

Also regarding this change in the patch

-		 * pg_largeobject_metadata, after the dump is restored.
+		 * pg_largeobject_metadata, after the dump is restored.  In versions
+		 * before v12, this is done via proper large object commands.  In
+		 * newer versions, we dump the content of pg_largeobject_metadata and
+		 * any associated pg_shdepend rows, which is faster to restore.
 		 */

Should the comment provide further detail on why this is only being done
for v12 and above?

Thanks & Regards,
Nitin Motiani
Google

---

From: Nathan Bossart <[email-masked]>
Date: Thu, 10 Jul 2025 16:42:23 -0500

> Should the comment provide further detail on why this is only being done

Yes.  I've fixed this in v3.

-- 
nathan

---

From: Nitin Motiani <[email-masked]>
Date: Fri, 11 Jul 2025 19:49:47 +0530

> Yes.  I've fixed this in v3.
Thanks. Looks good to me. Also just would like to confirm that the
pg_dump_sort change will go in a different patch.

Regards,
Nitin Motiani
Google

---

From: Nathan Bossart <[email-masked]>
Date: Fri, 11 Jul 2025 09:51:40 -0500

On Fri, Jul 11, 2025 at 07:49:47PM +0530, Nitin Motiani wrote:
> Thanks. Looks good to me.

Thank you for reviewing.

> Also just would like to confirm that the pg_dump_sort change will go in a
> different patch.

That's already been committed: http://postgr.es/c/fb6c860.

-- 
nathan

---

From: Nitin Motiani <[email-masked]>
Date: Fri, 11 Jul 2025 21:17:00 +0530

> That's already been committed: http://postgr.es/c/fb6c860.

That's great. Thank you.

---

From: Nathan Bossart <[email-masked]>
Date: Mon, 14 Jul 2025 11:24:14 -0500

Here is what I have staged for commit, which (barring feedback or
objections) I am planning to do towards the end of the week.

-- 
nathan

---

From: Tom Lane <[email-masked]>
Date: Mon, 14 Jul 2025 13:28:03 -0400

> objections) I am planning to do towards the end of the week.

Is it intentional that this does

+#include "catalog/pg_largeobject_metadata.h"
+#include "catalog/pg_shdepend.h"

rather than including the corresponding *_d.h headers?
If so, why?  Our normal coding convention is that frontend
code should only include catalog *_d.h files, since the main
headers might contain frontend-unfriendly declarations.
If there is something we need to expose in these catalogs'
*_d.h headers, we should probably do that.

			regards, tom lane

---

From: Nathan Bossart <[email-masked]>
Date: Mon, 14 Jul 2025 12:51:20 -0500

> rather than including the corresponding *_d.h headers?

Nope, that was an oversight.

-- 
nathan

---

From: Nathan Bossart <[email-masked]>
Date: Fri, 18 Jul 2025 11:05:04 -0500

Committed.

I'm cautiously optimistic that we can find some better gains for upgrades
from v16 and newer.  That would involve dumping lo_create() commands for
all LOs with comments/seclabels, dumping the relevant pg_shdepend rows, and
then copying/linking the pg_largeobject_metadata files like we did prior to
v12.

-- 
nathan

---

From: Hannu Krosing <[email-masked]>
Date: Mon, 21 Jul 2025 02:03:45 +0200

Do you think the current patch could be backported to at least some
latest versions ?

---

From: Nathan Bossart <[email-masked]>
Date: Mon, 21 Jul 2025 09:38:57 -0500

> Do you think the current patch could be backported to at least some

I think that's pretty unlikely.  It'd be a pretty big departure from our
versioning policy.  In the past, we have back-patched "critical"
performance fixes, but that involved changes that had already been in
released versions for years.

-- 
nathan