Add prefix checks in exclude lists for pg_rewind, pg_checksums and base backups

An instance of PostgreSQL crashing with a bad timing could leave behind
temporary pg_internal.init files, potentially causing failures when
verifying checksums.  As the same exclusion lists are used between
pg_rewind, pg_checksums and basebackup.c, all those tools are extended
with prefix checks to keep everything in sync, with dedicated checks
added for pg_internal.init.

Backpatch down to 11, where pg_checksums (pg_verify_checksums in 11) and
checksum verification for base backups have been introduced.

Reported-by: Michael Banck
Author: Michael Paquier
Reviewed-by: Kyotaro Horiguchi, David Steele
Discussion: https://postgr.es/m/62031974fd8e941dd8351fbc8c7eff60d59c5338.camel@credativ.de
Backpatch-through: 11
This commit is contained in:
Michael Paquier 2020-02-24 18:14:22 +09:00
parent 2eadd00dd4
commit da2a7180aa
4 changed files with 113 additions and 52 deletions

View File

@ -123,6 +123,18 @@ static int64 total_checksum_failures;
/* Do not verify checksums. */ /* Do not verify checksums. */
static bool noverify_checksums = false; static bool noverify_checksums = false;
/*
* Definition of one element part of an exclusion list, used for paths part
* of checksum validation or base backups. "name" is the name of the file
* or path to check for exclusion. If "match_prefix" is true, any items
* matching the name as prefix are excluded.
*/
struct exclude_list_item
{
const char *name;
bool match_prefix;
};
/* /*
* The contents of these directories are removed or recreated during server * The contents of these directories are removed or recreated during server
* start so they are not included in backups. The directories themselves are * start so they are not included in backups. The directories themselves are
@ -172,16 +184,19 @@ static const char *excludeDirContents[] =
/* /*
* List of files excluded from backups. * List of files excluded from backups.
*/ */
static const char *excludeFiles[] = static const struct exclude_list_item excludeFiles[] =
{ {
/* Skip auto conf temporary file. */ /* Skip auto conf temporary file. */
PG_AUTOCONF_FILENAME ".tmp", {PG_AUTOCONF_FILENAME ".tmp", false},
/* Skip current log file temporary file */ /* Skip current log file temporary file */
LOG_METAINFO_DATAFILE_TMP, {LOG_METAINFO_DATAFILE_TMP, false},
/* Skip relation cache because it is rebuilt on startup */ /*
RELCACHE_INIT_FILENAME, * Skip relation cache because it is rebuilt on startup. This includes
* temporary files.
*/
{RELCACHE_INIT_FILENAME, true},
/* /*
* If there's a backup_label or tablespace_map file, it belongs to a * If there's a backup_label or tablespace_map file, it belongs to a
@ -189,14 +204,14 @@ static const char *excludeFiles[] =
* for this backup. Our backup_label/tablespace_map is injected into the * for this backup. Our backup_label/tablespace_map is injected into the
* tar separately. * tar separately.
*/ */
BACKUP_LABEL_FILE, {BACKUP_LABEL_FILE, false},
TABLESPACE_MAP, {TABLESPACE_MAP, false},
"postmaster.pid", {"postmaster.pid", false},
"postmaster.opts", {"postmaster.opts", false},
/* end of list */ /* end of list */
NULL {NULL, false}
}; };
/* /*
@ -205,16 +220,15 @@ static const char *excludeFiles[] =
* Note: this list should be kept in sync with what pg_verify_checksums.c * Note: this list should be kept in sync with what pg_verify_checksums.c
* includes. * includes.
*/ */
static const char *noChecksumFiles[] = { static const struct exclude_list_item noChecksumFiles[] = {
"pg_control", {"pg_control", false},
"pg_filenode.map", {"pg_filenode.map", false},
"pg_internal.init", {"pg_internal.init", true},
"PG_VERSION", {"PG_VERSION", false},
#ifdef EXEC_BACKEND #ifdef EXEC_BACKEND
"config_exec_params", {"config_exec_params", true},
"config_exec_params.new",
#endif #endif
NULL, {NULL, false}
}; };
@ -1107,9 +1121,13 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
/* Scan for files that should be excluded */ /* Scan for files that should be excluded */
excludeFound = false; excludeFound = false;
for (excludeIdx = 0; excludeFiles[excludeIdx] != NULL; excludeIdx++) for (excludeIdx = 0; excludeFiles[excludeIdx].name != NULL; excludeIdx++)
{ {
if (strcmp(de->d_name, excludeFiles[excludeIdx]) == 0) int cmplen = strlen(excludeFiles[excludeIdx].name);
if (!excludeFiles[excludeIdx].match_prefix)
cmplen++;
if (strncmp(de->d_name, excludeFiles[excludeIdx].name, cmplen) == 0)
{ {
elog(DEBUG1, "file \"%s\" excluded from backup", de->d_name); elog(DEBUG1, "file \"%s\" excluded from backup", de->d_name);
excludeFound = true; excludeFound = true;
@ -1342,17 +1360,24 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
static bool static bool
is_checksummed_file(const char *fullpath, const char *filename) is_checksummed_file(const char *fullpath, const char *filename)
{ {
const char **f;
/* Check that the file is in a tablespace */ /* Check that the file is in a tablespace */
if (strncmp(fullpath, "./global/", 9) == 0 || if (strncmp(fullpath, "./global/", 9) == 0 ||
strncmp(fullpath, "./base/", 7) == 0 || strncmp(fullpath, "./base/", 7) == 0 ||
strncmp(fullpath, "/", 1) == 0) strncmp(fullpath, "/", 1) == 0)
{ {
/* Compare file against noChecksumFiles skiplist */ int excludeIdx;
for (f = noChecksumFiles; *f; f++)
if (strcmp(*f, filename) == 0) /* Compare file against noChecksumFiles skip list */
for (excludeIdx = 0; noChecksumFiles[excludeIdx].name != NULL; excludeIdx++)
{
int cmplen = strlen(noChecksumFiles[excludeIdx].name);
if (!noChecksumFiles[excludeIdx].match_prefix)
cmplen++;
if (strncmp(filename, noChecksumFiles[excludeIdx].name,
cmplen) == 0)
return false; return false;
}
return true; return true;
} }

View File

@ -6,7 +6,7 @@ use File::Basename qw(basename dirname);
use File::Path qw(rmtree); use File::Path qw(rmtree);
use PostgresNode; use PostgresNode;
use TestLib; use TestLib;
use Test::More tests => 106; use Test::More tests => 107;
program_help_ok('pg_basebackup'); program_help_ok('pg_basebackup');
program_version_ok('pg_basebackup'); program_version_ok('pg_basebackup');
@ -65,8 +65,8 @@ $node->restart;
# Write some files to test that they are not copied. # Write some files to test that they are not copied.
foreach my $filename ( foreach my $filename (
qw(backup_label tablespace_map postgresql.auto.conf.tmp current_logfiles.tmp) qw(backup_label tablespace_map postgresql.auto.conf.tmp
) current_logfiles.tmp global/pg_internal.init.123))
{ {
open my $file, '>>', "$pgdata/$filename"; open my $file, '>>', "$pgdata/$filename";
print $file "DONOTCOPY"; print $file "DONOTCOPY";
@ -135,7 +135,7 @@ foreach my $dirname (
# These files should not be copied. # These files should not be copied.
foreach my $filename ( foreach my $filename (
qw(postgresql.auto.conf.tmp postmaster.opts postmaster.pid tablespace_map current_logfiles.tmp qw(postgresql.auto.conf.tmp postmaster.opts postmaster.pid tablespace_map current_logfiles.tmp
global/pg_internal.init)) global/pg_internal.init global/pg_internal.init.123))
{ {
ok(!-f "$tempdir/backup/$filename", "$filename not copied"); ok(!-f "$tempdir/backup/$filename", "$filename not copied");
} }

View File

@ -32,6 +32,18 @@ static int final_filemap_cmp(const void *a, const void *b);
static void filemap_list_to_array(filemap_t *map); static void filemap_list_to_array(filemap_t *map);
static bool check_file_excluded(const char *path, bool is_source); static bool check_file_excluded(const char *path, bool is_source);
/*
* Definition of one element part of an exclusion list, used to exclude
* contents when rewinding. "name" is the name of the file or path to
* check for exclusion. If "match_prefix" is true, any items matching
* the name as prefix are excluded.
*/
struct exclude_list_item
{
const char *name;
bool match_prefix;
};
/* /*
* The contents of these directories are removed or recreated during server * The contents of these directories are removed or recreated during server
* start so they are not included in data processed by pg_rewind. * start so they are not included in data processed by pg_rewind.
@ -80,32 +92,34 @@ static const char *excludeDirContents[] =
}; };
/* /*
* List of files excluded from filemap processing. * List of files excluded from filemap processing. Files are excluded
* if their prefix match.
*/ */
static const char *excludeFiles[] = static const struct exclude_list_item excludeFiles[] =
{ {
/* Skip auto conf temporary file. */ /* Skip auto conf temporary file. */
"postgresql.auto.conf.tmp", /* defined as PG_AUTOCONF_FILENAME */ {"postgresql.auto.conf.tmp", false}, /* defined as PG_AUTOCONF_FILENAME */
/* Skip current log file temporary file */ /* Skip current log file temporary file */
"current_logfiles.tmp", /* defined as LOG_METAINFO_DATAFILE_TMP */ {"current_logfiles.tmp", false}, /* defined as
* LOG_METAINFO_DATAFILE_TMP */
/* Skip relation cache because it is rebuilt on startup */ /* Skip relation cache because it is rebuilt on startup */
"pg_internal.init", /* defined as RELCACHE_INIT_FILENAME */ {"pg_internal.init", true}, /* defined as RELCACHE_INIT_FILENAME */
/* /*
* If there's a backup_label or tablespace_map file, it belongs to a * If there's a backup_label or tablespace_map file, it belongs to a
* backup started by the user with pg_start_backup(). It is *not* correct * backup started by the user with pg_start_backup(). It is *not* correct
* for this backup. Our backup_label is written later on separately. * for this backup. Our backup_label is written later on separately.
*/ */
"backup_label", /* defined as BACKUP_LABEL_FILE */ {"backup_label", false}, /* defined as BACKUP_LABEL_FILE */
"tablespace_map", /* defined as TABLESPACE_MAP */ {"tablespace_map", false}, /* defined as TABLESPACE_MAP */
"postmaster.pid", {"postmaster.pid", false},
"postmaster.opts", {"postmaster.opts", false},
/* end of list */ /* end of list */
NULL {NULL, false}
}; };
/* /*
@ -498,14 +512,19 @@ check_file_excluded(const char *path, bool is_source)
const char *filename; const char *filename;
/* check individual files... */ /* check individual files... */
for (excludeIdx = 0; excludeFiles[excludeIdx] != NULL; excludeIdx++) for (excludeIdx = 0; excludeFiles[excludeIdx].name != NULL; excludeIdx++)
{ {
int cmplen = strlen(excludeFiles[excludeIdx].name);
filename = last_dir_separator(path); filename = last_dir_separator(path);
if (filename == NULL) if (filename == NULL)
filename = path; filename = path;
else else
filename++; filename++;
if (strcmp(filename, excludeFiles[excludeIdx]) == 0)
if (!excludeFiles[excludeIdx].match_prefix)
cmplen++;
if (strncmp(filename, excludeFiles[excludeIdx].name, cmplen) == 0)
{ {
if (is_source) if (is_source)
pg_log(PG_DEBUG, "entry \"%s\" excluded from source file list\n", pg_log(PG_DEBUG, "entry \"%s\" excluded from source file list\n",

View File

@ -50,31 +50,48 @@ usage(void)
printf(_("Report bugs to <pgsql-bugs@postgresql.org>.\n")); printf(_("Report bugs to <pgsql-bugs@postgresql.org>.\n"));
} }
/*
* Definition of one element part of an exclusion list, used for files
* to exclude from checksum validation. "name" is the name of the file
* or path to check for exclusion. If "match_prefix" is true, any items
* matching the name as prefix are excluded.
*/
struct exclude_list_item
{
const char *name;
bool match_prefix;
};
/* /*
* List of files excluded from checksum validation. * List of files excluded from checksum validation.
* *
* Note: this list should be kept in sync with what basebackup.c includes. * Note: this list should be kept in sync with what basebackup.c includes.
*/ */
static const char *const skip[] = { static const struct exclude_list_item skip[] = {
"pg_control", {"pg_control", false},
"pg_filenode.map", {"pg_filenode.map", false},
"pg_internal.init", {"pg_internal.init", true},
"PG_VERSION", {"PG_VERSION", false},
#ifdef EXEC_BACKEND #ifdef EXEC_BACKEND
"config_exec_params", {"config_exec_params", true},
"config_exec_params.new",
#endif #endif
NULL, {NULL, false}
}; };
static bool static bool
skipfile(const char *fn) skipfile(const char *fn)
{ {
const char *const *f; int excludeIdx;
for (f = skip; *f; f++) for (excludeIdx = 0; skip[excludeIdx].name != NULL; excludeIdx++)
if (strcmp(*f, fn) == 0) {
int cmplen = strlen(skip[excludeIdx].name);
if (!skip[excludeIdx].match_prefix)
cmplen++;
if (strncmp(skip[excludeIdx].name, fn, cmplen) == 0)
return true; return true;
}
return false; return false;
} }