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

diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c
index f46e73544b75fa1da01b6deeee69752675179115..2eba0dc21a102e4f9415d9d370460b2ecdfae133 100644 (file)
--- a/src/backend/replication/basebackup.c
+++ b/src/backend/replication/basebackup.c
@@ -123,6 +123,18 @@ static int64 total_checksum_failures;
 /* Do not verify checksums. */
 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
  * 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.
  */
-static const char *excludeFiles[] =
+static const struct exclude_list_item excludeFiles[] =
 {
        /* Skip auto conf temporary file. */
-       PG_AUTOCONF_FILENAME ".tmp",
+       {PG_AUTOCONF_FILENAME ".tmp", false},
 
        /* 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
@@ -189,14 +204,14 @@ static const char *excludeFiles[] =
         * for this backup.  Our backup_label/tablespace_map is injected into the
         * tar separately.
         */
-       BACKUP_LABEL_FILE,
-       TABLESPACE_MAP,
+       {BACKUP_LABEL_FILE, false},
+       {TABLESPACE_MAP, false},
 
-       "postmaster.pid",
-       "postmaster.opts",
+       {"postmaster.pid", false},
+       {"postmaster.opts", false},
 
        /* 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
  * includes.
  */
-static const char *noChecksumFiles[] = {
-       "pg_control",
-       "pg_filenode.map",
-       "pg_internal.init",
-       "PG_VERSION",
+static const struct exclude_list_item noChecksumFiles[] = {
+       {"pg_control", false},
+       {"pg_filenode.map", false},
+       {"pg_internal.init", true},
+       {"PG_VERSION", false},
 #ifdef EXEC_BACKEND
-       "config_exec_params",
-       "config_exec_params.new",
+       {"config_exec_params", true},
 #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 */
                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);
                                excludeFound = true;
@@ -1342,17 +1360,24 @@ sendDir(const char *path, int basepathlen, bool sizeonly, List *tablespaces,
 static bool
 is_checksummed_file(const char *fullpath, const char *filename)
 {
-       const char **f;
-
        /* Check that the file is in a tablespace */
        if (strncmp(fullpath, "./global/", 9) == 0 ||
                strncmp(fullpath, "./base/", 7) == 0 ||
                strncmp(fullpath, "/", 1) == 0)
        {
-               /* Compare file against noChecksumFiles skiplist */
-               for (f = noChecksumFiles; *f; f++)
-                       if (strcmp(*f, filename) == 0)
+               int                     excludeIdx;
+
+               /* 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 true;
        }

diff --git a/src/bin/pg_basebackup/t/010_pg_basebackup.pl b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
index 000e09e0a73bc7eddd9a05bc838ab36a01b124f7..851a7db52f733869e98ca90d7edd1243ead6a51d 100644 (file)
--- a/src/bin/pg_basebackup/t/010_pg_basebackup.pl
+++ b/src/bin/pg_basebackup/t/010_pg_basebackup.pl
@@ -6,7 +6,7 @@ use File::Basename qw(basename dirname);
 use File::Path qw(rmtree);
 use PostgresNode;
 use TestLib;
-use Test::More tests => 106;
+use Test::More tests => 107;
 
 program_help_ok('pg_basebackup');
 program_version_ok('pg_basebackup');
@@ -65,8 +65,8 @@ $node->restart;
 
 # Write some files to test that they are not copied.
 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";
        print $file "DONOTCOPY";
@@ -135,7 +135,7 @@ foreach my $dirname (
 # These files should not be copied.
 foreach my $filename (
        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");
 }

diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index d70f1189387ab76cf45381988155f61f76739df3..197163d5544f7c0d083f4f01d6c6dff5eafb9f8f 100644 (file)
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -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 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
  * 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. */
-       "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 */
-       "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 */
-       "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
         * backup started by the user with pg_start_backup().  It is *not* correct
         * for this backup.  Our backup_label is written later on separately.
         */
-       "backup_label",                         /* defined as BACKUP_LABEL_FILE */
-       "tablespace_map",                       /* defined as TABLESPACE_MAP */
+       {"backup_label", false},        /* defined as BACKUP_LABEL_FILE */
+       {"tablespace_map", false},      /* defined as TABLESPACE_MAP */
 
-       "postmaster.pid",
-       "postmaster.opts",
+       {"postmaster.pid", false},
+       {"postmaster.opts", false},
 
        /* end of list */
-       NULL
+       {NULL, false}
 };
 
 /*
@@ -498,14 +512,19 @@ check_file_excluded(const char *path, bool is_source)
        const char *filename;
 
        /* 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);
                if (filename == NULL)
                        filename = path;
                else
                        filename++;
-               if (strcmp(filename, excludeFiles[excludeIdx]) == 0)
+
+               if (!excludeFiles[excludeIdx].match_prefix)
+                       cmplen++;
+               if (strncmp(filename, excludeFiles[excludeIdx].name, cmplen) == 0)
                {
                        if (is_source)
                                pg_log(PG_DEBUG, "entry \"%s\" excluded from source file list\n",

diff --git a/src/bin/pg_verify_checksums/pg_verify_checksums.c b/src/bin/pg_verify_checksums/pg_verify_checksums.c
index 9316835995c76ae2120f4550bf16e03403929107..a33ac6924f89e7d4c2b777bd8ac066037ffbdb6c 100644 (file)
--- a/src/bin/pg_verify_checksums/pg_verify_checksums.c
+++ b/src/bin/pg_verify_checksums/pg_verify_checksums.c
@@ -50,31 +50,48 @@ usage(void)
        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.
  *
  * Note: this list should be kept in sync with what basebackup.c includes.
  */
-static const char *const skip[] = {
-       "pg_control",
-       "pg_filenode.map",
-       "pg_internal.init",
-       "PG_VERSION",
+static const struct exclude_list_item skip[] = {
+       {"pg_control", false},
+       {"pg_filenode.map", false},
+       {"pg_internal.init", true},
+       {"PG_VERSION", false},
 #ifdef EXEC_BACKEND
-       "config_exec_params",
-       "config_exec_params.new",
+       {"config_exec_params", true},
 #endif
-       NULL,
+       {NULL, false}
 };
 
 static bool
 skipfile(const char *fn)
 {
-       const char *const *f;
+       int             excludeIdx;
+
+       for (excludeIdx = 0; skip[excludeIdx].name != NULL; excludeIdx++)
+       {
+               int             cmplen = strlen(skip[excludeIdx].name);
 
-       for (f = skip; *f; f++)
-               if (strcmp(*f, fn) == 0)
+               if (!skip[excludeIdx].match_prefix)
+                       cmplen++;
+               if (strncmp(skip[excludeIdx].name, fn, cmplen) == 0)
                        return true;
+       }
 
        return false;
 }