On Thu, 5 Jun 2025, Frédéric Yhuel wrote:
On 6/4/25 16:12, Dimitrios Apostolou wrote:
In general I have noticed most operations are slower after a succesful
pg_restore until VACUUM is complete, which is unfortunate as the database
is huge and it takes days to run. Something I have on my list to try, is
whether a COPY FREEZE would alleviate all this trouble, since all tuples
are immediately visible then. Maybe a patch for a new pg_restore option
--freeze is a better solution. Are my assumptions right?
It seems that the idea has already been discussed:
https://www.postgresql.org/message-id/flat/CA%2BU5nM%2BXvkUu9ran%2B5cY%3DTWQquLTpvzte4KVMK%3DaDfbr-xfNXA%40mail.gmail.com#b61a7fee06e10e61afa68712bc0b3c5b
I've CCed Bruce Mojman, in the hope that he can tell us more about it.
Thanks for all the pointers, it shows that changes in postgres are harder
than they appear.
FWIW I implemented a pg_restore --freeze patch, see attached. It needs
another patch of mine from [1] that implements pg_restore --data-only
--clean, which for parallel restores encases each COPY in its own
transaction and prepends it with a TRUNCATE. All feedback is welcome.
[1]
https://www.postgresql.org/message-id/c61263f2-7472-5dd8-703d-01e683421f61%40gmx.net
It works really fast for the data, and I see that some, but not all items
from section=post-data, start parallel plans. For example I see CREATE
INDEX spawns parallel workers.
But unfortunately the item in question (ADD FOREIGN KEY) is not parallel
(probably because the discussion [2] you posted in your previous email
never concluded). I /think/ though it's reading all the data faster than
before, but still has to go through terabytes of data and this takes a
long time, for each of the foreign keys it adds.
[2]
https://www.postgresql.org/message-id/flat/0d21e3b4-dcde-290c-875e-6ed5013e8...@dalibo.com
Still I wonder why pg_restore can't issue many ADD FOREIGN
KEY for the same table in parallel.
Regards,
Dimitris
From aa5df1f9e5f45aa45a8d7ab3e1cd6e96fee82f17 Mon Sep 17 00:00:00 2001
From: Dimitrios Apostolou <ji...@qt.io>
Date: Fri, 6 Jun 2025 19:03:51 +0200
Subject: [PATCH v1] pg_restore --freeze
pg_restore now invokes
COPY FROM ... WITH (FREEZE)
Needs also options
--section=data --clean -j N
so that each pg_restore worker process wraps each COPY in a transaction
and precedes it with TRUNCATE. That way the data insertion is optimized
and the visibility table is written without needing VACUUMing.
---
src/bin/pg_dump/pg_backup.h | 1 +
src/bin/pg_dump/pg_backup_archiver.c | 43 +++++++++++++++++++++++++++-
src/bin/pg_dump/pg_dump.c | 9 +++++-
src/bin/pg_dump/pg_restore.c | 5 ++++
4 files changed, 56 insertions(+), 2 deletions(-)
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 9428b362c92..f43d2c196f7 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -112,16 +112,17 @@ typedef struct _restoreOptions
int column_inserts;
int if_exists;
int no_comments; /* Skip comments */
int no_policies; /* Skip row security policies */
int no_publications; /* Skip publication entries */
int no_security_labels; /* Skip security label entries */
int no_subscriptions; /* Skip subscription entries */
int strict_names;
+ int freeze; /* COPY FREEZE */
const char *filename;
int dumpSections;
int verbose;
int aclsSkip;
const char *lockWaitTimeout;
int include_everything;
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index d48c6d11e7c..8a620fa74ae 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -92,16 +92,19 @@ static void RestoreOutput(ArchiveHandle *AH, CompressFileHandle *savedOutput);
static int restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel);
static void restore_toc_entries_prefork(ArchiveHandle *AH,
TocEntry *pending_list);
static void restore_toc_entries_parallel(ArchiveHandle *AH,
ParallelState *pstate,
TocEntry *pending_list);
static void restore_toc_entries_postfork(ArchiveHandle *AH,
TocEntry *pending_list);
+static void do_copy(ArchiveHandle *AH,
+ const char *cp,
+ bool freeze);
static void pending_list_header_init(TocEntry *l);
static void pending_list_append(TocEntry *l, TocEntry *te);
static void pending_list_remove(TocEntry *te);
static int TocEntrySizeCompareQsort(const void *p1, const void *p2);
static int TocEntrySizeCompareBinaryheap(void *p1, void *p2, void *arg);
static void move_to_ready_heap(TocEntry *pending_list,
binaryheap *ready_heap,
RestorePass pass);
@@ -1023,17 +1026,17 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
fmtQualifiedId(te->namespace, te->tag));
}
/*
* If we have a copy statement, use it.
*/
if (te->copyStmt && strlen(te->copyStmt) > 0)
{
- ahprintf(AH, "%s", te->copyStmt);
+ do_copy(AH, te->copyStmt, ropt->freeze);
AH->outputKind = OUTPUT_COPYDATA;
}
else
AH->outputKind = OUTPUT_OTHERDATA;
AH->PrintTocDataPtr(AH, te);
/*
@@ -1086,16 +1089,54 @@ restore_toc_entry(ArchiveHandle *AH, TocEntry *te, bool is_parallel)
}
if (AH->public.n_errors > 0 && status == WORKER_OK)
status = WORKER_IGNORED_ERRORS;
return status;
}
+static void
+do_copy(ArchiveHandle *AH, const char *cp, bool freeze)
+{
+ const size_t cp_len = strlen(cp);
+ bool cp_is_well_formed = false;
+ const char *cp_end;
+
+ Assert(cp_len > 0); /* Have checked it in caller */
+
+ /*
+ * Check if the COPY statement is well written so that we can inject the
+ * FREEZE option.
+ */
+ if (freeze)
+ {
+ int i = cp_len - 1;
+
+ /* Cut off the trailing semicolon and whitespace. */
+ while (i > 0 &&
+ (cp[i] == ' ' || cp[i] == '\n' || cp[i] == ';'))
+ i--;
+ cp_end = &cp[i + 1];
+
+ if (cp_end - 10 > cp &&
+ strncmp(cp_end - 10, "FROM stdin", 10) == 0
+ )
+ cp_is_well_formed = true;
+ else
+ pg_log_warning("COPY statement from dump file is not in the right form; Will not inject the FREEZE option");
+ }
+
+ if (freeze && cp_is_well_formed)
+ ahprintf(AH, "%.*s WITH (FREEZE);\n",
+ (int) (cp_end - cp), cp);
+ else
+ ahprintf(AH, "%s", cp);
+}
+
/*
* Allocate a new RestoreOptions block.
* This is mainly so we can initialize it, but also for future expansion,
*/
RestoreOptions *
NewRestoreOptions(void)
{
RestoreOptions *opts;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 37432e66efd..fd57457c080 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -2811,17 +2811,24 @@ dumpTableData(Archive *fout, const TableDataInfo *tdinfo)
copyFrom);
tdDefn = pg_strdup(copyBuf->data);
}
else
copyFrom = fmtQualifiedDumpable(tbinfo);
if (dopt->dump_inserts == 0)
{
- /* Dump/restore using COPY */
+ /*
+ * Dump/restore using COPY.
+ *
+ * NOTE: do not use options in the COPY statement, as the restore
+ * process appends its own options. In fact pg_restore always expects
+ *
+ * COPY ... FROM stdin;
+ */
dumpFn = dumpTableData_copy;
/* must use 2 steps here 'cause fmtId is nonreentrant */
printfPQExpBuffer(copyBuf, "COPY %s ",
copyFrom);
appendPQExpBuffer(copyBuf, "%s FROM stdin;\n",
fmtCopyColumnList(tbinfo, clistBuf));
copyStmt = copyBuf->data;
}
diff --git a/src/bin/pg_dump/pg_restore.c b/src/bin/pg_dump/pg_restore.c
index d08d3979c41..c7cf5c1490e 100644
--- a/src/bin/pg_dump/pg_restore.c
+++ b/src/bin/pg_dump/pg_restore.c
@@ -109,16 +109,17 @@ main(int argc, char **argv)
static int no_security_labels = 0;
static int no_statistics = 0;
static int no_subscriptions = 0;
static int strict_names = 0;
static int statistics_only = 0;
static int with_data = 0;
static int with_schema = 0;
static int with_statistics = 0;
+ static int freeze = 0;
struct option cmdopts[] = {
{"clean", 0, NULL, 'c'},
{"create", 0, NULL, 'C'},
{"data-only", 0, NULL, 'a'},
{"globals-only", 0, NULL, 'g'},
{"dbname", 1, NULL, 'd'},
{"exit-on-error", 0, NULL, 'e'},
@@ -170,16 +171,17 @@ main(int argc, char **argv)
{"no-subscriptions", no_argument, &no_subscriptions, 1},
{"no-statistics", no_argument, &no_statistics, 1},
{"with-data", no_argument, &with_data, 1},
{"with-schema", no_argument, &with_schema, 1},
{"with-statistics", no_argument, &with_statistics, 1},
{"statistics-only", no_argument, &statistics_only, 1},
{"filter", required_argument, NULL, 4},
{"exclude-database", required_argument, NULL, 6},
+ {"freeze", no_argument, &freeze, 1},
{NULL, 0, NULL, 0}
};
pg_logging_init(argv[0]);
pg_logging_set_level(PG_LOG_WARNING);
set_pglocale_pgservice(argv[0], PG_TEXTDOMAIN("pg_dump"));
@@ -462,16 +464,17 @@ main(int argc, char **argv)
opts->noTableAm = outputNoTableAm;
opts->noTablespace = outputNoTablespaces;
opts->use_setsessauth = use_setsessauth;
opts->no_comments = no_comments;
opts->no_policies = no_policies;
opts->no_publications = no_publications;
opts->no_security_labels = no_security_labels;
opts->no_subscriptions = no_subscriptions;
+ opts->freeze = freeze;
if (if_exists && !opts->dropSchema)
pg_fatal("option --if-exists requires option -c/--clean");
opts->if_exists = if_exists;
opts->strict_names = strict_names;
if (opts->formatName)
{
@@ -710,16 +713,18 @@ usage(const char *progname)
" match at least one entity each\n"));
printf(_(" --transaction-size=N commit after every N objects\n"));
printf(_(" --use-set-session-authorization\n"
" use SET SESSION AUTHORIZATION commands instead of\n"
" ALTER OWNER commands to set ownership\n"));
printf(_(" --with-data dump the data\n"));
printf(_(" --with-schema dump the schema\n"));
printf(_(" --with-statistics dump the statistics\n"));
+ printf(_(" --freeze COPY FREEZE (read the manual for caveats)\n"));
+
printf(_("\nConnection options:\n"));
printf(_(" -h, --host=HOSTNAME database server host or socket directory\n"));
printf(_(" -p, --port=PORT database server port number\n"));
printf(_(" -U, --username=NAME connect as specified database user\n"));
printf(_(" -w, --no-password never prompt for password\n"));
printf(_(" -W, --password force password prompt (should happen automatically)\n"));
printf(_(" --role=ROLENAME do SET ROLE before restore\n"));
--
2.49.0
