fix PGPRO-1507: null-sized files were not copied to backup

Author: gsmolk

src/backup.c

@@ -586,7 +586,7 @@ do_backup_instance(void)
 						* For backup from master wait for previous segment.
 						* For backup from replica wait for current segment.
 						*/
-					   !from_replica);
+					   !from_replica, backup_files_list);
 	}
 
 	if (current.backup_mode == BACKUP_MODE_DIFF_PTRACK)
@@ -1948,7 +1948,7 @@ backup_files(void *arg)
 		if (S_ISREG(buf.st_mode))
 		{
 			/* Check that file exist in previous backup */
-			if (current.backup_mode == BACKUP_MODE_DIFF_DELTA)
+			if (current.backup_mode != BACKUP_MODE_FULL)
 			{
 				int			p;
 				char	   *relative;
@@ -1961,7 +1961,7 @@ backup_files(void *arg)
 					{
 						/* File exists in previous backup */
 						file->exists_in_prev = true;
-						elog(INFO, "File exists at the time of previous backup %s", relative);
+						// elog(VERBOSE, "File exists at the time of previous backup %s", relative);
 						break;
 					}
 				}
@@ -1981,7 +1981,17 @@ backup_files(void *arg)
 					continue;
 				}
 			}
-			else if (!copy_file(arguments->from_root,
+			else
+				/* TODO:
+				 * Check if file exists in previous backup
+				 * If exists:
+				 *   if mtime > start_backup_time of parent backup,
+				 *    copy file to backup
+				 *   if mtime < start_backup_time
+				 *    calculate crc, compare crc to old file
+				 *      if crc is the same -> skip file
+				 */
+				if (!copy_file(arguments->from_root,
 							   arguments->to_root,
 							   file))
 			{

src/data.c

@@ -422,15 +422,21 @@ backup_data_file(backup_files_args* arguments,
 	int n_blocks_skipped = 0;
 	int n_blocks_read = 0;
 
+	/*
+	 * Skip unchanged file only if it exists in previous backup.
+	 * This way we can correctly handle null-sized files which are
+	 * not tracked by pagemap and thus always marked as unchanged.
+	 */
 	if ((backup_mode == BACKUP_MODE_DIFF_PAGE ||
 		backup_mode == BACKUP_MODE_DIFF_PTRACK) &&
-		file->pagemap.bitmapsize == PageBitmapIsEmpty)
+		file->pagemap.bitmapsize == PageBitmapIsEmpty &&
+		file->exists_in_prev)
 	{
 		/*
 		 * There are no changed blocks since last backup. We want make
 		 * incremental backup, so we should exit.
 		 */
-		elog(VERBOSE, "Skipping the file because it didn`t changed: %s", file->path);
+		elog(VERBOSE, "Skipping the unchanged file: %s", file->path);
 		return false;
 	}
 
@@ -485,10 +491,12 @@ backup_data_file(backup_files_args* arguments,
 
 	/*
 	 * Read each page, verify checksum and write it to backup.
-	 * If page map is empty backup all pages of the relation.
+	 * If page map is empty or file is not present in previous backup
+	 * backup all pages of the relation.
 	 */
 	if (file->pagemap.bitmapsize == PageBitmapIsEmpty
-		|| file->pagemap.bitmapsize == PageBitmapIsAbsent)
+		|| file->pagemap.bitmapsize == PageBitmapIsAbsent
+		|| !file->exists_in_prev)
 	{
 		for (blknum = 0; blknum < nblocks; blknum++)
 		{

src/dir.c

@@ -156,7 +156,7 @@ pgFileInit(const char *path)
 	file->is_datafile = false;
 	file->linked = NULL;
 	file->pagemap.bitmap = NULL;
-	file->pagemap.bitmapsize = (current.backup_mode == BACKUP_MODE_DIFF_PAGE) ? PageBitmapIsEmpty : PageBitmapIsAbsent;
+	file->pagemap.bitmapsize = PageBitmapIsAbsent;
 	file->tblspcOid = 0;
 	file->dbOid = 0;
 	file->relOid = 0;

src/parsexlog.c

@@ -115,8 +115,9 @@ static int SimpleXLogPageRead(XLogReaderState *xlogreader,
  */
 void
 extractPageMap(const char *archivedir, XLogRecPtr startpoint, TimeLineID tli,
-			   XLogRecPtr endpoint, bool prev_segno)
+			   XLogRecPtr endpoint, bool prev_segno, parray *files)
 {
+	size_t		i;
 	XLogRecord *record;
 	XLogReaderState *xlogreader;
 	char	   *errormsg;
@@ -187,6 +188,15 @@ extractPageMap(const char *archivedir, XLogRecPtr startpoint, TimeLineID tli,
 		xlogreadfd = -1;
 		xlogexists = false;
 	}
+
+	/* Mark every datafile with empty pagemap as unchanged */
+	for (i = 0; i < parray_num(files); i++)
+	{
+		pgFile	   *file = (pgFile *) parray_get(files, i);
+		if (file->is_datafile && file->pagemap.bitmap == NULL)
+			file->pagemap.bitmapsize = PageBitmapIsEmpty;
+	}
+
 	elog(LOG, "Pagemap compiled");
 }
 

src/pg_probackup.h

@@ -107,8 +107,8 @@ typedef struct pgFile
 } pgFile;
 
 /* Special values of datapagemap_t bitmapsize */
-#define PageBitmapIsEmpty 0
-#define PageBitmapIsAbsent -1
+#define PageBitmapIsEmpty 0		/* Used to mark unchanged datafiles */
+#define PageBitmapIsAbsent -1	/* Used to mark files with unknown state of pagemap, i.e. datafiles without _ptrack */
 
 /* Current state of backup */
 typedef enum BackupStatus
@@ -468,7 +468,8 @@ extern bool calc_file_checksum(pgFile *file);
 extern void extractPageMap(const char *datadir,
 						   XLogRecPtr startpoint,
 						   TimeLineID tli,
-						   XLogRecPtr endpoint, bool prev_segno);
+						   XLogRecPtr endpoint, bool prev_segno,
+						   parray *backup_files_list);
 extern void validate_wal(pgBackup *backup,
 						 const char *archivedir,
 						 time_t target_time,

tests/delta.py

@@ -14,10 +14,12 @@ class DeltaTest(ProbackupTest, unittest.TestCase):
 
     # @unittest.skip("skip")
     def test_delta_vacuum_truncate_1(self):
-        """make node, create table, take full backup,
-           delete last 3 pages, vacuum relation,
-           take delta backup, take second delta backup,
-           restore latest delta backup and check data correctness"""
+        """
+        make node, create table, take full backup,
+        delete last 3 pages, vacuum relation,
+        take delta backup, take second delta backup,
+        restore latest delta backup and check data correctness
+        """
         fname = self.id().split('.')[3]
         backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
         node = self.make_simple_node(
@@ -100,10 +102,12 @@ def test_delta_vacuum_truncate_1(self):
 
     # @unittest.skip("skip")
     def test_delta_vacuum_truncate_2(self):
-        """make node, create table, take full backup,
-           delete last 3 pages, vacuum relation,
-           take delta backup, take second delta backup,
-           restore latest delta backup and check data correctness"""
+        """
+        make node, create table, take full backup,
+        delete last 3 pages, vacuum relation,
+        take delta backup, take second delta backup,
+        restore latest delta backup and check data correctness
+        """
         fname = self.id().split('.')[3]
         backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
         node = self.make_simple_node(
@@ -191,10 +195,12 @@ def test_delta_vacuum_truncate_2(self):
 
     # @unittest.skip("skip")
     def test_delta_vacuum_truncate_3(self):
-        """make node, create table, take full backup,
-           delete last 3 pages, vacuum relation,
-           take delta backup, take second delta backup,
-           restore latest delta backup and check data correctness"""
+        """
+        make node, create table, take full backup,
+        delete last 3 pages, vacuum relation,
+        take delta backup, take second delta backup,
+        restore latest delta backup and check data correctness
+        """
         fname = self.id().split('.')[3]
         backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
         node = self.make_simple_node(
@@ -421,7 +427,7 @@ def test_delta_archive(self):
         # Clean after yourself
         self.del_test_dir(module_name, fname)
 
-    @unittest.skip("skip")
+    # @unittest.skip("skip")
     def test_delta_multiple_segments(self):
         """
         Make node, create table with multiple segments,
@@ -508,8 +514,10 @@ def test_delta_multiple_segments(self):
 
     # @unittest.skip("skip")
     def test_delta_vacuum_full(self):
-        """make node, make full and delta stream backups,
-          restore them and check data correctness"""
+        """
+        make node, make full and delta stream backups,
+        restore them and check data correctness
+        """
         fname = self.id().split('.')[3]
         backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
         node = self.make_simple_node(
@@ -836,8 +844,10 @@ def test_exists_in_previous_backup(self):
 
     # @unittest.skip("skip")
     def test_alter_table_set_tablespace_delta(self):
-        """Make node, create tablespace with table, take full backup,
-         alter tablespace location, take delta backup, restore database."""
+        """
+        Make node, create tablespace with table, take full backup,
+        alter tablespace location, take delta backup, restore database.
+        """
         fname = self.id().split('.')[3]
         backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
         node = self.make_simple_node(
@@ -1023,8 +1033,10 @@ def test_alter_database_set_tablespace_delta(self):
 
     # @unittest.skip("skip")
     def test_delta_delete(self):
-        """Make node, create tablespace with table, take full backup,
-         alter tablespace location, take delta backup, restore database."""
+        """
+        Make node, create tablespace with table, take full backup,
+        alter tablespace location, take delta backup, restore database.
+        """
         fname = self.id().split('.')[3]
         backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
         node = self.make_simple_node(