Skip to content

Commit bf883b2

Browse files
michaelpqmichaelpq
committed
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
1 parent 79c2385 commit bf883b2

File tree

5 files changed

+116
-53
lines changed

5 files changed

+116
-53
lines changed

‎src/backend/replication/basebackup.c

Lines changed: 50 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -121,6 +121,18 @@ static long long int total_checksum_failures;
121121
/* Do not verify checksums. */
122122
static bool noverify_checksums = false;
123123

124+
/*
125+
* Definition of one element part of an exclusion list, used for paths part
126+
* of checksum validation or base backups. "name" is the name of the file
127+
* or path to check for exclusion. If "match_prefix" is true, any items
128+
* matching the name as prefix are excluded.
129+
*/
130+
struct exclude_list_item
131+
{
132+
const char *name;
133+
bool match_prefix;
134+
};
135+
124136
/*
125137
* The contents of these directories are removed or recreated during server
126138
* start so they are not included in backups. The directories themselves are
@@ -170,31 +182,34 @@ static const char *const excludeDirContents[] =
170182
/*
171183
* List of files excluded from backups.
172184
*/
173-
static const char *const excludeFiles[] =
185+
static const struct exclude_list_item excludeFiles[] =
174186
{
175187
/* Skip auto conf temporary file. */
176-
PG_AUTOCONF_FILENAME ".tmp",
188+
{PG_AUTOCONF_FILENAME ".tmp", false},
177189

178190
/* Skip current log file temporary file */
179-
LOG_METAINFO_DATAFILE_TMP,
191+
{LOG_METAINFO_DATAFILE_TMP, false},
180192

181-
/* Skip relation cache because it is rebuilt on startup */
182-
RELCACHE_INIT_FILENAME,
193+
/*
194+
* Skip relation cache because it is rebuilt on startup. This includes
195+
* temporary files.
196+
*/
197+
{RELCACHE_INIT_FILENAME, true},
183198

184199
/*
185200
* If there's a backup_label or tablespace_map file, it belongs to a
186201
* backup started by the user with pg_start_backup(). It is *not* correct
187202
* for this backup. Our backup_label/tablespace_map is injected into the
188203
* tar separately.
189204
*/
190-
BACKUP_LABEL_FILE,
191-
TABLESPACE_MAP,
205+
{BACKUP_LABEL_FILE, false},
206+
{TABLESPACE_MAP, false},
192207

193-
"postmaster.pid",
194-
"postmaster.opts",
208+
{"postmaster.pid", false},
209+
{"postmaster.opts", false},
195210

196211
/* end of list */
197-
NULL
212+
{NULL, false}
198213
};
199214

