From e1136ca7817e36f48346da8771ca1f0100158295 Mon Sep 17 00:00:00 2001 From: David Gilman Date: Sat, 23 May 2020 10:49:33 -0400 Subject: [PATCH 3/3] Add integration test for out-of-order TOC requests in pg_restore 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 src/bin/pg_dump/pg_backup.h src/bin/pg_dump/pg_backup.h index 8c0cedcd98..160d705eb5 100644 --- src/bin/pg_dump/pg_backup.h +++ 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 src/bin/pg_dump/pg_backup_archiver.c src/bin/pg_dump/pg_backup_archiver.c index 8f0b32ca17..0e5d4e0a60 100644 --- src/bin/pg_dump/pg_backup_archiver.c +++ 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 src/bin/pg_dump/pg_backup_archiver.h src/bin/pg_dump/pg_backup_archiver.h index f9e6b42752..acf38c20da 100644 --- src/bin/pg_dump/pg_backup_archiver.h +++ 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 src/bin/pg_dump/pg_backup_custom.c src/bin/pg_dump/pg_backup_custom.c index 91c60f7103..d471dfe267 100644 --- src/bin/pg_dump/pg_backup_custom.c +++ 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 src/bin/pg_dump/pg_dump.c src/bin/pg_dump/pg_dump.c index a4e949c636..6ed45d319f 100644 --- src/bin/pg_dump/pg_dump.c +++ 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 src/bin/pg_dump/t/002_pg_dump.pl src/bin/pg_dump/t/002_pg_dump.pl index e116235769..2423ec5b1d 100644 --- src/bin/pg_dump/t/002_pg_dump.pl +++ 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