From 1df5cc3ffdee85db8f9815dc0839769192b57158 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 26 Jan 2024 11:10:00 -0500 Subject: [PATCH v11 1/6] Some small preliminaries for pg_dump changes. Centralize management of the lo_buf used to hold data while restoring blobs. The code previously had each format handler create lo_buf, which seems rather pointless given that the format handlers all make it the same way. Moreover, the format handlers never use lo_buf directly, making this setup a failure from a separation-of-concerns standpoint. Let's move the responsibility into pg_backup_archiver.c, which is the only module concerned with lo_buf. The main reason to do this now is that it allows a centralized fix for the soon-to-be-false assumption that we never restore blobs in parallel. Also, get rid of dead code in DropLOIfExists: it's been a long time since we had any need to be able to restore to a pre-9.0 server. --- src/bin/pg_dump/pg_backup_archiver.c | 9 +++++++++ src/bin/pg_dump/pg_backup_custom.c | 7 ------- src/bin/pg_dump/pg_backup_db.c | 27 +++++---------------------- src/bin/pg_dump/pg_backup_directory.c | 6 ------ src/bin/pg_dump/pg_backup_null.c | 4 ---- src/bin/pg_dump/pg_backup_tar.c | 4 ---- 6 files changed, 14 insertions(+), 43 deletions(-) diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c index d97ebaff5b..f5935b08bb 100644 --- a/src/bin/pg_dump/pg_backup_archiver.c +++ b/src/bin/pg_dump/pg_backup_archiver.c @@ -1343,6 +1343,12 @@ StartRestoreLO(ArchiveHandle *AH, Oid oid, bool drop) AH->loCount++; /* Initialize the LO Buffer */ + if (AH->lo_buf == NULL) + { + /* First time through (in this process) so allocate the buffer */ + AH->lo_buf_size = LOBBUFSIZE; + AH->lo_buf = (void *) pg_malloc(LOBBUFSIZE); + } AH->lo_buf_used = 0; pg_log_info("restoring large object with OID %u", oid); @@ -4749,6 +4755,9 @@ CloneArchive(ArchiveHandle *AH) /* clone has its own error count, too */ clone->public.n_errors = 0; + /* clones should not share lo_buf */ + clone->lo_buf = NULL; + /* * Connect our new clone object to the database, using the same connection * parameters used for the original connection. diff --git a/src/bin/pg_dump/pg_backup_custom.c b/src/bin/pg_dump/pg_backup_custom.c index b576b29924..7c6ac89dd4 100644 --- a/src/bin/pg_dump/pg_backup_custom.c +++ b/src/bin/pg_dump/pg_backup_custom.c @@ -140,10 +140,6 @@ InitArchiveFmt_Custom(ArchiveHandle *AH) ctx = (lclContext *) pg_malloc0(sizeof(lclContext)); AH->formatData = (void *) ctx; - /* Initialize LO buffering */ - AH->lo_buf_size = LOBBUFSIZE; - AH->lo_buf = (void *) pg_malloc(LOBBUFSIZE); - /* * Now open the file */ @@ -902,9 +898,6 @@ _Clone(ArchiveHandle *AH) * share knowledge about where the data blocks are across threads. * _PrintTocData has to be careful about the order of operations on that * state, though. - * - * Note: we do not make a local lo_buf because we expect at most one BLOBS - * entry per archive, so no parallelism is possible. */ } diff --git a/src/bin/pg_dump/pg_backup_db.c b/src/bin/pg_dump/pg_backup_db.c index f766b65059..b297ca049d 100644 --- a/src/bin/pg_dump/pg_backup_db.c +++ b/src/bin/pg_dump/pg_backup_db.c @@ -544,26 +544,9 @@ CommitTransaction(Archive *AHX) void DropLOIfExists(ArchiveHandle *AH, Oid oid) { - /* - * If we are not restoring to a direct database connection, we have to - * guess about how to detect whether the LO exists. Assume new-style. - */ - if (AH->connection == NULL || - PQserverVersion(AH->connection) >= 90000) - { - ahprintf(AH, - "SELECT pg_catalog.lo_unlink(oid) " - "FROM pg_catalog.pg_largeobject_metadata " - "WHERE oid = '%u';\n", - oid); - } - else - { - /* Restoring to pre-9.0 server, so do it the old way */ - ahprintf(AH, - "SELECT CASE WHEN EXISTS(" - "SELECT 1 FROM pg_catalog.pg_largeobject WHERE loid = '%u'" - ") THEN pg_catalog.lo_unlink('%u') END;\n", - oid, oid); - } + ahprintf(AH, + "SELECT pg_catalog.lo_unlink(oid) " + "FROM pg_catalog.pg_largeobject_metadata " + "WHERE oid = '%u';\n", + oid); } diff --git a/src/bin/pg_dump/pg_backup_directory.c b/src/bin/pg_dump/pg_backup_directory.c index dba57443e8..de3cfea02e 100644 --- a/src/bin/pg_dump/pg_backup_directory.c +++ b/src/bin/pg_dump/pg_backup_directory.c @@ -143,10 +143,6 @@ InitArchiveFmt_Directory(ArchiveHandle *AH) ctx->dataFH = NULL; ctx->LOsTocFH = NULL; - /* Initialize LO buffering */ - AH->lo_buf_size = LOBBUFSIZE; - AH->lo_buf = (void *) pg_malloc(LOBBUFSIZE); - /* * Now open the TOC file */ @@ -823,8 +819,6 @@ _Clone(ArchiveHandle *AH) ctx = (lclContext *) AH->formatData; /* - * Note: we do not make a local lo_buf because we expect at most one BLOBS - * entry per archive, so no parallelism is possible. Likewise, * TOC-entry-local state isn't an issue because any one TOC entry is * touched by just one worker child. */ diff --git a/src/bin/pg_dump/pg_backup_null.c b/src/bin/pg_dump/pg_backup_null.c index 08f096251b..776f057770 100644 --- a/src/bin/pg_dump/pg_backup_null.c +++ b/src/bin/pg_dump/pg_backup_null.c @@ -63,10 +63,6 @@ InitArchiveFmt_Null(ArchiveHandle *AH) AH->ClonePtr = NULL; AH->DeClonePtr = NULL; - /* Initialize LO buffering */ - AH->lo_buf_size = LOBBUFSIZE; - AH->lo_buf = (void *) pg_malloc(LOBBUFSIZE); - /* * Now prevent reading... */ diff --git a/src/bin/pg_dump/pg_backup_tar.c b/src/bin/pg_dump/pg_backup_tar.c index aad88ad559..4cb9707e63 100644 --- a/src/bin/pg_dump/pg_backup_tar.c +++ b/src/bin/pg_dump/pg_backup_tar.c @@ -156,10 +156,6 @@ InitArchiveFmt_Tar(ArchiveHandle *AH) ctx->filePos = 0; ctx->isSpecialScript = 0; - /* Initialize LO buffering */ - AH->lo_buf_size = LOBBUFSIZE; - AH->lo_buf = (void *) pg_malloc(LOBBUFSIZE); - /* * Now open the tar file, and load the TOC if we're in read mode. */ -- 2.39.3