Merge branch 'master' into issue_79

src/delete.c

@@ -259,6 +259,7 @@ do_retention_internal(parray *backup_list, parray *to_keep_list, parray *to_purg
 	{
 
 		bool redundancy_keep = false;
+		time_t backup_time = 0;
 		pgBackup   *backup = (pgBackup *) parray_get(backup_list, (size_t) i);
 
 		/* check if backup`s FULL ancestor is in redundancy list */
@@ -280,10 +281,16 @@ do_retention_internal(parray *backup_list, parray *to_keep_list, parray *to_purg
 			cur_full_backup_num++;
 		}
 
-		/* Check if backup in needed by retention policy
+		/* Invalid and running backups most likely to have recovery_time == 0,
-		 * TODO: consider that ERROR backup most likely to have recovery_time == 0
+		 * so in this case use start_time instead.
 		 */
-		if ((days_threshold == 0 || (days_threshold > backup->recovery_time)) &&
+		if (backup->recovery_time)
+			backup_time = backup->recovery_time;
+		else
+			backup_time = backup->start_time;
+
+		/* Check if backup in needed by retention policy */
+		if ((days_threshold == 0 || (days_threshold > backup_time)) &&
 			(instance_config.retention_redundancy == 0 || !redundancy_keep))
 		{
 			/* This backup is not guarded by retention
@@ -622,6 +629,7 @@ do_retention_wal(void)
 	XLogRecPtr oldest_lsn = InvalidXLogRecPtr;
 	TimeLineID oldest_tli = 0;
 	bool backup_list_is_empty = false;
+	int i;
 
 	/* Get list of backups. */
 	backup_list = catalog_get_backup_list(INVALID_BACKUP_ID);
@@ -630,14 +638,17 @@ do_retention_wal(void)
 		backup_list_is_empty = true;
 
 	/* Save LSN and Timeline to remove unnecessary WAL segments */
-	if (!backup_list_is_empty)
+	for (i = (int) parray_num(backup_list) - 1; i >= 0; i--)
 	{
-		pgBackup   *backup = NULL;
+		pgBackup   *backup = (pgBackup *) parray_get(backup_list, i);
-		/* Get LSN and TLI of oldest alive backup */
-		backup = (pgBackup *) parray_get(backup_list, parray_num(backup_list) -1);
 
-		oldest_tli = backup->tli;
+		/* Get LSN and TLI of the oldest backup with valid start_lsn and tli */
-		oldest_lsn = backup->start_lsn;
+		if (backup->tli > 0 && !XLogRecPtrIsInvalid(backup->start_lsn))
+		{
+			oldest_tli = backup->tli;
+			oldest_lsn = backup->start_lsn;
+			break;
+		}
 	}
 
 	/* Be paranoid */

src/pg_probackup.c

@@ -663,7 +663,7 @@ main(int argc, char *argv[])
 							  recovery_target_options,
 							 restore_params);
 		case VALIDATE_CMD:
-			if (current.backup_id == 0 && target_time == 0 && target_xid == 0)
+			if (current.backup_id == 0 && target_time == 0 && target_xid == 0 && !target_lsn)
 				return do_validate_all();
 			else
 				return do_restore_or_validate(current.backup_id,

src/restore.c

@@ -143,10 +143,16 @@ do_restore_or_validate(time_t target_backup_id, pgRecoveryTarget *rt,
 		/*
 		 * [PGPRO-1164] If BACKUP_ID is not provided for restore command,
 		 *  we must find the first valid(!) backup.
+
+		 * If target_backup_id is not provided, we can be sure that
+		 * PITR for restore or validate is requested.
+		 * So we can assume that user is more interested in recovery to specific point
+		 * in time and NOT interested in revalidation of invalid backups.
+		 * So based on that assumptions we should choose only OK and DONE backups
+		 * as candidates for validate and restore.
 		 */
 
-		if (params->is_restore &&
+		if (target_backup_id == INVALID_BACKUP_ID &&
-			target_backup_id == INVALID_BACKUP_ID &&
 			(current_backup->status != BACKUP_STATUS_OK &&
 			 current_backup->status != BACKUP_STATUS_DONE))
 		{
@@ -1079,7 +1085,7 @@ parseRecoveryTargetOptions(const char *target_time,
 		if (parse_time(target_time, &dummy_time, false))
 			rt->target_time = dummy_time;
 		else
-			elog(ERROR, "Invalid value for --recovery-target-time option %s",
+			elog(ERROR, "Invalid value for '--recovery-target-time' option %s",
 				 target_time);
 	}
 
@@ -1097,7 +1103,7 @@ parseRecoveryTargetOptions(const char *target_time,
 #endif
 			rt->target_xid = dummy_xid;
 		else
-			elog(ERROR, "Invalid value for --recovery-target-xid option %s",
+			elog(ERROR, "Invalid value for '--recovery-target-xid' option %s",
 				 target_xid);
 	}
 
@@ -1110,7 +1116,7 @@ parseRecoveryTargetOptions(const char *target_time,
 		if (parse_lsn(target_lsn, &dummy_lsn))
 			rt->target_lsn = dummy_lsn;
 		else
-			elog(ERROR, "Invalid value of --recovery-target-lsn option %s",
+			elog(ERROR, "Invalid value of '--recovery-target-lsn' option %s",
 				 target_lsn);
 	}
 
@@ -1120,7 +1126,7 @@ parseRecoveryTargetOptions(const char *target_time,
 		if (parse_bool(target_inclusive, &dummy_bool))
 			rt->target_inclusive = dummy_bool;
 		else
-			elog(ERROR, "Invalid value for --recovery-target-inclusive option %s",
+			elog(ERROR, "Invalid value for '--recovery-target-inclusive' option %s",
 				 target_inclusive);
 	}
 
@@ -1129,7 +1135,7 @@ parseRecoveryTargetOptions(const char *target_time,
 	{
 		if ((strcmp(target_stop, "immediate") != 0)
 			&& (strcmp(target_stop, "latest") != 0))
-			elog(ERROR, "Invalid value for --recovery-target option %s",
+			elog(ERROR, "Invalid value for '--recovery-target' option %s",
 				 target_stop);
 
 		recovery_target_specified++;
@@ -1147,7 +1153,7 @@ parseRecoveryTargetOptions(const char *target_time,
 		if ((strcmp(target_action, "pause") != 0)
 			&& (strcmp(target_action, "promote") != 0)
 			&& (strcmp(target_action, "shutdown") != 0))
-			elog(ERROR, "Invalid value for --recovery-target-action option %s",
+			elog(ERROR, "Invalid value for '--recovery-target-action' option %s",
 				 target_action);
 
 		rt->target_action = target_action;

tests/backup.py

@@ -1437,6 +1437,9 @@ class BackupTest(ProbackupTest, unittest.TestCase):
     # @unittest.skip("skip")
     def test_basic_missing_file_permissions(self):
         """"""
+        if os.name == 'nt':
+            return unittest.skip('Skipped because it is POSIX only test')
+
         fname = self.id().split('.')[3]
         backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
         node = self.make_simple_node(
@@ -1481,6 +1484,9 @@ class BackupTest(ProbackupTest, unittest.TestCase):
     # @unittest.skip("skip")
     def test_basic_missing_dir_permissions(self):
         """"""
+        if os.name == 'nt':
+            return unittest.skip('Skipped because it is POSIX only test')
+
         fname = self.id().split('.')[3]
         backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
         node = self.make_simple_node(

tests/retention.py

@@ -1,7 +1,7 @@
 import os
 import unittest
 from datetime import datetime, timedelta
-from .helpers.ptrack_helpers import ProbackupTest
+from .helpers.ptrack_helpers import ProbackupTest, ProbackupException
 from time import sleep
 
 
@@ -1459,3 +1459,45 @@ class RetentionTest(ProbackupTest, unittest.TestCase):
 
         # Clean after yourself
         self.del_test_dir(module_name, fname)
+
+    def test_wal_purge_victim(self):
+        """
+        https://github.com/postgrespro/pg_probackup/issues/103
+        """
+        fname = self.id().split('.')[3]
+        node = self.make_simple_node(
+            base_dir=os.path.join(module_name, fname, 'node'),
+            initdb_params=['--data-checksums'])
+
+        backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
+        self.init_pb(backup_dir)
+        self.add_instance(backup_dir, 'node', node)
+        self.set_archiving(backup_dir, 'node', node)
+        node.slow_start()
+
+        # Make ERROR incremental backup
+        try:
+            self.backup_node(backup_dir, 'node', node, backup_type='page')
+            # we should die here because exception is what we expect to happen
+            self.assertEqual(
+                1, 0,
+                "Expecting Error because page backup should not be possible "
+                "without valid full backup.\n Output: {0} \n CMD: {1}".format(
+                    repr(self.output), self.cmd))
+        except ProbackupException as e:
+            self.assertIn(
+                "ERROR: Valid backup on current timeline 1 is not found. "
+                "Create new FULL backup before an incremental one.",
+                e.message,
+                "\n Unexpected Error Message: {0}\n CMD: {1}".format(
+                    repr(e.message), self.cmd))
+
+        page_id = self.show_pb(backup_dir, 'node')[0]['id']
+
+        sleep(1)
+
+        # Make FULL backup
+        self.backup_node(backup_dir, 'node', node, options=['--delete-wal'])
+
+        # Clean after yourself
+        self.del_test_dir(module_name, fname)

tests/validate.py

@@ -3443,6 +3443,119 @@ class ValidateTest(ProbackupTest, unittest.TestCase):
                 '\n Unexpected Error Message: {0}\n CMD: {1}'.format(
                     repr(e.message), self.cmd))
 
+    # @unittest.expectedFailure
+    # @unittest.skip("skip")
+    def test_validate_target_lsn(self):
+        """
+        Check validation to specific LSN
+        """
+        fname = self.id().split('.')[3]
+        backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
+        node = self.make_simple_node(
+            base_dir=os.path.join(module_name, fname, 'node'),
+            set_replication=True,
+            initdb_params=['--data-checksums'])
+
+        self.init_pb(backup_dir)
+        self.add_instance(backup_dir, 'node', node)
+        self.set_archiving(backup_dir, 'node', node)
+        node.slow_start()
+
+        # FULL backup
+        self.backup_node(backup_dir, 'node', node)
+
+        node.safe_psql(
+            "postgres",
+            "create table t_heap as select 1 as id, md5(i::text) as text, "
+            "md5(repeat(i::text,10))::tsvector as tsvector "
+            "from generate_series(0,10000) i")
+
+        node_restored = self.make_simple_node(
+            base_dir=os.path.join(module_name, fname, 'node_restored'))
+        node_restored.cleanup()
+
+        self.restore_node(backup_dir, 'node', node_restored)
+
+        node_restored.append_conf(
+            "postgresql.auto.conf", "port = {0}".format(node_restored.port))
+
+        node_restored.slow_start()
+
+        self.switch_wal_segment(node)
+
+        backup_id = self.backup_node(
+            backup_dir, 'node', node_restored,
+            data_dir=node_restored.data_dir)
+
+        target_lsn = self.show_pb(backup_dir, 'node')[1]['stop-lsn']
+        
+        self.delete_pb(backup_dir, 'node', backup_id)
+
+        self.validate_pb(
+                backup_dir, 'node',
+                options=[
+                    '--recovery-target-timeline=2',
+                    '--recovery-target-lsn={0}'.format(target_lsn)])
+
+    # @unittest.expectedFailure
+    # @unittest.skip("skip")
+    def test_recovery_target_backup_victim(self):
+        """
+        Check that for validation to recovery target
+        probackup chooses valid backup
+        https://github.com/postgrespro/pg_probackup/issues/104
+        """
+        fname = self.id().split('.')[3]
+        backup_dir = os.path.join(self.tmp_path, module_name, fname, 'backup')
+        node = self.make_simple_node(
+            base_dir=os.path.join(module_name, fname, 'node'),
+            set_replication=True,
+            initdb_params=['--data-checksums'])
+
+        self.init_pb(backup_dir)
+        self.add_instance(backup_dir, 'node', node)
+        self.set_archiving(backup_dir, 'node', node)
+        node.slow_start()
+
+        # FULL backup
+        self.backup_node(backup_dir, 'node', node)
+
+        node.safe_psql(
+            "postgres",
+            "create table t_heap as select 1 as id, md5(i::text) as text, "
+            "md5(repeat(i::text,10))::tsvector as tsvector "
+            "from generate_series(0,10000) i")
+
+        target_time = node.safe_psql(
+            "postgres",
+            "select now()").rstrip()
+
+        node.safe_psql(
+            "postgres",
+            "create table t_heap1 as select 1 as id, md5(i::text) as text, "
+            "md5(repeat(i::text,10))::tsvector as tsvector "
+            "from generate_series(0,100) i")
+
+        gdb = self.backup_node(backup_dir, 'node', node, gdb=True)
+
+        gdb.set_breakpoint('pg_stop_backup')
+        gdb.run_until_break()
+        gdb.remove_all_breakpoints()
+        gdb._execute('signal SIGINT')
+        gdb.continue_execution_until_error()
+
+        backup_id = self.show_pb(backup_dir, 'node')[1]['id']
+
+        self.assertEqual(
+            'ERROR',
+            self.show_pb(backup_dir, 'node', backup_id)['status'],
+            'Backup STATUS should be "ERROR"')
+
+        self.validate_pb(
+            backup_dir, 'node',
+            options=['--recovery-target-time={0}'.format(target_time)])
+
+
 # validate empty backup list
 # page from future during validate
 # page from future during backup