pg_combinebackup: Fix incorrect tablespace handling.

The previous coding mangled the pathname calculation for
incremental files located in user-defined tablespaces.

Enhance the test cases to cover such cases, as I should have
done originally. Thanks to Andres Freund for alerting me to the
lack of test coverage.

Discussion: http://postgr.es/m/CA+TgmoYdXTjo9iQeoipTccDpWZzvBNS6EndY2uARM+T4yG_yDg@mail.gmail.com
This commit is contained in:
Robert Haas 2024-04-19 13:30:42 -04:00
parent 6bf5c42b55
commit cd64dc42d1
3 changed files with 33 additions and 8 deletions

View File

@ -990,7 +990,7 @@ process_directory_recursively(Oid tsoid,
/* Reconstruction logic will do the rest. */ /* Reconstruction logic will do the rest. */
reconstruct_from_incremental_file(ifullpath, ofullpath, reconstruct_from_incremental_file(ifullpath, ofullpath,
relative_path, manifest_prefix,
de->d_name + INCREMENTAL_PREFIX_LENGTH, de->d_name + INCREMENTAL_PREFIX_LENGTH,
n_prior_backups, n_prior_backups,
prior_backup_dirs, prior_backup_dirs,

View File

@ -75,9 +75,10 @@ static void read_block(rfile *s, off_t off, uint8 *buffer);
* output_filename should be the path where the reconstructed file is to be * output_filename should be the path where the reconstructed file is to be
* written. * written.
* *
* relative_path should be the relative path to the directory containing this * relative_path should be the path to the directory containing this file,
* file. bare_file_name should be the name of the file within that directory, * relative to the root of the backup (NOT relative to the root of the
* without "INCREMENTAL.". * tablespace). bare_file_name should be the name of the file within that
* directory, without "INCREMENTAL.".
* *
* n_prior_backups is the number of prior backups, and prior_backup_dirs is * n_prior_backups is the number of prior backups, and prior_backup_dirs is
* an array of pathnames where those backups can be found. * an array of pathnames where those backups can be found.

View File

@ -7,11 +7,15 @@ use PostgreSQL::Test::Cluster;
use PostgreSQL::Test::Utils; use PostgreSQL::Test::Utils;
use Test::More; use Test::More;
my $tempdir = PostgreSQL::Test::Utils::tempdir;
# Set up a new database instance. # Set up a new database instance.
my $primary = PostgreSQL::Test::Cluster->new('primary'); my $primary = PostgreSQL::Test::Cluster->new('primary');
$primary->init(has_archiving => 1, allows_streaming => 1); $primary->init(has_archiving => 1, allows_streaming => 1);
$primary->append_conf('postgresql.conf', 'summarize_wal = on'); $primary->append_conf('postgresql.conf', 'summarize_wal = on');
$primary->start; $primary->start;
my $tsprimary = $tempdir . '/ts';
mkdir($tsprimary) || die "mkdir $tsprimary: $!";
# Create some test tables, each containing one row of data, plus a whole # Create some test tables, each containing one row of data, plus a whole
# extra database. # extra database.
@ -29,24 +33,37 @@ INSERT INTO will_get_dropped VALUES (1, 'initial test row');
CREATE TABLE will_get_rewritten (a int, b text); CREATE TABLE will_get_rewritten (a int, b text);
INSERT INTO will_get_rewritten VALUES (1, 'initial test row'); INSERT INTO will_get_rewritten VALUES (1, 'initial test row');
CREATE DATABASE db_will_get_dropped; CREATE DATABASE db_will_get_dropped;
CREATE TABLESPACE ts1 LOCATION '$tsprimary';
CREATE TABLE will_not_change_in_ts (a int, b text) TABLESPACE ts1;
INSERT INTO will_not_change_in_ts VALUES (1, 'initial test row');
CREATE TABLE will_change_in_ts (a int, b text) TABLESPACE ts1;
INSERT INTO will_change_in_ts VALUES (1, 'initial test row');
CREATE TABLE will_get_dropped_in_ts (a int, b text);
INSERT INTO will_get_dropped_in_ts VALUES (1, 'initial test row');
EOM EOM
# Take a full backup. # Take a full backup.
my $backup1path = $primary->backup_dir . '/backup1'; my $backup1path = $primary->backup_dir . '/backup1';
my $tsbackup1path = $tempdir . '/ts1backup';
mkdir($tsbackup1path) || die "mkdir $tsbackup1path: $!";
$primary->command_ok( $primary->command_ok(
[ 'pg_basebackup', '-D', $backup1path, '--no-sync', '-cfast' ], [ 'pg_basebackup', '-D', $backup1path, '--no-sync', '-cfast',
"full backup"); "-T${tsprimary}=${tsbackup1path}" ], "full backup");
# Now make some database changes. # Now make some database changes.
$primary->safe_psql('postgres', <<EOM); $primary->safe_psql('postgres', <<EOM);
UPDATE will_change SET b = 'modified value' WHERE a = 1; UPDATE will_change SET b = 'modified value' WHERE a = 1;
UPDATE will_change_in_ts SET b = 'modified value' WHERE a = 1;
INSERT INTO will_grow INSERT INTO will_grow
SELECT g, 'additional row' FROM generate_series(2, 5000) g; SELECT g, 'additional row' FROM generate_series(2, 5000) g;
TRUNCATE will_shrink; TRUNCATE will_shrink;
VACUUM will_get_vacuumed; VACUUM will_get_vacuumed;
DROP TABLE will_get_dropped; DROP TABLE will_get_dropped;
DROP TABLE will_get_dropped_in_ts;
CREATE TABLE newly_created (a int, b text); CREATE TABLE newly_created (a int, b text);
INSERT INTO newly_created VALUES (1, 'row for new table'); INSERT INTO newly_created VALUES (1, 'row for new table');
CREATE TABLE newly_created_in_ts (a int, b text) TABLESPACE ts1;
INSERT INTO newly_created_in_ts VALUES (1, 'row for new table');
VACUUM FULL will_get_rewritten; VACUUM FULL will_get_rewritten;
DROP DATABASE db_will_get_dropped; DROP DATABASE db_will_get_dropped;
CREATE DATABASE db_newly_created; CREATE DATABASE db_newly_created;
@ -54,8 +71,11 @@ EOM
# Take an incremental backup. # Take an incremental backup.
my $backup2path = $primary->backup_dir . '/backup2'; my $backup2path = $primary->backup_dir . '/backup2';
my $tsbackup2path = $tempdir . '/tsbackup2';
mkdir($tsbackup2path) || die "mkdir $tsbackup2path: $!";
$primary->command_ok( $primary->command_ok(
[ 'pg_basebackup', '-D', $backup2path, '--no-sync', '-cfast', [ 'pg_basebackup', '-D', $backup2path, '--no-sync', '-cfast',
"-T${tsprimary}=${tsbackup2path}",
'--incremental', $backup1path . '/backup_manifest' ], '--incremental', $backup1path . '/backup_manifest' ],
"incremental backup"); "incremental backup");
@ -78,9 +98,11 @@ $primary->poll_query_until('postgres', $archive_wait_query)
# Perform PITR from the full backup. Disable archive_mode so that the archive # Perform PITR from the full backup. Disable archive_mode so that the archive
# doesn't find out about the new timeline; that way, the later PITR below will # doesn't find out about the new timeline; that way, the later PITR below will
# choose the same timeline. # choose the same timeline.
my $tspitr1path = $tempdir . '/tspitr1';
my $pitr1 = PostgreSQL::Test::Cluster->new('pitr1'); my $pitr1 = PostgreSQL::Test::Cluster->new('pitr1');
$pitr1->init_from_backup($primary, 'backup1', $pitr1->init_from_backup($primary, 'backup1',
standby => 1, has_restoring => 1); standby => 1, has_restoring => 1,
tablespace_map => { $tsbackup1path => $tspitr1path });
$pitr1->append_conf('postgresql.conf', qq{ $pitr1->append_conf('postgresql.conf', qq{
recovery_target_lsn = '$lsn' recovery_target_lsn = '$lsn'
recovery_target_action = 'promote' recovery_target_action = 'promote'
@ -90,10 +112,12 @@ $pitr1->start();
# Perform PITR to the same LSN from the incremental backup. Use the same # Perform PITR to the same LSN from the incremental backup. Use the same
# basic configuration as before. # basic configuration as before.
my $tspitr2path = $tempdir . '/tspitr2';
my $pitr2 = PostgreSQL::Test::Cluster->new('pitr2'); my $pitr2 = PostgreSQL::Test::Cluster->new('pitr2');
$pitr2->init_from_backup($primary, 'backup2', $pitr2->init_from_backup($primary, 'backup2',
standby => 1, has_restoring => 1, standby => 1, has_restoring => 1,
combine_with_prior => [ 'backup1' ]); combine_with_prior => [ 'backup1' ],
tablespace_map => { $tsbackup2path => $tspitr2path });
$pitr2->append_conf('postgresql.conf', qq{ $pitr2->append_conf('postgresql.conf', qq{
recovery_target_lsn = '$lsn' recovery_target_lsn = '$lsn'
recovery_target_action = 'promote' recovery_target_action = 'promote'