public inbox for [email protected]help / color / mirror / Atom feed
[PATCH v46 7/7] Exclude pg_stat directory from base backup 2+ messages / 2 participants [nested] [flat]
* [PATCH v46 7/7] Exclude pg_stat directory from base backup @ 2020-09-29 14:19 Kyotaro Horiguchi <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Kyotaro Horiguchi @ 2020-09-29 14:19 UTC (permalink / raw) basebackup sends the content of pg_stat directory, which is doomed to be removed at startup from the backup. Now that pg_stat_statements saves a temporary file into the directory, let exclude pg_stat directory from a base backup. --- src/backend/replication/basebackup.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c index d2c3064678..25677c5c6e 100644 --- a/src/backend/replication/basebackup.c +++ b/src/backend/replication/basebackup.c @@ -149,6 +149,13 @@ struct exclude_list_item */ static const char *const excludeDirContents[] = { + /* + * Skip statistics files. PGSTAT_STAT_PERMANENT_DIRECTORY must be skipped + * because the files in the directory will be removed at startup from the + * backup. + */ + PGSTAT_STAT_PERMANENT_DIRECTORY, + /* * It is generally not useful to backup the contents of this directory * even if the intention is to restore to another primary. See backup.sgml -- 2.27.0 ----Next_Part(Thu_Jan_14_15_14_25_2021_903)---- ^ permalink raw reply [nested|flat] 2+ messages in thread
* Re: pg_combinebackup --clone doesn't work @ 2024-06-21 16:10 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 2+ messages in thread From: Tomas Vondra @ 2024-06-21 16:10 UTC (permalink / raw) To: Peter Eisentraut <[email protected]>; pgsql-hackers On 6/21/24 00:07, Tomas Vondra wrote: > Here's a fix adding the missing headers to pg_combinebackup, and fixing > some compile-time issues in the ifdef-ed block. > > I've done some basic manual testing today - I plan to test this a bit > more tomorrow, and I'll also look at integrating this into the existing > tests. > Here's a bit more complete / cleaned patch, adding the testing changes in separate parts. 0001 adds the missing headers / fixes the now-accessible code a bit 0002 adds the --copy option for consistency with pg_upgrade 0003 adds the PG_TEST_PG_COMBINEBACKUP_MODE, so that we can override the copy method for tests 0004 tweaks two of the Cirrus CI tasks to use --clone/--copy-file-range I believe 0001-0003 are likely non-controversial, although if someone could take a look at the Perl in 0003 that'd be nice. Also, 0002 seems nice not only because of consistency with pg_upgrade, but it also makes 0003 easier as we don't need to special-case the default mode etc. I'm not sure about 0004 - I initially did this mostly to check we have the right headers on other platforms, but not sure we want to actually do this. Or maybe we want to test a different combination (e.g. also test the --clone on Linux)? regards -- Tomas Vondra EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company Attachments: [text/x-patch] 0001-Add-headers-needed-by-pg_combinebackup-clone.patch (2.5K, ../../[email protected]/2-0001-Add-headers-needed-by-pg_combinebackup-clone.patch) download | inline diff: From 94082624db5d7608f3fb7a5b9322ee90dc5cfcb5 Mon Sep 17 00:00:00 2001 From: Tomas Vondra <[email protected]> Date: Thu, 20 Jun 2024 23:50:22 +0200 Subject: [PATCH 1/4] Add headers needed by pg_combinebackup --clone The code for cloning files existed, but was inaccessible as it relied on constants from missing headers. The consequence is that on Linux --clone always failed with error: file cloning not supported on this platform Fixed by including the missing headers to relevant places. This revealed a couple compile errors in copy_file_clone(), so fix those too. Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/48da4a1f-ccd9-4988-9622-24f37b1de2b4%40eisentraut.org --- src/bin/pg_combinebackup/copy_file.c | 12 +++++++++++- src/bin/pg_combinebackup/pg_combinebackup.c | 8 ++++++++ 2 files changed, 19 insertions(+), 1 deletion(-) diff --git a/src/bin/pg_combinebackup/copy_file.c b/src/bin/pg_combinebackup/copy_file.c index 08c3b875420..8b50e937346 100644 --- a/src/bin/pg_combinebackup/copy_file.c +++ b/src/bin/pg_combinebackup/copy_file.c @@ -13,6 +13,10 @@ #ifdef HAVE_COPYFILE_H #include <copyfile.h> #endif +#ifdef __linux__ +#include <sys/ioctl.h> +#include <linux/fs.h> +#endif #include <fcntl.h> #include <limits.h> #include <sys/stat.h> @@ -214,6 +218,9 @@ copy_file_clone(const char *src, const char *dest, pg_fatal("error while cloning file \"%s\" to \"%s\": %m", src, dest); #elif defined(__linux__) && defined(FICLONE) { + int src_fd; + int dest_fd; + if ((src_fd = open(src, O_RDONLY | PG_BINARY, 0)) < 0) pg_fatal("could not open file \"%s\": %m", src); @@ -228,8 +235,11 @@ copy_file_clone(const char *src, const char *dest, unlink(dest); pg_fatal("error while cloning file \"%s\" to \"%s\": %s", - src, dest); + src, dest, strerror(save_errno)); } + + close(src_fd); + close(dest_fd); } #else pg_fatal("file cloning not supported on this platform"); diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c index 01d7fb98e79..f67ccf76ea2 100644 --- a/src/bin/pg_combinebackup/pg_combinebackup.c +++ b/src/bin/pg_combinebackup/pg_combinebackup.c @@ -16,6 +16,14 @@ #include <fcntl.h> #include <limits.h> +#ifdef HAVE_COPYFILE_H +#include <copyfile.h> +#endif +#ifdef __linux__ +#include <sys/ioctl.h> +#include <linux/fs.h> +#endif + #include "backup_label.h" #include "common/blkreftable.h" #include "common/checksum_helper.h" -- 2.45.2 [text/x-patch] 0002-Introduce-pg_combinebackup-copy-option.patch (2.7K, ../../[email protected]/3-0002-Introduce-pg_combinebackup-copy-option.patch) download | inline diff: From 93ecbe611936279127cebc5ddfefb6ebf1f4730f Mon Sep 17 00:00:00 2001 From: Tomas Vondra <[email protected]> Date: Fri, 21 Jun 2024 14:36:04 +0200 Subject: [PATCH 2/4] Introduce pg_combinebackup --copy option The --copy option simply allows picking the default mode to copy data, and does not change the behavior. This makes pg_combinebackup options more consistent with pg_upgrade. Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/48da4a1f-ccd9-4988-9622-24f37b1de2b4%40eisentraut.org --- doc/src/sgml/ref/pg_combinebackup.sgml | 10 ++++++++++ src/bin/pg_combinebackup/pg_combinebackup.c | 7 ++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/doc/src/sgml/ref/pg_combinebackup.sgml b/doc/src/sgml/ref/pg_combinebackup.sgml index 375307d57bd..091982f62ad 100644 --- a/doc/src/sgml/ref/pg_combinebackup.sgml +++ b/doc/src/sgml/ref/pg_combinebackup.sgml @@ -162,6 +162,16 @@ PostgreSQL documentation </listitem> </varlistentry> + <varlistentry> + <term><option>--copy</option></term> + <listitem> + <para> + Perform regular file copy. This is the default. (See also + <option>--copy-file-range</option> and <option>--clone</option>.) + </para> + </listitem> + </varlistentry> + <varlistentry> <term><option>--copy-file-range</option></term> <listitem> diff --git a/src/bin/pg_combinebackup/pg_combinebackup.c b/src/bin/pg_combinebackup/pg_combinebackup.c index f67ccf76ea2..6ddf5b534b1 100644 --- a/src/bin/pg_combinebackup/pg_combinebackup.c +++ b/src/bin/pg_combinebackup/pg_combinebackup.c @@ -139,7 +139,8 @@ main(int argc, char *argv[]) {"no-manifest", no_argument, NULL, 2}, {"sync-method", required_argument, NULL, 3}, {"clone", no_argument, NULL, 4}, - {"copy-file-range", no_argument, NULL, 5}, + {"copy", no_argument, NULL, 5}, + {"copy-file-range", no_argument, NULL, 6}, {NULL, 0, NULL, 0} }; @@ -209,6 +210,9 @@ main(int argc, char *argv[]) opt.copy_method = COPY_METHOD_CLONE; break; case 5: + opt.copy_method = COPY_METHOD_COPY; + break; + case 6: opt.copy_method = COPY_METHOD_COPY_FILE_RANGE; break; default: @@ -763,6 +767,7 @@ help(const char *progname) printf(_(" -T, --tablespace-mapping=OLDDIR=NEWDIR\n" " relocate tablespace in OLDDIR to NEWDIR\n")); printf(_(" --clone clone (reflink) instead of copying files\n")); + printf(_(" --copy copy files (default)\n")); printf(_(" --copy-file-range copy using copy_file_range() syscall\n")); printf(_(" --manifest-checksums=SHA{224,256,384,512}|CRC32C|NONE\n" " use algorithm for manifest checksums\n")); -- 2.45.2 [text/x-patch] 0003-Add-PG_TEST_PG_COMBINEBACKUP_MODE.patch (9.4K, ../../[email protected]/4-0003-Add-PG_TEST_PG_COMBINEBACKUP_MODE.patch) download | inline diff: From 0b3a9afe25d8837c0dcf5a03b856c8db1f4c26cd Mon Sep 17 00:00:00 2001 From: Tomas Vondra <[email protected]> Date: Fri, 21 Jun 2024 14:16:16 +0200 Subject: [PATCH 3/4] Add PG_TEST_PG_COMBINEBACKUP_MODE This introduces an environment variable PG_TEST_PG_COMBINEBACKUP_MODE that determines copy mode used by pg_combinebackup in TAP tests. This defaults to "--copy" but may be set to "--clone" / "--copy-file-range" to use the alternative stategies. Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/48da4a1f-ccd9-4988-9622-24f37b1de2b4%40eisentraut.org --- .../pg_combinebackup/t/002_compare_backups.pl | 8 +++++- src/bin/pg_combinebackup/t/003_timeline.pl | 8 +++++- src/bin/pg_combinebackup/t/004_manifest.pl | 9 +++++-- src/bin/pg_combinebackup/t/005_integrity.pl | 25 +++++++++++-------- .../pg_combinebackup/t/006_db_file_copy.pl | 8 +++++- src/test/perl/PostgreSQL/Test/Cluster.pm | 5 ++++ 6 files changed, 48 insertions(+), 15 deletions(-) diff --git a/src/bin/pg_combinebackup/t/002_compare_backups.pl b/src/bin/pg_combinebackup/t/002_compare_backups.pl index f032959ef5c..63a0255de15 100644 --- a/src/bin/pg_combinebackup/t/002_compare_backups.pl +++ b/src/bin/pg_combinebackup/t/002_compare_backups.pl @@ -9,6 +9,11 @@ use Test::More; my $tempdir = PostgreSQL::Test::Utils::tempdir_short(); +# Can be changed to test the other modes. +my $mode = $ENV{PG_TEST_PG_COMBINEBACKUP_MODE} || '--copy'; + +note "testing using mode $mode"; + # Set up a new database instance. my $primary = PostgreSQL::Test::Cluster->new('primary'); $primary->init(has_archiving => 1, allows_streaming => 1); @@ -134,7 +139,8 @@ $pitr2->init_from_backup( standby => 1, has_restoring => 1, combine_with_prior => ['backup1'], - tablespace_map => { $tsbackup2path => $tspitr2path }); + tablespace_map => { $tsbackup2path => $tspitr2path }, + combine_mode => $mode); $pitr2->append_conf( 'postgresql.conf', qq{ recovery_target_lsn = '$lsn' diff --git a/src/bin/pg_combinebackup/t/003_timeline.pl b/src/bin/pg_combinebackup/t/003_timeline.pl index 52eb642a392..83ab674a244 100644 --- a/src/bin/pg_combinebackup/t/003_timeline.pl +++ b/src/bin/pg_combinebackup/t/003_timeline.pl @@ -10,6 +10,11 @@ use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; +# Can be changed to test the other modes. +my $mode = $ENV{PG_TEST_PG_COMBINEBACKUP_MODE} || '--copy'; + +note "testing using mode $mode"; + # Set up a new database instance. my $node1 = PostgreSQL::Test::Cluster->new('node1'); $node1->init(has_archiving => 1, allows_streaming => 1); @@ -68,7 +73,8 @@ $node2->command_ok( # Restore the incremental backup and use it to create a new node. my $node3 = PostgreSQL::Test::Cluster->new('node3'); $node3->init_from_backup($node1, 'backup3', - combine_with_prior => [ 'backup1', 'backup2' ]); + combine_with_prior => [ 'backup1', 'backup2' ], + combine_mode => $mode); $node3->start(); # Let's insert one more row. diff --git a/src/bin/pg_combinebackup/t/004_manifest.pl b/src/bin/pg_combinebackup/t/004_manifest.pl index 0df9fed73a8..6d475163ab9 100644 --- a/src/bin/pg_combinebackup/t/004_manifest.pl +++ b/src/bin/pg_combinebackup/t/004_manifest.pl @@ -12,6 +12,11 @@ use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; +# Can be changed to test the other modes. +my $mode = $ENV{PG_TEST_PG_COMBINEBACKUP_MODE} || '--copy'; + +note "testing using mode $mode"; + # Set up a new database instance. my $node = PostgreSQL::Test::Cluster->new('node'); $node->init(has_archiving => 1, allows_streaming => 1); @@ -53,9 +58,9 @@ sub combine_and_test_one_backup combine_and_test_one_backup('nomanifest', qr/could not open file.*backup_manifest/, '--no-manifest'); -combine_and_test_one_backup('csum_none', undef, '--manifest-checksums=NONE'); +combine_and_test_one_backup('csum_none', undef, '--manifest-checksums=NONE', $mode); combine_and_test_one_backup('csum_sha224', - undef, '--manifest-checksums=SHA224'); + undef, '--manifest-checksums=SHA224', $mode); # Verify that SHA224 is mentioned in the SHA224 manifest lots of times. my $sha224_manifest = diff --git a/src/bin/pg_combinebackup/t/005_integrity.pl b/src/bin/pg_combinebackup/t/005_integrity.pl index 636c3cc1b14..3caed13f6ed 100644 --- a/src/bin/pg_combinebackup/t/005_integrity.pl +++ b/src/bin/pg_combinebackup/t/005_integrity.pl @@ -13,6 +13,11 @@ use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; +# Can be changed to test the other modes. +my $mode = $ENV{PG_TEST_PG_COMBINEBACKUP_MODE} || '--copy'; + +note "testing using mode $mode"; + # Set up a new database instance. my $node1 = PostgreSQL::Test::Cluster->new('node1'); $node1->init(has_archiving => 1, allows_streaming => 1); @@ -79,13 +84,13 @@ my $resultpath = $node1->backup_dir . '/result'; # Can't combine 2 full backups. $node1->command_fails_like( - [ 'pg_combinebackup', $backup1path, $backup1path, '-o', $resultpath ], + [ 'pg_combinebackup', $backup1path, $backup1path, '-o', $resultpath, $mode ], qr/is a full backup, but only the first backup should be a full backup/, "can't combine full backups"); # Can't combine 2 incremental backups. $node1->command_fails_like( - [ 'pg_combinebackup', $backup2path, $backup2path, '-o', $resultpath ], + [ 'pg_combinebackup', $backup2path, $backup2path, '-o', $resultpath, $mode ], qr/is an incremental backup, but the first backup should be a full backup/, "can't combine full backups"); @@ -93,7 +98,7 @@ $node1->command_fails_like( $node1->command_fails_like( [ 'pg_combinebackup', $backup1path, $backupother2path, '-o', - $resultpath + $resultpath, $mode ], qr/expected system identifier.*but found/, "can't combine backups from different nodes"); @@ -106,7 +111,7 @@ copy("$backupother2path/backup_manifest", "$backup2path/backup_manifest") $node1->command_fails_like( [ 'pg_combinebackup', $backup1path, $backup2path, $backup3path, - '-o', $resultpath + '-o', $resultpath, $mode ], qr/ manifest system identifier is .*, but control file has /, "can't combine backups with different manifest system identifier "); @@ -116,7 +121,7 @@ move("$backup2path/backup_manifest.orig", "$backup2path/backup_manifest") # Can't omit a required backup. $node1->command_fails_like( - [ 'pg_combinebackup', $backup1path, $backup3path, '-o', $resultpath ], + [ 'pg_combinebackup', $backup1path, $backup3path, '-o', $resultpath, $mode ], qr/starts at LSN.*but expected/, "can't omit a required backup"); @@ -124,7 +129,7 @@ $node1->command_fails_like( $node1->command_fails_like( [ 'pg_combinebackup', $backup1path, $backup3path, $backup2path, - '-o', $resultpath + '-o', $resultpath, $mode ], qr/starts at LSN.*but expected/, "can't combine backups in the wrong order"); @@ -133,7 +138,7 @@ $node1->command_fails_like( $node1->command_ok( [ 'pg_combinebackup', $backup1path, $backup2path, $backup3path, - '-o', $resultpath + '-o', $resultpath, $mode ], "can combine 3 matching backups"); rmtree($resultpath); @@ -143,19 +148,19 @@ my $synthetic12path = $node1->backup_dir . '/synthetic12'; $node1->command_ok( [ 'pg_combinebackup', $backup1path, $backup2path, '-o', - $synthetic12path + $synthetic12path, $mode ], "can combine 2 matching backups"); # Can combine result of previous step with second incremental. $node1->command_ok( - [ 'pg_combinebackup', $synthetic12path, $backup3path, '-o', $resultpath ], + [ 'pg_combinebackup', $synthetic12path, $backup3path, '-o', $resultpath, $mode ], "can combine synthetic backup with later incremental"); rmtree($resultpath); # Can't combine result of 1+2 with 2. $node1->command_fails_like( - [ 'pg_combinebackup', $synthetic12path, $backup2path, '-o', $resultpath ], + [ 'pg_combinebackup', $synthetic12path, $backup2path, '-o', $resultpath, $mode ], qr/starts at LSN.*but expected/, "can't combine synthetic backup with included incremental"); diff --git a/src/bin/pg_combinebackup/t/006_db_file_copy.pl b/src/bin/pg_combinebackup/t/006_db_file_copy.pl index d57b550af21..f44788e82bf 100644 --- a/src/bin/pg_combinebackup/t/006_db_file_copy.pl +++ b/src/bin/pg_combinebackup/t/006_db_file_copy.pl @@ -7,6 +7,11 @@ use PostgreSQL::Test::Cluster; use PostgreSQL::Test::Utils; use Test::More; +# Can be changed to test the other modes. +my $mode = $ENV{PG_TEST_PG_COMBINEBACKUP_MODE} || '--copy'; + +note "testing using mode $mode"; + # Set up a new database instance. my $primary = PostgreSQL::Test::Cluster->new('primary'); $primary->init(has_archiving => 1, allows_streaming => 1); @@ -45,7 +50,8 @@ $primary->command_ok( # Recover the incremental backup. my $restore = PostgreSQL::Test::Cluster->new('restore'); $restore->init_from_backup($primary, 'backup2', - combine_with_prior => ['backup1']); + combine_with_prior => ['backup1'], + combine_mode => $mode); $restore->start(); # Query the DB. diff --git a/src/test/perl/PostgreSQL/Test/Cluster.pm b/src/test/perl/PostgreSQL/Test/Cluster.pm index 83f385a4870..0135c5a795c 100644 --- a/src/test/perl/PostgreSQL/Test/Cluster.pm +++ b/src/test/perl/PostgreSQL/Test/Cluster.pm @@ -856,6 +856,11 @@ sub init_from_backup push @combineargs, "-T$olddir=$newdir"; } } + # use the combine mode (clone/copy-file-range) if specified + if (defined $params{combine_mode}) + { + push @combineargs, $params{combine_mode}; + } push @combineargs, @prior_backup_path, $backup_path, '-o', $data_path; PostgreSQL::Test::Utils::system_or_bail(@combineargs); } -- 2.45.2 [text/x-patch] 0004-Add-PG_TEST_PG_COMBINEBACKUP_MODE-to-CI-tasks.patch (1.2K, ../../[email protected]/5-0004-Add-PG_TEST_PG_COMBINEBACKUP_MODE-to-CI-tasks.patch) download | inline diff: From 836e20a0ed34b7a9c367674e771fa8cb023e4341 Mon Sep 17 00:00:00 2001 From: Tomas Vondra <[email protected]> Date: Fri, 21 Jun 2024 17:55:38 +0200 Subject: [PATCH 4/4] Add PG_TEST_PG_COMBINEBACKUP_MODE to CI tasks This changes pg_combinebackup mode for two of the Cirrus CI tasks, to exercise the alternative copy methods in pg_combinebackup tests. We test --clone on macOS and --copy-file-range on one of the Linux tasks. Reported-by: Peter Eisentraut Discussion: https://postgr.es/m/48da4a1f-ccd9-4988-9622-24f37b1de2b4%40eisentraut.org --- .cirrus.tasks.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.cirrus.tasks.yml b/.cirrus.tasks.yml index 33646faeadf..4e029103c7e 100644 --- a/.cirrus.tasks.yml +++ b/.cirrus.tasks.yml @@ -318,6 +318,7 @@ task: env: SANITIZER_FLAGS: -fsanitize=address + PG_TEST_PG_COMBINEBACKUP_MODE: --copy-file-range # Normally, the "relation segment" code basically has no coverage in our # tests, because we (quite reasonably) don't generate tables large @@ -432,6 +433,7 @@ task: CXXFLAGS: -Og -ggdb PG_TEST_PG_UPGRADE_MODE: --clone + PG_TEST_PG_COMBINEBACKUP_MODE: --clone <<: *macos_task_template -- 2.45.2 ^ permalink raw reply [nested|flat] 2+ messages in thread
end of thread, other threads:[~2024-06-21 16:10 UTC | newest] Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-09-29 14:19 [PATCH v46 7/7] Exclude pg_stat directory from base backup Kyotaro Horiguchi <[email protected]> 2024-06-21 16:10 Re: pg_combinebackup --clone doesn't work Tomas Vondra <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox