Fix do_validate() and do_restore()

delete.c

@@ -260,7 +260,7 @@ pgBackupDeleteFiles(pgBackup *backup)
 	if (backup->status == BACKUP_STATUS_DELETED)
 		return 0;
 
-	time2iso(timestamp, lengthof(timestamp), backup->start_time);
+	time2iso(timestamp, lengthof(timestamp), backup->recovery_time);
 
 	elog(INFO, "delete: %s %s", base36enc(backup->start_time), timestamp);
 

doc/pg_probackup.md

@@ -7,7 +7,7 @@ pg_probackup — backup and recovery manager for PostgreSQL.
 pg_probackup [option...] init
 pg_probackup [option...] backup
 pg_probackup [option...] restore [backup_ID]
-pg_probackup [option...] validate backup_ID
+pg_probackup [option...] validate [backup_ID]
 pg_probackup [option...] show    [backup_ID]
 pg_probackup [option...] delete   backup_ID
 pg_probackup [option...] delwal  [backup_ID]

parsexlog.c

@@ -115,7 +115,8 @@ validate_wal(pgBackup *backup,
 	XLogPageReadPrivate private;
 	TransactionId last_xid = InvalidTransactionId;
 	TimestampTz last_time = 0;
-	char		timestamp[100];
+	char		last_timestamp[100],
+				target_timestamp[100];
 	bool		all_wal = false,
 				got_endpoint = false;
 
@@ -125,6 +126,9 @@ validate_wal(pgBackup *backup,
 	if (xlogreader == NULL)
 		elog(ERROR, "out of memory");
 
+	/* We will check it in the end */
+	xlogfpath[0] = '\0';
+
 	while (true)
 	{
 		bool timestamp_record;
@@ -170,19 +174,31 @@ validate_wal(pgBackup *backup,
 	}
 
 	if (last_time > 0)
-		time2iso(timestamp, lengthof(timestamp), timestamptz_to_time_t(last_time));
+		time2iso(last_timestamp, lengthof(last_timestamp),
+				 timestamptz_to_time_t(last_time));
 	else
-		time2iso(timestamp, lengthof(timestamp), backup->recovery_time);
+		time2iso(last_timestamp, lengthof(last_timestamp),
+				 backup->recovery_time);
 	if (last_xid == InvalidTransactionId)
 		last_xid = backup->recovery_xid;
 
 	/* There are all need WAL records */
 	if (all_wal)
 		elog(INFO, "Backup validation stopped on %s time and xid:" XID_FMT,
-			 timestamp, last_xid);
+			 last_timestamp, last_xid);
 	/* There are not need WAL records */
 	else
 	{
+		if (xlogfpath[0] != 0)
+		{
+			/* XLOG reader couldnt read WAL segment */
+			if (xlogreadfd < 0)
+				elog(WARNING, "WAL segment \"%s\" is absent", xlogfpath);
+			else
+				elog(WARNING, "error was occured during reading WAL segment \"%s\"",
+					 xlogfpath);
+		}
+
 		if (!got_endpoint)
 			elog(ERROR, "there are not enough WAL records to restore from %X/%X to %X/%X",
 				 (uint32) (backup->start_lsn >> 32),
@@ -192,21 +208,21 @@ validate_wal(pgBackup *backup,
 		else
 		{
 			if (target_time > 0)
-				time2iso(timestamp, lengthof(timestamp),
+				time2iso(target_timestamp, lengthof(target_timestamp),
-						 timestamptz_to_time_t(target_time));
+						 target_time);
-
-			if (TransactionIdIsValid(target_xid) && target_time != 0)
-				elog(WARNING, "there are not WAL records to time %s and xid " XID_FMT,
-					 timestamp, target_xid);
-			else if (TransactionIdIsValid(target_xid))
-				elog(WARNING, "there are not WAL records to xid " XID_FMT,
-					 target_xid);
-			else if (target_time != 0)
-				elog(WARNING, "there are not WAL records to time %s ",
-					 timestamp);
 
 			elog(WARNING, "recovery can be done to time %s and xid " XID_FMT,
-				 timestamp, last_xid);
+				 last_timestamp, last_xid);
+
+			if (TransactionIdIsValid(target_xid) && target_time != 0)
+				elog(ERROR, "there are not WAL records to time %s and xid " XID_FMT,
+					 target_timestamp, target_xid);
+			else if (TransactionIdIsValid(target_xid))
+				elog(ERROR, "there are not WAL records to xid " XID_FMT,
+					 target_xid);
+			else if (target_time != 0)
+				elog(ERROR, "there are not WAL records to time %s ",
+					 target_timestamp);
 		}
 	}
 

pg_probackup.c

@@ -210,15 +210,11 @@ main(int argc, char *argv[])
 	else if (pg_strcasecmp(cmd, "show") == 0)
 		return do_show(backup_id);
 	else if (pg_strcasecmp(cmd, "validate") == 0)
-	{
-		if (backup_id == 0)
-			elog(ERROR, "you must specify backup-ID for this command");
 		return do_validate(backup_id,
 						   target_time,
 						   target_xid,
 						   target_inclusive,
 						   target_tli);
-	}
 	else if (pg_strcasecmp(cmd, "delete") == 0)
 		return do_delete(backup_id);
 	else if (pg_strcasecmp(cmd, "delwal") == 0)
@@ -247,7 +243,7 @@ pgut_help(bool details)
 	printf(_("  %s [option...] backup\n"), PROGRAM_NAME);
 	printf(_("  %s [option...] restore [backup-ID]\n"), PROGRAM_NAME);
 	printf(_("  %s [option...] show [backup-ID]\n"), PROGRAM_NAME);
-	printf(_("  %s [option...] validate backup-ID\n"), PROGRAM_NAME);
+	printf(_("  %s [option...] validate [backup-ID]\n"), PROGRAM_NAME);
 	printf(_("  %s [option...] delete backup-ID\n"), PROGRAM_NAME);
 	printf(_("  %s [option...] delwal [backup-ID]\n"), PROGRAM_NAME);
 	printf(_("  %s [option...] retention show|purge\n"), PROGRAM_NAME);

restore.c

@@ -47,8 +47,6 @@ do_restore(time_t backup_id,
 	int			base_index;				/* index of base (full) backup */
 	int			ret;
 	TimeLineID	cur_tli;
-	TimeLineID	backup_tli;
-	TimeLineID	newest_tli;
 	parray	   *backups;
 
 	parray	   *files;
@@ -88,21 +86,14 @@ do_restore(time_t backup_id,
 		elog(ERROR, "cannot process any more.");
 
 	cur_tli = get_current_timeline(true);
-	newest_tli = findNewestTimeLine(1);
-	backup_tli = get_fullbackup_timeline(backups, rt);
-
-	/* determine target timeline */
-	if (target_tli == 0)
-		target_tli = newest_tli != 1 ? newest_tli : backup_tli;
-
 	elog(LOG, "current instance timeline ID = %u", cur_tli);
-	elog(LOG, "newest timeline ID for wal dir = %u", newest_tli);
+
-	elog(LOG, "latest full backup timeline ID = %u", backup_tli);
+	if (target_tli)
+	{
 		elog(LOG, "target timeline ID = %u", target_tli);
-
-
 		/* Read timeline history files from archives */
 		timelines = readTimeLineHistory(target_tli);
+	}
 
 	/* find last full backup which can be used as base backup. */
 	elog(LOG, "searching recent full backup");
@@ -121,9 +112,9 @@ do_restore(time_t backup_id,
 		}
 
 		if (backup_id == base_backup->start_time &&
-			base_backup->status != BACKUP_STATUS_OK
+			base_backup->status != BACKUP_STATUS_OK)
-		)
+			elog(ERROR, "given backup %s is %s", base36enc(backup_id),
-			elog(ERROR, "given backup %s is %s", base36enc(backup_id), status2str(base_backup->status));
+				 status2str(base_backup->status));
 
 		if (dest_backup != NULL &&
 			base_backup->backup_mode == BACKUP_MODE_FULL &&
@@ -137,11 +128,18 @@ do_restore(time_t backup_id,
 			base_backup->status != BACKUP_STATUS_OK)
 			continue;
 
+		if (target_tli)
+		{
 			if (satisfy_timeline(timelines, base_backup) &&
 				satisfy_recovery_target(base_backup, rt) &&
 				(backup_id_found || backup_id == 0))
 				goto base_backup_found;
+		}
 		else
+			if (satisfy_recovery_target(base_backup, rt) &&
+				(backup_id_found || backup_id == 0))
+				goto base_backup_found;
+
 		backup_id_found = false;
 	}
 	/* no full backup found, cannot restore */
@@ -204,9 +202,15 @@ base_backup_found:
 			continue;
 
 		/* is the backup is necessary for restore to target timeline ? */
+		if (target_tli)
+		{
 			if (!satisfy_timeline(timelines, backup) ||
 				!satisfy_recovery_target(backup, rt))
 				continue;
+		}
+		else
+			if (!satisfy_recovery_target(backup, rt))
+				continue;
 
 		if (backup_id != 0)
 			stream_wal = backup->stream;
@@ -217,7 +221,8 @@ base_backup_found:
 
 	/* create recovery.conf */
 	if (!stream_wal || target_time != NULL || target_xid != NULL)
-		create_recovery_conf(backup_id, target_time, target_xid, target_inclusive, target_tli);
+		create_recovery_conf(backup_id, target_time, target_xid,
+							 target_inclusive, base_backup->tli);
 
 	/* release catalog lock */
 	catalog_unlock();

tests/expected/option_help.out

@@ -5,7 +5,7 @@ Usage:
   pg_probackup [option...] backup
   pg_probackup [option...] restore [backup-ID]
   pg_probackup [option...] show [backup-ID]
-  pg_probackup [option...] validate backup-ID
+  pg_probackup [option...] validate [backup-ID]
   pg_probackup [option...] delete backup-ID
   pg_probackup [option...] delwal [backup-ID]
   pg_probackup [option...] retention show|purge

validate.c

@@ -68,7 +68,8 @@ void do_validate_last(void)
 	catalog_unlock();
 }
 
-int do_validate(time_t backup_id,
+int
+do_validate(time_t backup_id,
 			const char *target_time,
 			const char *target_xid,
 			const char *target_inclusive,
@@ -77,8 +78,6 @@ int do_validate(time_t backup_id,
 	int			i;
 	int			base_index;				/* index of base (full) backup */
 	int			last_restored_index;	/* index of last restored database backup */
-	TimeLineID	backup_tli;
-	TimeLineID	newest_tli;
 	parray	   *timelines;
 	parray	   *backups;
 	pgRecoveryTarget *rt = NULL;
@@ -96,14 +95,8 @@ int do_validate(time_t backup_id,
 	if (!backups)
 		elog(ERROR, "cannot process any more.");
 
-	newest_tli = findNewestTimeLine(1);
-	backup_tli = get_fullbackup_timeline(backups, rt);
-
-	/* determine target timeline */
-	if (target_tli == 0)
-		target_tli = newest_tli != 1 ? newest_tli : backup_tli;
-
 	/* Read timeline history files from archives */
+	if (target_tli)
 		timelines = readTimeLineHistory(target_tli);
 
 	/* find last full backup which can be used as base backup. */
@@ -116,25 +109,32 @@ int do_validate(time_t backup_id,
 			continue;
 
 		if (backup_id == base_backup->start_time &&
-			(base_backup->status == BACKUP_STATUS_OK || base_backup->status == BACKUP_STATUS_CORRUPT)
+			(base_backup->status == BACKUP_STATUS_OK ||
-		)
+			 base_backup->status == BACKUP_STATUS_CORRUPT))
 			backup_id_found = true;
 
 		if (backup_id == base_backup->start_time &&
-			(base_backup->status != BACKUP_STATUS_OK && base_backup->status != BACKUP_STATUS_CORRUPT)
+			(base_backup->status != BACKUP_STATUS_OK &&
-		)
+			 base_backup->status != BACKUP_STATUS_CORRUPT))
 			elog(ERROR, "given backup %s is %s", base36enc(backup_id), status2str(base_backup->status));
 
 		if (base_backup->backup_mode < BACKUP_MODE_FULL ||
-			(base_backup->status != BACKUP_STATUS_OK && base_backup->status != BACKUP_STATUS_CORRUPT)
+			(base_backup->status != BACKUP_STATUS_OK &&
-		)
+			 base_backup->status != BACKUP_STATUS_CORRUPT))
 			continue;
 
+		if (target_tli)
+		{
 			if (satisfy_timeline(timelines, base_backup) &&
 				satisfy_recovery_target(base_backup, rt) &&
 				(backup_id_found || backup_id == 0))
 				goto base_backup_found;
+		}
 		else
+			if (satisfy_recovery_target(base_backup, rt) &&
+				(backup_id_found || backup_id == 0))
+				goto base_backup_found;
+
 		backup_id_found = false;
 	}
 	/* no full backup found, cannot restore */
@@ -159,7 +159,8 @@ base_backup_found:
 		pgBackup *backup = (pgBackup *) parray_get(backups, i);
 
 		/* don't use incomplete nor different timeline backup */
-		if ((backup->status != BACKUP_STATUS_OK && backup->status != BACKUP_STATUS_CORRUPT) ||
+		if ((backup->status != BACKUP_STATUS_OK &&
+			 backup->status != BACKUP_STATUS_CORRUPT) ||
 			backup->tli != base_backup->tli)
 			continue;
 
@@ -175,9 +176,15 @@ base_backup_found:
 			continue;
 
 		/* is the backup is necessary for restore to target timeline ? */
+		if (target_tli)
+		{
 			if (!satisfy_timeline(timelines, backup) ||
 				!satisfy_recovery_target(backup, rt))
 				continue;
+		}
+		else
+			if (!satisfy_recovery_target(backup, rt))
+				continue;
 
 		if (backup_id != 0)
 			stream_wal = backup->stream;
@@ -187,12 +194,12 @@ base_backup_found:
 	}
 
 	/* and now we must check WALs */
-	if (!stream_wal)
+	if (!stream_wal || rt->time_specified || rt->xid_specified)
 		validate_wal((pgBackup *) parray_get(backups, last_restored_index),
 					 arclog_path,
 					 rt->recovery_target_time,
 					 rt->recovery_target_xid,
-					 target_tli);
+					 base_backup->tli);
 
 	/* release catalog lock */
 	catalog_unlock();