Date: Sat, 11 Dec 2021 16:43:08 -0600
From: Justin Pryzby <pryzby@telsasoft.com>
To: Jan Wieck <jan@wi3ck.info>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Bruce Momjian <bruce@momjian.us>,
	Zhihong Yu <zyu@yugabyte.com>, Andrew Dunstan <andrew@dunslane.net>,
	Magnus Hagander <magnus@hagander.net>,
	Robins Tharakan <tharakan@gmail.com>,
	Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
	pgsql-hackers@postgresql.org
Subject: Re: pg_upgrade failing for 200+ million Large Objects
Message-ID: <20211211224308.GM17618@telsasoft.com>
References: <91b02dc1-f0d9-e50d-849c-18d9a66484fb@wi3ck.info>
 <985941.1616524546@sss.pgh.pa.us>
 <c0f5fde5-5cad-65ed-dd30-cd852301d074@wi3ck.info>
 <986904.1616525964@sss.pgh.pa.us>
 <6cccaa33-c263-b8a2-b064-985605d33d25@wi3ck.info>
 <988415.1616528159@sss.pgh.pa.us>
 <872315a8-99fc-da4e-463d-784cfb5a025d@wi3ck.info>
 <1010642.1616532950@sss.pgh.pa.us>
 <802b96e9-f5e1-015c-dfb9-8756974b11fc@wi3ck.info>
 <0263bf35-05d6-02a1-519b-b7895a918314@wi3ck.info>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <0263bf35-05d6-02a1-519b-b7895a918314@wi3ck.info>
User-Agent: Mutt/1.9.4 (2018-02-28)
Archived-At: 
 <https://www.postgresql.org/message-id/20211211224308.GM17618%40telsasoft.com>
Precedence: bulk

On Wed, Mar 24, 2021 at 12:05:27PM -0400, Jan Wieck wrote:
> On 3/24/21 12:04 PM, Jan Wieck wrote:
> > In any case I changed the options so that they behave the same way, the
> > existing -o and -O (for old/new postmaster options) work. I don't think
> > it would be wise to have option forwarding work differently between
> > options for postmaster and options for pg_dump/pg_restore.
> 
> Attaching the actual diff might help.

I think the original issue with XIDs was fixed by 74cf7d46a.

Are you still planning to progress the patches addressing huge memory use of
pg_restore?

Note this other, old thread on -general, which I believe has variations on the
same patches.
https://www.postgresql.org/message-id/flat/7bf19bf2-e6b7-01a7-1d96-f0607c728c49@wi3ck.info

There was discussion about using pg_restore --single.  Note that that was used
at some point in the past: see 12ee6ec71 and 861ad67bd.

The immediate problem is that --single conflicts with --create.
I cleaned up a patch I'd written to work around that.  It preserves DB settings
and passes pg_upgrade's test.  It's probably not portable as written, but if need be
could pass an empty file instead of /dev/null...

diff --git a/src/bin/pg_upgrade/pg_upgrade.c b/src/bin/pg_upgrade/pg_upgrade.c
index 3628bd74a7..9c504aff79 100644
--- a/src/bin/pg_upgrade/pg_upgrade.c
+++ b/src/bin/pg_upgrade/pg_upgrade.c
@@ -364,6 +364,16 @@ create_new_objects(void)
 		DbInfo	   *old_db = &old_cluster.dbarr.dbs[dbnum];
 		const char *create_opts;
 
+		PQExpBufferData connstr,
+				escaped_connstr;
+
+		initPQExpBuffer(&connstr);
+		initPQExpBuffer(&escaped_connstr);
+		appendPQExpBufferStr(&connstr, "dbname=");
+		appendConnStrVal(&connstr, old_db->db_name);
+		appendShellString(&escaped_connstr, connstr.data);
+		termPQExpBuffer(&connstr);
+
 		/* Skip template1 in this pass */
 		if (strcmp(old_db->db_name, "template1") == 0)
 			continue;
@@ -378,18 +388,31 @@ create_new_objects(void)
 		 * propagate its database-level properties.
 		 */
 		if (strcmp(old_db->db_name, "postgres") == 0)
-			create_opts = "--clean --create";
+			create_opts = "--clean";
 		else
-			create_opts = "--create";
+			create_opts = "";
 
+		/* Create the DB but exclude all objects */
 		parallel_exec_prog(log_file_name,
 						   NULL,
 						   "\"%s/pg_restore\" %s %s --exit-on-error --verbose "
+							"--create -L /dev/null "
 						   "--dbname template1 \"%s\"",
 						   new_cluster.bindir,
 						   cluster_conn_opts(&new_cluster),
 						   create_opts,
 						   sql_file_name);
+
+		parallel_exec_prog(log_file_name,
+						   NULL,
+						   "\"%s/pg_restore\" %s %s --exit-on-error --verbose --single "
+						   "--dbname=%s \"%s\"",
+						   new_cluster.bindir,
+						   cluster_conn_opts(&new_cluster),
+						   create_opts,
+							escaped_connstr.data,
+						   sql_file_name);
+
 	}
 
 	/* reap all children */