[Issue #120] minor improvements

Author: gsmolk

src/catalog.c

@@ -2447,3 +2447,23 @@ get_backup_index_number(parray *backup_list, pgBackup *backup)
 	elog(WARNING, "Failed to find backup %s", base36enc(backup->start_time));
 	return -1;
 }
+
+/* On backup_list lookup children of target_backup and append them to append_list */
+void
+append_children(parray *backup_list, pgBackup *target_backup, parray *append_list)
+{
+	int i;
+
+	for (i = 0; i < parray_num(backup_list); i++)
+	{
+		pgBackup *backup = (pgBackup *) parray_get(backup_list, i);
+
+		/* check if backup is descendant of target backup */
+		if (is_parent(target_backup->start_time, backup, false))
+		{
+			/* if backup is already in the list, then skip it */
+			if (!parray_contains(append_list, backup))
+				parray_append(append_list, backup);
+		}
+	}
+}

src/delete.c

@@ -1021,48 +1021,16 @@ do_delete_instance(void)
 	return 0;
 }
 
-/* checks that parray contains element */
-bool parray_contains(parray *array, void *elem)
-{
-	int i;
-	for (i = 0; i < parray_num(array); i++)
-	{
-		if (parray_get(array, i) == elem) return true;
-	}
-	return false;
-}
-
-
-void append_childs(parray *backup_list, pgBackup  *target_backup, parray *delete_list)
-{
-	int i;
-	pgBackup   *backup;
-	for (i = 0; i < parray_num(backup_list); i++)
-	{
-		backup = (pgBackup *)parray_get(backup_list, i);
-		if (backup == target_backup) continue;
-		/* check if backup is descendant of delete target */
-		if (is_parent(target_backup->start_time, backup, false))
-		{
-			if (!parray_contains(delete_list, backup))
-				parray_append(delete_list, backup);
-			/* recursive call */
-			append_childs(backup_list, backup, delete_list);
-		}
-	}
-
-}
-
 /* Delete all backups of given status in instance */
 void
 do_delete_status(InstanceConfig *instance_config, const char *status)
 {
-	parray		*backup_list, *delete_list;;
-	int 		i;
-	const char  *pretty_status;
-	int 		n_deleted = 0, n_found = 0;
-	size_t		size_to_delete = 0;
-	char		size_to_delete_pretty[20];
+	int         i;
+	parray     *backup_list, *delete_list;
+	const char *pretty_status;
+	int         n_deleted = 0, n_found = 0;
+	size_t      size_to_delete = 0;
+	char        size_to_delete_pretty[20];
 	pgBackup   *backup;
 
 	BackupStatus status_for_delete = str2status(status);
@@ -1086,44 +1054,49 @@ do_delete_status(InstanceConfig *instance_config, const char *status)
 		return;
 	}
 
-	elog(INFO, "Deleting all backups with status '%s'", pretty_status);
+	if (dry_run)
+		elog(INFO, "Deleting all backups with status '%s' in dry run mode", pretty_status);
+	else
+		elog(INFO, "Deleting all backups with status '%s'", pretty_status);
 
-	/* Selects backups for deleting to delete_list array.  Will delete all backups with specified status and childs*/
+	/* Selects backups with specified status and their children into delete_list array. */
 	for (i = 0; i < parray_num(backup_list); i++)
 	{
 		backup = (pgBackup *) parray_get(backup_list, i);
 
 		if (backup->status == status_for_delete)
 		{
 			n_found++;
-			if (parray_contains(delete_list, backup)) continue;
+
+			/* incremental backup can be already in delete_list due to append_children() */
+			if (parray_contains(delete_list, backup))
+				continue;
 			parray_append(delete_list, backup);
-			append_childs(backup_list, backup, delete_list);
+
+			append_children(backup_list, backup, delete_list);
 		}
 	}
+
+	parray_qsort(delete_list, pgBackupCompareIdDesc);
+
 	/* delete and calculate free size from delete_list */
 	for (i = 0; i < parray_num(delete_list); i++)
 	{
 		backup = (pgBackup *)parray_get(delete_list, i);
-		elog(dry_run ? INFO : LOG, "Backup %s with status %s %s be deleted",
+
+		elog(INFO, "Backup %s with status %s %s be deleted",
 			base36enc(backup->start_time), status2str(backup->status), dry_run ? "can" : "will");
 
 		size_to_delete += backup->data_bytes;
 		if (backup->stream)
 			size_to_delete += backup->wal_bytes;
 
-
 		if (!dry_run && lock_backup(backup))
-		{
-			if (interrupted)
-				elog(ERROR, "interrupted during delete backup");
-
 			delete_backup_files(backup);
-			n_deleted++;
-
-		}
 
+		n_deleted++;
 	}
+
 	/* Inform about data size to free */
 	if (size_to_delete >= 0)
 	{
@@ -1133,7 +1106,7 @@ do_delete_status(InstanceConfig *instance_config, const char *status)
 	}
 
 	/* delete selected backups */
-	if (!dry_run)
+	if (!dry_run && n_deleted > 0)
 		elog(INFO, "Successfully deleted %i %s from instance '%s'",
 			n_deleted, n_deleted == 1 ? "backup" : "backups",
 			instance_config->name);
@@ -1143,9 +1116,8 @@ do_delete_status(InstanceConfig *instance_config, const char *status)
 		elog(WARNING, "Instance '%s' has no backups with status '%s'",
 			instance_config->name, pretty_status);
 
-	/* Clean WAL segments */
-	if (delete_wal)
-		do_retention_wal(dry_run);
+	// we don`t do WAL purge here, because it is impossible to correctly handle
+	// dry-run case.
 
 	/* Cleanup */
 	parray_free(delete_list);

src/help.c

@@ -638,8 +638,8 @@ help_delete(void)
 	printf(_("      --wal-depth=wal-depth        number of latest valid backups per timeline that must\n"));
 	printf(_("                                   retain the ability to perform PITR; 0 disables; (default: 0)\n"));
 	printf(_("      --dry-run                    perform a trial run without any changes\n"));
-	printf(_("      --status=backups_status      delete all backups with specific status\n"));
-	
+	printf(_("      --status=backup_status       delete all backups with specified status\n"));
+
 	printf(_("\n  Logging options:\n"));
 	printf(_("      --log-level-console=log-level-console\n"));
 	printf(_("                                   level for console logging (default: info)\n"));

src/pg_probackup.h

@@ -808,8 +808,8 @@ extern int scan_parent_chain(pgBackup *current_backup, pgBackup **result_backup)
 
 extern bool is_parent(time_t parent_backup_time, pgBackup *child_backup, bool inclusive);
 extern bool is_prolific(parray *backup_list, pgBackup *target_backup);
-extern bool in_backup_list(parray *backup_list, pgBackup *target_backup);
 extern int get_backup_index_number(parray *backup_list, pgBackup *backup);
+extern void append_children(parray *backup_list, pgBackup *target_backup, parray *append_list);
 extern bool launch_agent(void);
 extern void launch_ssh(char* argv[]);
 extern void wait_ssh(void);

src/utils/parray.c

@@ -197,3 +197,16 @@ parray_bsearch(parray *array, const void *key, int(*compare)(const void *, const
 {
 	return bsearch(&key, array->data, array->used, sizeof(void *), compare);
 }
+
+/* checks that parray contains element */
+bool parray_contains(parray *array, void *elem)
+{
+	int i;
+
+	for (i = 0; i < parray_num(array); i++)
+	{
+		if (parray_get(array, i) == elem)
+			return true;
+	}
+	return false;
+}

src/utils/parray.h

@@ -30,6 +30,7 @@ extern size_t parray_num(const parray *array);
 extern void parray_qsort(parray *array, int(*compare)(const void *, const void *));
 extern void *parray_bsearch(parray *array, const void *key, int(*compare)(const void *, const void *));
 extern void parray_walk(parray *array, void (*action)(void *));
+extern bool parray_contains(parray *array, void *elem);
 
 #endif /* PARRAY_H */