agora inbox for [email protected]
help / color / mirror / Atom feedFrom: David Gilman <[email protected]>
Subject: [PATCH 2/4] Add integration test for out-of-order TOC requests in pg_restore
Date: Sat, 23 May 2020 10:49:33 -0400
Add undocumented --disable-seeking argument to pg_dump to emulate
writing to an unseekable output file.
---
src/bin/pg_dump/pg_backup.h | 4 +-
src/bin/pg_dump/pg_backup_archiver.c | 14 ++--
src/bin/pg_dump/pg_backup_archiver.h | 1 +
src/bin/pg_dump/pg_backup_custom.c | 7 +-
src/bin/pg_dump/pg_dump.c | 6 +-
src/bin/pg_dump/t/002_pg_dump.pl | 97 +++++++++++++++++++++++++++-
6 files changed, 117 insertions(+), 12 deletions(-)
diff --git a/src/bin/pg_dump/pg_backup.h b/src/bin/pg_dump/pg_backup.h
index 8c0cedcd98..160d705eb5 100644
--- a/src/bin/pg_dump/pg_backup.h
+++ b/src/bin/pg_dump/pg_backup.h
@@ -158,6 +158,7 @@ typedef struct _dumpOptions
int use_setsessauth;
int enable_row_security;
int load_via_partition_root;
+ int disable_seeking;
/* default, if no "inclusion" switches appear, is to dump everything */
bool include_everything;
@@ -270,7 +271,8 @@ extern Archive *OpenArchive(const char *FileSpec, const ArchiveFormat fmt);
/* Create a new archive */
extern Archive *CreateArchive(const char *FileSpec, const ArchiveFormat fmt,
const int compression, bool dosync, ArchiveMode mode,
- SetupWorkerPtrType setupDumpWorker);
+ SetupWorkerPtrType setupDumpWorker,
+ bool disableSeeking);
/* The --list option */
extern void PrintTOCSummary(Archive *AH);
diff --git a/src/bin/pg_dump/pg_backup_archiver.c b/src/bin/pg_dump/pg_backup_archiver.c
index 8f0b32ca17..0e5d4e0a60 100644
--- a/src/bin/pg_dump/pg_backup_archiver.c
+++ b/src/bin/pg_dump/pg_backup_archiver.c
@@ -69,7 +69,8 @@ typedef struct _parallelReadyList
static ArchiveHandle *_allocAH(const char *FileSpec, const ArchiveFormat fmt,
const int compression, bool dosync, ArchiveMode mode,
- SetupWorkerPtrType setupWorkerPtr);
+ SetupWorkerPtrType setupWorkerPtr,
+ bool disableSeeking);
static void _getObjectDescription(PQExpBuffer buf, TocEntry *te,
ArchiveHandle *AH);
static void _printTocEntry(ArchiveHandle *AH, TocEntry *te, bool isData);
@@ -233,11 +234,12 @@ setupRestoreWorker(Archive *AHX)
Archive *
CreateArchive(const char *FileSpec, const ArchiveFormat fmt,
const int compression, bool dosync, ArchiveMode mode,
- SetupWorkerPtrType setupDumpWorker)
+ SetupWorkerPtrType setupDumpWorker,
+ bool disableSeeking)
{
ArchiveHandle *AH = _allocAH(FileSpec, fmt, compression, dosync,
- mode, setupDumpWorker);
+ mode, setupDumpWorker, disableSeeking);
return (Archive *) AH;
}
@@ -247,7 +249,7 @@ CreateArchive(const char *FileSpec, const ArchiveFormat fmt,
Archive *
OpenArchive(const char *FileSpec, const ArchiveFormat fmt)
{
- ArchiveHandle *AH = _allocAH(FileSpec, fmt, 0, true, archModeRead, setupRestoreWorker);
+ ArchiveHandle *AH = _allocAH(FileSpec, fmt, 0, true, archModeRead, setupRestoreWorker, false);
return (Archive *) AH;
}
@@ -2274,7 +2276,8 @@ _discoverArchiveFormat(ArchiveHandle *AH)
static ArchiveHandle *
_allocAH(const char *FileSpec, const ArchiveFormat fmt,
const int compression, bool dosync, ArchiveMode mode,
- SetupWorkerPtrType setupWorkerPtr)
+ SetupWorkerPtrType setupWorkerPtr,
+ bool disableSeeking)
{
ArchiveHandle *AH;
@@ -2325,6 +2328,7 @@ _allocAH(const char *FileSpec, const ArchiveFormat fmt,
AH->mode = mode;
AH->compression = compression;
AH->dosync = dosync;
+ AH->disableSeeking = disableSeeking;
memset(&(AH->sqlparse), 0, sizeof(AH->sqlparse));
diff --git a/src/bin/pg_dump/pg_backup_archiver.h b/src/bin/pg_dump/pg_backup_archiver.h
index f9e6b42752..acf38c20da 100644
--- a/src/bin/pg_dump/pg_backup_archiver.h
+++ b/src/bin/pg_dump/pg_backup_archiver.h
@@ -340,6 +340,7 @@ struct _archiveHandle
bool dosync; /* data requested to be synced on sight */
ArchiveMode mode; /* File mode - r or w */
void *formatData; /* Header data specific to file format */
+ bool disableSeeking; /* Don't use fseeko() */
/* these vars track state to avoid sending redundant SET commands */
char *currUser; /* current username, or NULL if unknown */
diff --git a/src/bin/pg_dump/pg_backup_custom.c b/src/bin/pg_dump/pg_backup_custom.c
index 5aa7ab33db..90a026e0f4 100644
--- a/src/bin/pg_dump/pg_backup_custom.c
+++ b/src/bin/pg_dump/pg_backup_custom.c
@@ -145,6 +145,7 @@ InitArchiveFmt_Custom(ArchiveHandle *AH)
AH->lo_buf = (void *) pg_malloc(LOBBUFSIZE);
ctx->filePos = 0;
+ ctx->hasSeek = 0;
/*
* Now open the file
@@ -164,7 +165,8 @@ InitArchiveFmt_Custom(ArchiveHandle *AH)
fatal("could not open output file: %m");
}
- ctx->hasSeek = checkSeek(AH->FH);
+ if (!AH->disableSeeking)
+ ctx->hasSeek = checkSeek(AH->FH);
}
else
{
@@ -181,7 +183,8 @@ InitArchiveFmt_Custom(ArchiveHandle *AH)
fatal("could not open input file: %m");
}
- ctx->hasSeek = checkSeek(AH->FH);
+ if (!AH->disableSeeking)
+ ctx->hasSeek = checkSeek(AH->FH);
ReadHead(AH);
ReadToc(AH);
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index a4e949c636..6ed45d319f 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -319,6 +319,7 @@ main(int argc, char **argv)
int plainText = 0;
ArchiveFormat archiveFormat = archUnknown;
ArchiveMode archiveMode;
+ bool disableSeeking = false;
static DumpOptions dopt;
@@ -360,6 +361,7 @@ main(int argc, char **argv)
{"binary-upgrade", no_argument, &dopt.binary_upgrade, 1},
{"column-inserts", no_argument, &dopt.column_inserts, 1},
{"disable-dollar-quoting", no_argument, &dopt.disable_dollar_quoting, 1},
+ {"disable-seeking", no_argument, &dopt.disable_seeking, 1},
{"disable-triggers", no_argument, &dopt.disable_triggers, 1},
{"enable-row-security", no_argument, &dopt.enable_row_security, 1},
{"exclude-table-data", required_argument, NULL, 4},
@@ -673,6 +675,8 @@ main(int argc, char **argv)
if (archiveFormat == archNull)
plainText = 1;
+ disableSeeking = (bool) dopt.disable_seeking;
+
/* Custom and directory formats are compressed by default, others not */
if (compressLevel == -1)
{
@@ -715,7 +719,7 @@ main(int argc, char **argv)
/* Open the output file */
fout = CreateArchive(filename, archiveFormat, compressLevel, dosync,
- archiveMode, setupDumpWorker);
+ archiveMode, setupDumpWorker, disableSeeking);
/* Make dump options accessible right away */
SetArchiveOptions(fout, &dopt, NULL);
diff --git a/src/bin/pg_dump/t/002_pg_dump.pl b/src/bin/pg_dump/t/002_pg_dump.pl
index e116235769..2423ec5b1d 100644
--- a/src/bin/pg_dump/t/002_pg_dump.pl
+++ b/src/bin/pg_dump/t/002_pg_dump.pl
@@ -31,9 +31,20 @@ my $tempdir_short = TestLib::tempdir_short;
# generate a text file to run through the tests from the
# non-text file generated by pg_dump.
#
-# TODO: Have pg_restore actually restore to an independent
-# database and then pg_dump *that* database (or something along
-# those lines) to validate that part of the process.
+# secondary_dump_cmd is an optional key that enables a
+#
+# pg_dump (dump_cmd) ->
+# pg_restore (restore_cmd, real database) ->
+# pg_dump (secondary_dmp_cmd, SQL output (for matching))
+#
+# test process instead of the default
+#
+# pg_dump (dump_cmd) ->
+# pg_restore (restore_cmd, SQL output (for matching))
+#
+# test process. The secondary_dump database is created for
+# you and your restore_cmd and secondary_dmp_cmd must read
+# and write from it.
my %pgdump_runs = (
binary_upgrade => {
@@ -139,6 +150,23 @@ my %pgdump_runs = (
"$tempdir/defaults_custom_format.dump",
],
},
+ defaults_custom_format_no_seek_parallel_restore => {
+ test_key => 'defaults',
+ dump_cmd => [
+ 'pg_dump', '-Fc', '-Z6', '--no-sync', '--disable-seeking',
+ "--file=$tempdir/defaults_custom_format_no_seek_parallel_restore.dump", 'postgres',
+ ],
+ restore_cmd => [
+ 'pg_restore', '-Fc', '--jobs=2',
+ '--dbname=secondary_dump',
+ "$tempdir/defaults_custom_format_no_seek_parallel_restore.dump",
+ ],
+ secondary_dump_cmd => [
+ 'pg_dump', '-Fp', '--no-sync',
+ "--file=$tempdir/defaults_custom_format_no_seek_parallel_restore.sql",
+ 'secondary_dump',
+ ],
+ },
# Do not use --no-sync to give test coverage for data sync.
defaults_dir_format => {
@@ -153,6 +181,24 @@ my %pgdump_runs = (
"$tempdir/defaults_dir_format",
],
},
+ defaults_dir_format_parallel => {
+ test_key => 'defaults',
+ dump_cmd => [
+ 'pg_dump', '-Fd',
+ '--jobs=2', '--no-sync',
+ "--file=$tempdir/defaults_dir_format_parallel", 'postgres',
+ ],
+ restore_cmd => [
+ 'pg_restore', '-Fd',
+ '--jobs=2', '--dbname=secondary_dump',
+ "$tempdir/defaults_dir_format_parallel",
+ ],
+ secondary_dump_cmd => [
+ 'pg_dump', '-Fp', '--no-sync',
+ "--file=$tempdir/defaults_dir_format_parallel.sql",
+ 'secondary_dump',
+ ],
+ },
# Do not use --no-sync to give test coverage for data sync.
defaults_parallel => {
@@ -3298,6 +3344,27 @@ my %tests = (
%full_runs, %dump_test_schema_runs, section_pre_data => 1,
},
unlike => { exclude_dump_test_schema => 1 },
+ },
+
+ # This reliably produces the "possibly due to out-of-order restore request"
+ # when restoring with -j 2 when this was written but future changes to how
+ # pg_dump works could accidentally correctly order things as we're not really
+ # telling pg_dump how to do its on-disk layout.
+ 'custom dump out-of-order' => {
+ create_sql => '
+ CREATE TABLE ooo_parent0 (id integer primary key);
+ CREATE TABLE ooo_child0 (parent0 int references ooo_parent0 (id));
+ CREATE TABLE ooo_parent1 (id integer primary key);
+ CREATE TABLE ooo_child1 (parent0 int references ooo_parent1 (id));',
+ regexp => qr/^
+ \QCREATE TABLE public.ooo_child0 (\E\n
+ \s+\Qparent0 integer\E\n
+ \Q);\E\n/xm,
+ like => {
+ %full_runs, section_pre_data => 1,
+ defaults_custom_format_no_seek => 1,
+ defaults_custom_format_no_seek_parallel_restore => 1,
+ },
});
#########################################
@@ -3350,6 +3417,11 @@ foreach my $run (sort keys %pgdump_runs)
$num_tests++;
}
+ if ($pgdump_runs{$run}->{secondary_dump_cmd})
+ {
+ $num_tests++;
+ }
+
if ($pgdump_runs{$run}->{test_key})
{
$test_key = $pgdump_runs{$run}->{test_key};
@@ -3499,12 +3571,23 @@ foreach my $run (sort keys %pgdump_runs)
$node->command_ok(\@{ $pgdump_runs{$run}->{dump_cmd} },
"$run: pg_dump runs");
+ if ($pgdump_runs{$run}->{secondary_dump_cmd})
+ {
+ $node->safe_psql('postgres', 'create database secondary_dump;');
+ }
+
if ($pgdump_runs{$run}->{restore_cmd})
{
$node->command_ok(\@{ $pgdump_runs{$run}->{restore_cmd} },
"$run: pg_restore runs");
}
+ if ($pgdump_runs{$run}->{secondary_dump_cmd})
+ {
+ $node->command_ok(\@{ $pgdump_runs{$run}->{secondary_dump_cmd} },
+ "$run: secondary pg_dump runs");
+ }
+
if ($pgdump_runs{$run}->{test_key})
{
$test_key = $pgdump_runs{$run}->{test_key};
@@ -3561,6 +3644,14 @@ foreach my $run (sort keys %pgdump_runs)
}
}
}
+
+ if ($pgdump_runs{$run}->{secondary_dump_cmd})
+ {
+ $node->safe_psql('secondary_dump',
+ 'alter subscription sub1 set (slot_name = NONE);');
+ $node->safe_psql('secondary_dump', 'drop subscription sub1;');
+ $node->safe_psql('postgres', 'drop database secondary_dump;');
+ }
}
#########################################
--
2.26.2
--46ew22bxjqgbs45w
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
filename="0003-Remove-unused-seek-check-in-tar-dump-format.patch"
view thread (5+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected]
Subject: Re: [PATCH 2/4] Add integration test for out-of-order TOC requests in pg_restore
In-Reply-To: <no-message-id-187417@localhost>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox