PGPRO-1457: minor changes proposed by Arthur Zakirov

Author: gsmolk

src/data.c

@@ -316,20 +316,20 @@ backup_data_page(backup_files_args *arguments,
 			if (is_checksum_enabled)
 				((PageHeader) page)->pd_checksum = pg_checksum_page(page, absolute_blknum);
 		}
-		/* get lsn from page */
+		/* get lsn from page, provided by pg_ptrack_get_block() */
 		if (backup_mode == BACKUP_MODE_DIFF_DELTA &&
-				file->exists_in_prev &&
-					header.compressed_size != PageIsTruncated &&
-						!parse_page(page, &page_lsn))
-			elog(ERROR, "Cannot parse page after pg_ptrack_get_block. "
-							"Possible risk of a memory corruption");
+			file->exists_in_prev &&
+			header.compressed_size != PageIsTruncated &&
+			!parse_page(page, &page_lsn))
+				elog(ERROR, "Cannot parse page after pg_ptrack_get_block. "
+								"Possible risk of a memory corruption");
 
 	}
 
 	if (backup_mode == BACKUP_MODE_DIFF_DELTA &&
-			file->exists_in_prev &&
-				header.compressed_size != PageIsTruncated &&
-						page_lsn < prev_backup_start_lsn)
+		file->exists_in_prev &&
+		header.compressed_size != PageIsTruncated &&
+		page_lsn < prev_backup_start_lsn)
 	{
 		elog(VERBOSE, "Skipping blknum: %u in file: %s", blknum, file->path);
 		(*n_skipped)++;

src/restore.c

@@ -379,26 +379,6 @@ restore_backup(pgBackup *backup)
 	pgBackupGetPath(backup, list_path, lengthof(list_path), DATABASE_FILE_LIST);
 	files = dir_read_file_list(database_path, list_path);
 
-	/*
-	 * For PAGE and PTRACK backups remove files which haven't changed
-	 * since previous backup and thus were not backed up.
-	 * We can`t do the same when restoring DELTA backup because we need information
-	 * about every file to correctly truncate them.
-	 */
-	if (backup->backup_mode == BACKUP_MODE_DIFF_PAGE ||
-			backup->backup_mode == BACKUP_MODE_DIFF_PTRACK)
-	{
-		for (i = parray_num(files) - 1; i >= 0; i--)
-		{
-			pgFile *file = (pgFile *) parray_get(files, i);
-			if (file->write_size == BYTES_INVALID)
-			{
-				elog(VERBOSE, "The file didn`t changed. Skip restore: %s", file->path);
-				pgFileFree(parray_remove(files, i));
-			}
-		}
-	}
-
 	/* setup threads */
 	for (i = 0; i < parray_num(files); i++)
 	{
@@ -725,6 +705,20 @@ restore_files(void *arg)
 				 i + 1, (unsigned long) parray_num(arguments->files), rel_path);
 
 
+		/*
+		 * For PAGE and PTRACK backups skip files which haven't changed
+		 * since previous backup and thus were not backed up.
+		 * We cannot do the same when restoring DELTA backup because we need information
+		 * about every file to correctly truncate them.
+		 */
+		if (file->write_size == BYTES_INVALID &&
+			(arguments->backup->backup_mode == BACKUP_MODE_DIFF_PAGE
+			|| arguments->backup->backup_mode == BACKUP_MODE_DIFF_PTRACK))
+		{
+			elog(VERBOSE, "The file didn`t changed. Skip restore: %s", file->path);
+			continue;
+		}
+
 		/* Directories was created before */
 		if (S_ISDIR(file->mode))
 		{

tests/delta.py

@@ -156,12 +156,10 @@ def test_delta_vacuum_truncate_2(self):
 
         self.backup_node(
             backup_dir, 'node', node, backup_type='delta'
-#            options=['--log-level-file=verbose']
         )
 
         self.backup_node(
             backup_dir, 'node', node, backup_type='delta'
-#            options=['--log-level-file=verbose']
         )
 
         pgdata = self.pgdata_content(node.data_dir)
@@ -231,25 +229,21 @@ def test_delta_vacuum_truncate_3(self):
             "select pg_relation_filepath('t_heap')"
         ).rstrip()
 
-
         self.backup_node(backup_dir, 'node', node)
 
         print(os.path.join(node.data_dir, filepath + '.1'))
         os.unlink(os.path.join(node.data_dir, filepath + '.1'))
 
         self.backup_node(
             backup_dir, 'node', node, backup_type='delta'
-#            options=['--log-level-file=verbose']
         )
 
         self.backup_node(
             backup_dir, 'node', node, backup_type='delta'
-#            options=['--log-level-file=verbose']
         )
 
         pgdata = self.pgdata_content(node.data_dir)
 
-
         self.restore_node(
             backup_dir,
             'node',
@@ -273,8 +267,10 @@ def test_delta_vacuum_truncate_3(self):
 
     # @unittest.skip("skip")
     def test_delta_stream(self):
-        """make archive node, take full and delta stream backups, restore them and check data correctness"""
-        self.maxDiff = None
+        """
+        make archive node, take 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(
@@ -350,7 +346,10 @@ def test_delta_stream(self):
 
     # @unittest.skip("skip")
     def test_delta_archive(self):
-        """make archive node, take full and delta archive backups, restore them and check data correctness"""
+        """
+        make archive node, take full and delta archive backups,
+        restore them and check data correctness
+        """
         self.maxDiff = None
         fname = self.id().split('.')[3]
         backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
@@ -422,9 +421,12 @@ 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, write some data to it, check delta and data correctness"""
+        """
+        Make node, create table with multiple segments,
+        write some data to it, check delta and data correctness
+        """
         fname = self.id().split('.')[3]
         backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
         node = self.make_simple_node(
@@ -928,8 +930,8 @@ def test_alter_table_set_tablespace_delta(self):
     def test_alter_database_set_tablespace_delta(self):
         """
         Make node, take full backup, create database,
-        take delta backup, alter database tablespace location, take delta backup
-        restore last delta backup.
+        take delta backup, alter database tablespace location,
+        take delta backup restore last delta backup.
         """
         fname = self.id().split('.')[3]
         backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
@@ -953,7 +955,9 @@ def test_alter_database_set_tablespace_delta(self):
         self.backup_node(backup_dir, 'node', node, options=["--stream"])
 
         # CREATE DATABASE DB1
-        node.safe_psql("postgres", "create database db1 tablespace = 'somedata'")
+        node.safe_psql(
+            "postgres",
+            "create database db1 tablespace = 'somedata'")
         node.safe_psql(
             "db1",
             "create table t_heap as select i as id, md5(i::text) as text, "