Allow only one keep parameter to trigger backup deletion

Michael Paquier · Michael Paquier · commit fcaae1a34bf8 · 2014-01-10T02:37:33.000+09:00
Having files satisfying both conditions seems somewhat awkward, as users
would usually choose either the number of generations to keep or the
amount of days to keep the files. Hence deletion of a backup is bypassed
only when both parameters are set to infinite.

At the same time correct some typos and incorrections in the deletion
code.
diff --git a/delete.c b/delete.c
@@ -25,23 +25,22 @@ do_delete(pgBackupRange *range, bool force)
 	if (!pgBackupRangeIsValid(range))
 		elog(ERROR_ARGS, _("required delete range option not specified: delete DATE"));
 
-	/* get exclusive lock of backup catalog */
+	/* Lock backup catalog */
 	ret = catalog_lock();
 	if (ret == -1)
 		elog(ERROR_SYSTEM, _("can't lock backup catalog."));
 	else if (ret == 1)
 		elog(ERROR_ALREADY_RUNNING,
 			_("another pg_rman is running, stop delete."));
 
-	/* get list of backups. */
+	/* Get complete list of backup */
 	backup_list = catalog_get_backup_list(NULL);
-	if(!backup_list){
-		elog(ERROR_SYSTEM, _("can't process any more."));
-	}
+	if (!backup_list)
+		elog(ERROR_SYSTEM, _("No backup list found, can't process any more."));
 
+	/* Find backups to be deleted */
 	do_delete = false;
 	force_delete = false;
-	/* find delete target backup. */
 	for (i = 0; i < parray_num(backup_list); i++)
 	{
 		pgBackup *backup = (pgBackup *)parray_get(backup_list, i);
@@ -60,7 +59,7 @@ do_delete(pgBackupRange *range, bool force)
 			continue;
 		}
 
-		/* find latest full backup. */
+		/* Found the latest full backup */
 		if (backup->backup_mode >= BACKUP_MODE_FULL &&
 			backup->status == BACKUP_STATUS_OK &&
 			backup->start_time <= range->begin)
@@ -111,45 +110,45 @@ pgBackupDelete(int keep_generations, int keep_days)
 			generations_str, days_str);
 	}
 
-	/* delete files which satisfy both condition */
-	if (keep_generations == KEEP_INFINITE || keep_days == KEEP_INFINITE)
+	/* Leave if an infinite generation of backups is kept */
+	if (keep_generations == KEEP_INFINITE && keep_days == KEEP_INFINITE)
 	{
 		elog(LOG, "%s() infinite", __FUNCTION__);
 		return;
 	}
 
-	/* get list of backups. */
+	/* Get a complete list of backups. */
 	backup_list = catalog_get_backup_list(NULL);
 
+	/* Find target backups to be deleted */
 	backup_num = 0;
-	/* find delete target backup. */
 	for (i = 0; i < parray_num(backup_list); i++)
 	{
 		pgBackup *backup = (pgBackup *)parray_get(backup_list, i);
 
 		elog(LOG, "%s() %lu", __FUNCTION__, backup->start_time);
+
 		/*
-		 * when validate full backup was found, we can delete the backup
-		 * that is older than it
+		 * When a validate full backup was found, we can delete the
+		 * backup that is older than it.
 		 */
 		if (backup->backup_mode >= BACKUP_MODE_FULL &&
 			backup->status == BACKUP_STATUS_OK)
 			backup_num++;
 
-		/* do not include the latest full backup in a count. */
-//		if (backup_num - 1 <= keep_generations)
+		/* Evaluate if this backup is eligible for removal */
 		if (backup_num <= keep_generations)
 		{
+			/* Do not include the latest full backup in this count */
 			elog(LOG, "%s() backup are only %d", __FUNCTION__, backup_num);
 			continue;
 		}
-
-		/*
-		 * If the start time of the backup is older than the threshold and
-		 * there are enough generations of full backups, delete the backup.
-		 */
-		if (backup->start_time >= days_threshold)
+		else if (backup->start_time >= days_threshold)
 		{
+			/*
+			 * If the start time of the backup is older than the threshold and
+			 * there are enough generations of full backups, delete the backup.
+			 */
 			elog(LOG, "%s() %lu is not older than %lu", __FUNCTION__,
 				backup->start_time, days_threshold);
 			continue;
diff --git a/pg_rman.txt b/pg_rman.txt
@@ -257,7 +257,7 @@ absolute paths; relative paths are not allowed.
     Specify how long backuped data files will be kept.
     --keep-data-generations means number of backup generations.
     --keep-data-days means days to be kept.
-    Only files exceeded both settings are deleted. 
+    Only files exceeded one of those settings are deleted.
 
 --keep-arclog-files / --keep-arclog-days
     Specify how long backuped archive WAL files will be kept.
