Subject: Re: pg_upgrade failing for 200+ million Large Objects

From: "Kumar, Sachin" <[email-masked]>
Date: Thu, 9 Nov 2023 17:35:01 +0000

Hi Everyone , I want to continue this thread , I have rebased the patch to latest
master and fixed an issue when pg_restore prints to file.

`
╰─$ pg_restore  dump_small.custom  --restore-blob-batch-size=2 --file=a
--
-- End BLOB restore batch
--
COMMIT;
`

> On 09/11/2023, 17:05, "Jacob Champion" <[email-masked] <mailto:[email-masked]>> wrote:
> To clarify, I agree that pg_dump should contain the core fix. What I'm
> questioning is the addition of --dump-options to make use of that fix
> from pg_upgrade, since it also lets the user do "exciting" new things
> like --exclude-schema and --include-foreign-data and so on. I don't
> think we should let them do that without a good reason.

Earlier idea was to not expose these options to users and use [1]
   --restore-jobs=NUM             --jobs parameter passed to pg_restore
   --restore-blob-batch-size=NUM  number of blobs restored in one xact
But this was later expanded to use --dump-options and --restore-options [2].
With --restore-options user can use --exclude-schema , 
So maybe we can go back to [1]

[1] https://www.postgresql.org/message-id/a1e200e6-adde-2561-422b-a166ec084e3b%40wi3ck.info
[2] https://www.postgresql.org/message-id/8d8d3961-8e8b-3dbe-f911-6f418c5fb1d3%40wi3ck.info

Regards
Sachin
Amazon Web Services: https://aws.amazon.com

---

From: Tom Lane <[email-masked]>
Date: Thu, 09 Nov 2023 13:41:01 -0500

[ Jacob's email address updated ]

> master and fixed an issue when pg_restore prints to file.

Um ... you didn't attach the patch?

FWIW, I agree with Jacob's concern about it being a bad idea to let
users of pg_upgrade pass down arbitrary options to pg_dump/pg_restore.
I think we'd regret going there, because it'd hugely expand the set
of cases pg_upgrade has to deal with.

Also, pg_upgrade is often invoked indirectly via scripts, so I do
not especially buy the idea that we're going to get useful control
input from some human somewhere.  I think we'd be better off to
assume that pg_upgrade is on its own to manage the process, so that
if we need to switch strategies based on object count or whatever,
we should put in a heuristic to choose the strategy automatically.
It might not be perfect, but that will give better results for the
pretty large fraction of users who are not going to mess with
weird little switches.

			regards, tom lane

---

From: Andres Freund <[email-masked]>
Date: Thu, 09 Nov 2023 15:12:50 -0800

Hi, 

>pretty large fraction of users who are not going to mess with

+1 - even leaving everything else aside, just about no user would know about the option. There are cases where we can't do better than giving the user control, but we are certainly adding options at a rate that doesn't seem sustainable. And here it doesn't seem that hard to do better. 

Andres 
-- 
Sent from my Android device with K-9 Mail. Please excuse my brevity.

---

From: "Kumar, Sachin" <[email-masked]>
Date: Mon, 4 Dec 2023 16:07:59 +0000

> "Tom Lane" <[email-masked] <mailto:[email-masked]>> wrote:

> pretty large fraction of users who are not going to mess with

I have updated the patch to use heuristic, During pg_upgrade we count
Large objects per database. During pg_restore execution if db large_objects
count is greater than LARGE_OBJECTS_THRESOLD (1k) we will use 
--restore-blob-batch-size.
I also modified pg_upgrade --jobs behavior if we have large_objects (> LARGE_OBJECTS_THRESOLD)

+  /*  Restore all the dbs where LARGE_OBJECTS_THRESOLD is not breached */
+  restore_dbs(stats, true);
+  /* reap all children */
+  while (reap_child(true) == true)
+     ;
+  /*  Restore rest of the dbs one by one  with pg_restore --jobs = user_opts.jobs */
+  restore_dbs(stats, false);
   /* reap all children */
   while (reap_child(true) == true)
      ;

Regards
Sachin

---

From: "Kumar, Sachin" <[email-masked]>
Date: Thu, 7 Dec 2023 14:05:13 +0000

> --restore-blob-batch-size.

I think both SECTION_DATA and SECTION_POST_DATA can be parallelized by pg_restore, So instead of storing 
large objects in heuristics, we can store SECTION_DATA + SECTION_POST_DATA.

Regards
Sachin