200215
/*
@@ -203,16 +218,15 @@ static const char *const excludeFiles[] =
203218
* Note: this list should be kept in sync with what pg_checksums.c
204219
* includes.
205220
*/
206-
static const char *const noChecksumFiles[] = {
207-
"pg_control",
208-
"pg_filenode.map",
209-
"pg_internal.init",
210-
"PG_VERSION",
221+
static const struct exclude_list_item noChecksumFiles[] = {
222+
{"pg_control", false},
223+
{"pg_filenode.map", false},
224+
{"pg_internal.init", true},
225+
{"PG_VERSION", false},
211226
#ifdef EXEC_BACKEND
212-
"config_exec_params",
213-
"config_exec_params.new",
227+
{"config_exec_params", true},
214228
#endif
215-
NULL,
229+
{NULL, false}
216230
};
217231

218232
/*
@@ -1082,9 +1096,13 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
10821096

10831097
/* Scan for files that should be excluded */
10841098
excludeFound = false;
1085-
for (excludeIdx = 0; excludeFiles[excludeIdx] != NULL; excludeIdx++)
1099+
for (excludeIdx = 0; excludeFiles[excludeIdx].name != NULL; excludeIdx++)
10861100
{
1087-
if (strcmp(de->d_name, excludeFiles[excludeIdx]) == 0)
1101+
int cmplen = strlen(excludeFiles[excludeIdx].name);
1102+
1103+
if (!excludeFiles[excludeIdx].match_prefix)
1104+
cmplen++;
1105+
if (strncmp(de->d_name, excludeFiles[excludeIdx].name, cmplen) == 0)
10881106
{
10891107
elog(DEBUG1, "file \"%s\" excluded from backup", de->d_name);
10901108
excludeFound = true;
@@ -1317,17 +1335,24 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
13171335
static bool
13181336
is_checksummed_file(const char *fullpath, const char *filename)
13191337
{
1320-
const char *const *f;
1321-
13221338
/* Check that the file is in a tablespace */
13231339
if (strncmp(fullpath, "./global/", 9) == 0 ||
13241340
strncmp(fullpath, "./base/", 7) == 0 ||
13251341
strncmp(fullpath, "/", 1) == 0)
13261342
{
1327-
/* Compare file against noChecksumFiles skiplist */
1328-
for (f = noChecksumFiles; *f; f++)
1329-
if (strcmp(*f, filename) == 0)
1343+
int excludeIdx;
1344+
1345+
/* Compare file against noChecksumFiles skip list */
1346+
for (excludeIdx = 0; noChecksumFiles[excludeIdx].name != NULL; excludeIdx++)
1347+
{
1348+
int cmplen = strlen(noChecksumFiles[excludeIdx].name);
1349+
1350+
if (!noChecksumFiles[excludeIdx].match_prefix)
1351+
cmplen++;
1352+
if (strncmp(filename, noChecksumFiles[excludeIdx].name,
1353+
cmplen) == 0)
13301354
return false;
1355+
}
13311356

13321357
return true;
13331358
}

‎src/bin/pg_basebackup/t/010_pg_basebackup.pl

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
use File::Path qw(rmtree);
77
use PostgresNode;
88
use TestLib;
9-
use Test::More tests => 106;
9+
use Test::More tests => 107;
1010

1111
program_help_ok('pg_basebackup');
1212
program_version_ok('pg_basebackup');
@@ -65,8 +65,8 @@
6565

6666
# Write some files to test that they are not copied.
6767
foreach my $filename (
68-
qw(backup_label tablespace_map postgresql.auto.conf.tmp current_logfiles.tmp)
69-
)
68+
qw(backup_label tablespace_map postgresql.auto.conf.tmp
69+
current_logfiles.tmp global/pg_internal.init.123))
7070
{
7171
open my $file, '>>', "$pgdata/$filename";
7272
print $file "DONOTCOPY";
@@ -135,7 +135,7 @@
135135
# These files should not be copied.
136136
foreach my $filename (
137137
qw(postgresql.auto.conf.tmp postmaster.opts postmaster.pid tablespace_map current_logfiles.tmp
138-
global/pg_internal.init))
138+
global/pg_internal.init global/pg_internal.init.123))
139139
{
140140
ok(!-f "$tempdir/backup/$filename", "$filename not copied");
141141
}

‎src/bin/pg_checksums/pg_checksums.c

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -91,21 +91,32 @@ usage(void)
9191
printf(_("Report bugs to <pgsql-bugs@lists.postgresql.org>.\n"));
9292
}
9393

94+
/*
95+
* Definition of one element part of an exclusion list, used for files
96+
* to exclude from checksum validation. "name" is the name of the file
97+
* or path to check for exclusion. If "match_prefix" is true, any items
98+
* matching the name as prefix are excluded.
99+
*/
100+
struct exclude_list_item
101+
{
102+
const char *name;
103+
bool match_prefix;
104+
};
105+
94106
/*
95107
* List of files excluded from checksum validation.
96108
*
97109
* Note: this list should be kept in sync with what basebackup.c includes.
98110
*/
99-
static const char *const skip[] = {
100-
"pg_control",
101-
"pg_filenode.map",
102-
"pg_internal.init",
103-
"PG_VERSION",
111+
static const struct exclude_list_item skip[] = {
112+
{"pg_control", false},
113+
{"pg_filenode.map", false},
114+
{"pg_internal.init", true},
115+
{"PG_VERSION", false},
104116
#ifdef EXEC_BACKEND
105-
"config_exec_params",
106-
"config_exec_params.new",
117+
{"config_exec_params", true},
107118
#endif
108-
NULL,
119+
{NULL, false}
109120
};
110121

111122
/*
@@ -157,11 +168,17 @@ progress_report(bool force)
157168
static bool
158169
skipfile(const char *fn)
159170
{
160-
const char *const *f;
171+
int excludeIdx;
172+
173+
for (excludeIdx = 0; skip[excludeIdx].name != NULL; excludeIdx++)
174+
{
175+
int cmplen = strlen(skip[excludeIdx].name);
161176

162-
for (f = skip; *f; f++)
163-
if (strcmp(*f, fn) == 0)
177+
if (!skip[excludeIdx].match_prefix)
178+
cmplen++;
179+
if (strncmp(skip[excludeIdx].name, fn, cmplen) == 0)
164180
return true;
181+
}
165182

166183
return false;
167184
}

‎src/bin/pg_checksums/t/002_actions.pl

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,7 +111,9 @@ sub check_relation_corruption
111111
# should be ignored by the scan.
112112
append_to_file "$pgdata/global/pgsql_tmp_123", "foo";
113113
mkdir "$pgdata/global/pgsql_tmp";
114-
append_to_file "$pgdata/global/pgsql_tmp/1.1", "foo";
114+
append_to_file "$pgdata/global/pgsql_tmp/1.1", "foo";
115+
append_to_file "$pgdata/global/pg_internal.init", "foo";
116+
append_to_file "$pgdata/global/pg_internal.init.123", "foo";
115117

116118
# Enable checksums.
117119
command_ok([ 'pg_checksums', '--enable', '--no-sync', '-D', $pgdata ],

‎src/bin/pg_rewind/filemap.c

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,18 @@ static int final_filemap_cmp(const void *a, const void *b);
3030
static void filemap_list_to_array(filemap_t *map);
3131
static bool check_file_excluded(const char *path, bool is_source);
3232

33+
/*
34+
* Definition of one element part of an exclusion list, used to exclude
35+
* contents when rewinding. "name" is the name of the file or path to
36+
* check for exclusion. If "match_prefix" is true, any items matching
37+
* the name as prefix are excluded.
38+
*/
39+
struct exclude_list_item
40+
{
41+
const char *name;
42+
bool match_prefix;
43+
};
44+
3345
/*
3446
* The contents of these directories are removed or recreated during server
3547
* start so they are not included in data processed by pg_rewind.
@@ -78,32 +90,34 @@ static const char *excludeDirContents[] =
7890
};
7991

8092
/*
81-
* List of files excluded from filemap processing.
93+
* List of files excluded from filemap processing. Files are excluded
94+
* if their prefix match.
8295
*/
83-
static const char *excludeFiles[] =
96+
static const struct exclude_list_item excludeFiles[] =
8497
{
8598
/* Skip auto conf temporary file. */
86-
"postgresql.auto.conf.tmp", /* defined as PG_AUTOCONF_FILENAME */
99+
{"postgresql.auto.conf.tmp", false}, /* defined as PG_AUTOCONF_FILENAME */
87100

88101
/* Skip current log file temporary file */
89-
"current_logfiles.tmp", /* defined as LOG_METAINFO_DATAFILE_TMP */
102+
{"current_logfiles.tmp", false}, /* defined as
103+
* LOG_METAINFO_DATAFILE_TMP */
90104

91105
/* Skip relation cache because it is rebuilt on startup */
92-
"pg_internal.init", /* defined as RELCACHE_INIT_FILENAME */
106+
{"pg_internal.init", true}, /* defined as RELCACHE_INIT_FILENAME */
93107

94108
/*
95109
* If there's a backup_label or tablespace_map file, it belongs to a
96110
* backup started by the user with pg_start_backup(). It is *not* correct
97111
* for this backup. Our backup_label is written later on separately.
98112
*/
99-
"backup_label", /* defined as BACKUP_LABEL_FILE */
100-
"tablespace_map", /* defined as TABLESPACE_MAP */
113+
{"backup_label", false}, /* defined as BACKUP_LABEL_FILE */
114+
{"tablespace_map", false}, /* defined as TABLESPACE_MAP */
101115

102-
"postmaster.pid",
103-
"postmaster.opts",
116+
{"postmaster.pid", false},
117+
{"postmaster.opts", false},
104118

105119
/* end of list */
106-
NULL
120+
{NULL, false}
107121
};
108122

109123
/*
@@ -496,14 +510,19 @@ check_file_excluded(const char *path, bool is_source)
496510
const char *filename;
497511

498512
/* check individual files... */
499-
for (excludeIdx = 0; excludeFiles[excludeIdx] != NULL; excludeIdx++)
513+
for (excludeIdx = 0; excludeFiles[excludeIdx].name != NULL; excludeIdx++)
500514
{
515+
int cmplen = strlen(excludeFiles[excludeIdx].name);
516+
501517
filename = last_dir_separator(path);
502518
if (filename == NULL)
503519
filename = path;
504520
else
505521
filename++;
506-
if (strcmp(filename, excludeFiles[excludeIdx]) == 0)
522+
523+
if (!excludeFiles[excludeIdx].match_prefix)
524+
cmplen++;
525+
if (strncmp(filename, excludeFiles[excludeIdx].name, cmplen) == 0)
507526
{
508527
if (is_source)
509528
pg_log_debug("entry \"%s\" excluded from source file list",

0 commit comments

Comments
 (0)