bugfix: do not set is_datafile in set_forkname() (#19)

Co-authored-by: Grigory Smolkin <gsmolkin@ozon.ru>

Author: gsmolk

src/dir.c

@@ -189,6 +189,7 @@ pgFileInit(const char *rel_path)
 	// May be add?
 	// pg_atomic_clear_flag(file->lock);
 	file->excluded = false;
+	file->is_datafile = false;
 	return file;
 }
 
@@ -627,22 +628,14 @@ dir_check_file(pgFile *file, bool backup_logs)
 	{
 		if (strcmp(file->name, "pg_internal.init") == 0)
 			return CHECK_FALSE;
-		/* Do not backup ptrack2.x temp map files */
-//		else if (strcmp(file->name, "ptrack.map") == 0)
-//			return CHECK_FALSE;
-		else if (strcmp(file->name, "ptrack.map.mmap") == 0)
-			return CHECK_FALSE;
-		else if (strcmp(file->name, "ptrack.map.tmp") == 0)
-			return CHECK_FALSE;
 		/* Do not backup temp files */
 		else if (file->name[0] == 't' && isdigit(file->name[1]))
 			return CHECK_FALSE;
+		/* if filename starts with digits, it can be either datafile or fork */
 		else if (isdigit(file->name[0]))
 		{
 			set_forkname(file);
-
-			if (file->forkName == ptrack) /* Compatibility with left-overs from ptrack1 */
-				return CHECK_FALSE;
+			file->is_datafile = file->forkName == none;
 		}
 	}
 
@@ -1633,7 +1626,7 @@ is_forkname(char *name, size_t *pos, const char *forkname)
 #define SEGNOCHARS 5 /* when BLCKSZ == (1<<15) */
 
 /* Set forkName if possible */
-bool
+void
 set_forkname(pgFile *file)
 {
 	size_t i = 0;
@@ -1643,16 +1636,15 @@ set_forkname(pgFile *file)
 	/* pretend it is not relation file */
 	file->relOid = 0;
 	file->forkName = none;
-	file->is_datafile = false;
 
 	for (i = 0; isdigit(file->name[i]); i++)
 	{
 		if (i == 0 && file->name[i] == '0')
-			return false;
+			return;
 		oid = oid * 10 + file->name[i] - '0';
 	}
 	if (i == 0 || i > OIDCHARS || oid > UINT32_MAX)
-		return false;
+		return;
 
 	/* usual fork name */
 	/* /^\d+_(vm|fsm|init|ptrack)$/ */
@@ -1673,22 +1665,21 @@ set_forkname(pgFile *file)
 		for (i++; isdigit(file->name[i]); i++)
 		{
 			if (i == start && file->name[i] == '0')
-				return false;
+				return;
 			segno = segno * 10 + file->name[i] - '0';
 		}
 		if (i - start > SEGNOCHARS || segno > MAXSEGNO)
-			return false;
+			return;
 	}
 
 	/* If there are excess characters, it is not relation file */
 	if (file->name[i] != 0)
 	{
 		file->forkName = none;
-		return false;
+		return;
 	}
 
 	file->relOid = oid;
 	file->segno = segno;
-	file->is_datafile = file->forkName == none;
-	return true;
+	return;
 }

src/pg_probackup.h

@@ -1005,7 +1005,7 @@ extern int pgCompareString(const void *str1, const void *str2);
 extern int pgPrefixCompareString(const void *str1, const void *str2);
 extern int pgCompareOid(const void *f1, const void *f2);
 extern void pfilearray_clear_locks(parray *file_list);
-extern bool set_forkname(pgFile *file);
+extern void set_forkname(pgFile *file);
 
 /* in data.c */
 extern bool check_data_file(ConnectionArgs *arguments, pgFile *file,

src/validate.c

@@ -287,7 +287,20 @@ pgBackupValidateFiles(void *arg)
 		 * If option skip-block-validation is set, compute only file-level CRC for
 		 * datafiles, otherwise check them block by block.
 		 */
-		if (!file->is_datafile || skip_block_validation)
+		if (file->is_datafile && !skip_block_validation)
+		{
+			/*
+			 * validate relation block by block
+			 * check page headers, checksums (if enabled)
+			 * and compute checksum of the file
+			 */
+			if (!validate_file_pages(file, file_fullpath, arguments->stop_lsn,
+								  arguments->checksum_version,
+								  arguments->program_version_num,
+								  arguments->hdr_map))
+				arguments->corrupted = true;
+		}
+		else
 		{
 			if (strcmp(file->rel_path, XLOG_CONTROL_FILE) == 0)
 				crc = get_pgcontrol_checksum(arguments->base_path);
@@ -301,19 +314,6 @@ pgBackupValidateFiles(void *arg)
 				arguments->corrupted = true;
 			}
 		}
-		else
-		{
-			/*
-			 * validate relation block by block
-			 * check page headers, checksums (if enabled)
-			 * and compute checksum of the file
-			 */
-			if (!validate_file_pages(file, file_fullpath, arguments->stop_lsn,
-								  arguments->checksum_version,
-								  arguments->program_version_num,
-								  arguments->hdr_map))
-				arguments->corrupted = true;
-		}
 	}
 
 	/* Data files validation is successful */

tests/Readme.md

@@ -8,7 +8,7 @@ Note: tests require python3 to work properly.
 
 ```
 Check physical correctness of restored instances:
- Apply this patch to disable HINT BITS: https://gist.github.com/gsmol/2bb34fd3ba31984369a72cc1c27a36b6
+ Apply this patch to disable HINT BITS: https://gist.github.com/gsmolk/2bb34fd3ba31984369a72cc1c27a36b6
  export PG_PROBACKUP_PARANOIA=ON
 
 Check archive compression:

tests/helpers/ptrack_helpers.py

@@ -112,7 +112,6 @@ class PostgresNodeExtended(testgres.PostgresNode):
 
     def __init__(self, base_dir=None, *args, **kwargs):
         super(PostgresNodeExtended, self).__init__(name='test', base_dir=base_dir, *args, **kwargs)
-        self.is_started = False
 
     def slow_start(self, replica=False):
 
@@ -150,13 +149,11 @@ def slow_start(self, replica=False):
     def start(self, *args, **kwargs):
         if not self.is_started:
             super(PostgresNodeExtended, self).start(*args, **kwargs)
-            self.is_started = True
         return self
 
     def stop(self, *args, **kwargs):
         if self.is_started:
             result = super(PostgresNodeExtended, self).stop(*args, **kwargs)
-            self.is_started = False
             return result
 
     def kill(self, someone = None):
@@ -165,7 +162,6 @@ def kill(self, someone = None):
                 os.kill(self.pid, signal.SIGKILL)
             else:
                 os.kill(self.auxiliary_pids[someone][0], signal.SIGKILL)
-            self.is_started = False
 
     def table_checksum(self, table, dbname="postgres"):
         con = self.connect(dbname=dbname)