Simplify code related to HAVE_DATABASE, HAVE_ARCLOG, TOTAL_READ_SIZE

Those macros were mainly used in code paths where they didn't make that much sense, complicating heavily the code. Correct at the same time some code comments.
2025-01-20 11:34:51 +02:00 · 2013-12-16 00:30:49 +09:00 · 2013-12-16 00:30:49 +09:00 · 05ce188607
commit 05ce188607
parent 96cfb3eb13
6 changed files with 94 additions and 67 deletions
--- a/backup.c
+++ b/backup.c
@ -660,28 +660,35 @@ do_backup(pgBackupOption bkupopt)

 	/* PGDATA and BACKUP_MODE are always required */
 	if (pgdata == NULL)
-		elog(ERROR_ARGS, _("required parameter not specified: PGDATA (-D, --pgdata)"));
+		elog(ERROR_ARGS, _("Required parameter not specified: PGDATA "
+						   "(-D, --pgdata)"));

+	/* A backup mode is needed */
 	if (current.backup_mode == BACKUP_MODE_INVALID)
-		elog(ERROR_ARGS, _("required parameter not specified: BACKUP_MODE (-b, --backup-mode)"));
+		elog(ERROR_ARGS, _("Required parameter not specified: BACKUP_MODE "
+						   "(-b, --backup-mode)"));

-	/* ARCLOG_PATH is requried only when backup archive WAL */
-	if (HAVE_ARCLOG(&current) && arclog_path == NULL)
-		elog(ERROR_ARGS, _("required parameter not specified: ARCLOG_PATH (-A, --arclog-path)"));
+	/* ARCLOG_PATH is required for all the modes */
+	if (arclog_path == NULL)
+		elog(ERROR_ARGS,
+			 _("Required parameter not specified: ARCLOG_PATH "
+			   "(-A, --arclog-path)"));

 	/* SRVLOG_PATH is required only when backup serverlog */
 	if (current.with_serverlog && srvlog_path == NULL)
-		elog(ERROR_ARGS, _("required parameter not specified: SRVLOG_PATH (-S, --srvlog-path)"));
+		elog(ERROR_ARGS, _("required parameter not specified: SRVLOG_PATH "
+						   "(-S, --srvlog-path)"));

 #ifndef HAVE_LIBZ
 	if (current.compress_data)
 	{
-		elog(WARNING, _("requested compression not available in this: installation -- archive will be uncompressed"));
+		elog(WARNING, _("requested compression not available in this "
+						"installation. Archive will not be compressed"));
 		current.compress_data = false;
 	}
 #endif

-	/* confirm data block size and xlog block size are compatible */
+	/* Confirm data block size and xlog block size are compatible */
 	check_server_version();

 	/* setup cleanup callback function */
@ -758,23 +765,37 @@ do_backup(pgBackupOption bkupopt)
 	if (!check)
 		pgBackupWriteIni(&current);

+	/* Calculate the total data read */
 	if (verbose)
 	{
-		if (TOTAL_READ_SIZE(&current) == 0)
+		int64 total_read = 0;
+
+		/* WAL archives */
+		total_read += current.read_arclog_bytes;
+
+		/* Database data */
+		if (current.backup_mode == BACKUP_MODE_FULL ||
+			current.backup_mode == BACKUP_MODE_INCREMENTAL)
+			total_read += current.read_arclog_bytes;
+
+		/* Server logs */
+		if (current.with_serverlog)
+			total_read += current.read_srvlog_bytes;
+
+		if (total_read == 0)
 			printf(_("nothing to backup\n"));
 		else
 			printf(_("all backup completed(read: " INT64_FORMAT " write: "
 				INT64_FORMAT ")\n"),
-				TOTAL_READ_SIZE(&current), current.write_bytes);
+				total_read, current.write_bytes);
 		printf(_("========================================\n"));
 	}

 	/*
 	 * Delete old files (archived WAL and serverlog) after update of status.
 	 */
-	if (HAVE_ARCLOG(&current))
-		delete_old_files(arclog_path, files_arclog, keep_arclog_files,
-			keep_arclog_days, true);
+	delete_old_files(arclog_path, files_arclog, keep_arclog_files,
+		keep_arclog_days, true);
 	if (current.with_serverlog)
 		delete_old_files(srvlog_path, files_srvlog, keep_srvlog_files,
 			keep_srvlog_days, false);
--- a/catalog.c
+++ b/catalog.c
@ -254,8 +254,13 @@ catalog_get_last_data_backup(parray *backup_list)
 	{
 		backup = (pgBackup *) parray_get(backup_list, i);

-		/* we need completed database backup */
-		if (backup->status == BACKUP_STATUS_OK && HAVE_DATABASE(backup))
+		/*
+		 * We need completed database backup in the case of a full or
+		 * incremental backup.
+		 */
+		if (backup->status == BACKUP_STATUS_OK &&
+			(backup->backup_mode == BACKUP_MODE_INCREMENTAL ||
+			 backup->backup_mode == BACKUP_MODE_FULL))
 			return backup;
 	}

@ -277,7 +282,7 @@ catalog_get_last_arclog_backup(parray *backup_list)
 		backup = (pgBackup *) parray_get(backup_list, i);

 		/* we need completed archived WAL backup */
-		if (backup->status == BACKUP_STATUS_OK && HAVE_ARCLOG(backup))
+		if (backup->status == BACKUP_STATUS_OK)
 			return backup;
 	}

--- a/pg_rman.h
+++ b/pg_rman.h
@ -176,13 +176,6 @@ typedef struct pgBackupOption
 #define KEEP_INFINITE			(INT_MAX)
 #define BYTES_INVALID			(-1)

-#define HAVE_DATABASE(backup)	((backup)->backup_mode >= BACKUP_MODE_INCREMENTAL)
-#define HAVE_ARCLOG(backup)		((backup)->backup_mode >= BACKUP_MODE_ARCHIVE)
-#define TOTAL_READ_SIZE(backup)	\
-	((HAVE_DATABASE((backup)) ? (backup)->read_data_bytes : 0) + \
-	 (HAVE_ARCLOG((backup)) ? (backup)->read_arclog_bytes : 0) + \
-	 ((backup)->with_serverlog ? (backup)->read_srvlog_bytes : 0))
-
 typedef struct pgTimeLine
 {
 	TimeLineID	tli;
@ -265,7 +258,9 @@ extern char *slurpFile(const char *datadir,

 /* in validate.c */
 extern int do_validate(pgBackupRange *range);
-extern void pgBackupValidate(pgBackup *backup, bool size_only, bool for_get_timeline, bool with_database);
+extern void pgBackupValidate(pgBackup *backup,
+							 bool size_only,
+							 bool for_get_timeline);

 /* in catalog.c */
 extern pgBackup *catalog_get_backup(time_t timestamp);
--- a/restore.c
+++ b/restore.c
@ -164,14 +164,14 @@ do_restore(const char *target_time,

 #ifndef HAVE_LIBZ
 		/* Make sure we won't need decompression we haven't got */
-		if (base_backup->compress_data &&
-			(HAVE_DATABASE(base_backup) || HAVE_ARCLOG(base_backup)))
-		{
+		if (base_backup->compress_data)
 			elog(ERROR_SYSTEM,
-				_("can't restore from compressed backup (compression not supported in this installation)"));
-		}
+				_("can't restore from compressed backup (compression "
+				  "not supported in this installation)"));
+
 #endif
-		if (satisfy_timeline(timelines, base_backup) && satisfy_recovery_target(base_backup, rt))
+		if (satisfy_timeline(timelines, base_backup) &&
+			satisfy_recovery_target(base_backup, rt))
 			goto base_backup_found;
 	}
 	/* no full backup found, can't restore */
@ -238,9 +238,6 @@ base_backup_found:
 		if (backup->status != BACKUP_STATUS_OK)
 			continue;

-		if (!HAVE_ARCLOG(backup))
-			continue;
-
 		/* care timeline junction */
 		if (!satisfy_timeline(timelines, backup))
 			continue;
@ -333,7 +330,7 @@ restore_database(pgBackup *backup)
 	 * Validate backup files with its size, because load of CRC calculation is
 	 * not right.
 	 */
-	pgBackupValidate(backup, true, false, true);
+	pgBackupValidate(backup, true, false);

 	/* make direcotries and symbolic links */
 	pgBackupGetPath(backup, path, lengthof(path), MKDIRS_SH_FILE);
@ -497,7 +494,7 @@ restore_archive_logs(pgBackup *backup, bool is_hard_copy)
 	 * Validate backup files with its size, because load of CRC calculation is
 	 * not light.
 	 */
-	pgBackupValidate(backup, true, false, false);
+	pgBackupValidate(backup, true, false);

 	pgBackupGetPath(backup, list_path, lengthof(list_path), ARCLOG_FILE_LIST);
 	pgBackupGetPath(backup, base_path, lengthof(list_path), ARCLOG_DIR);
@ -850,13 +847,13 @@ readTimeLineHistory(TimeLineID targetTLI)
 static bool
 satisfy_recovery_target(const pgBackup *backup, const pgRecoveryTarget *rt)
 {
-	if(rt->xid_specified)
-		return backup->recovery_xid <= rt->recovery_target_xid);
+	if (rt->xid_specified)
+		return backup->recovery_xid <= rt->recovery_target_xid;

 	if (rt->time_specified)
-		return backup->recovery_time <= rt->recovery_target_time);
-	else
-		return true;
+		return backup->recovery_time <= rt->recovery_target_time;
+
+	return true;
 }

 static bool
@ -948,7 +945,7 @@ get_fullbackup_timeline(parray *backups, const pgRecoveryTarget *rt)
 			 * calculation is not right.
 			 */
 			if (base_backup->status == BACKUP_STATUS_DONE)
-				pgBackupValidate(base_backup, true, true, false);
+				pgBackupValidate(base_backup, true, true);

 			if(!satisfy_recovery_target(base_backup, rt))
 				continue;
--- a/show.c
+++ b/show.c
@ -189,15 +189,14 @@ show_backup_list(FILE *out, parray *backup_list, bool show_all)
 			snprintf(duration, lengthof(duration), "%lum",
 				(backup->end_time - backup->start_time) / 60);
 		/* "Full" is only for full backup */
-		if (backup->backup_mode >= BACKUP_MODE_FULL)
+		if (backup->backup_mode == BACKUP_MODE_FULL)
 			pretty_size(backup->total_data_bytes, total_data_bytes_str,
 					lengthof(total_data_bytes_str));
-		else if (backup->backup_mode >= BACKUP_MODE_INCREMENTAL)
+		if (backup->backup_mode == BACKUP_MODE_INCREMENTAL)
 			pretty_size(backup->read_data_bytes, read_data_bytes_str,
 					lengthof(read_data_bytes_str));
-		if (HAVE_ARCLOG(backup))
-			pretty_size(backup->read_arclog_bytes, read_arclog_bytes_str,
-					lengthof(read_arclog_bytes_str));
+		pretty_size(backup->read_arclog_bytes, read_arclog_bytes_str,
+				lengthof(read_arclog_bytes_str));
 		if (backup->with_serverlog)
 			pretty_size(backup->read_srvlog_bytes, read_srvlog_bytes_str,
 					lengthof(read_srvlog_bytes_str));
--- a/validate.c
+++ b/validate.c
@ -51,7 +51,7 @@ do_validate(pgBackupRange *range)
 			continue;

 		/* validate with CRC value and update status to OK */
-		pgBackupValidate(backup, false, false, (HAVE_DATABASE(backup)));
+		pgBackupValidate(backup, false, false);
 	}

 	/* cleanup */
@ -67,7 +67,9 @@ do_validate(pgBackupRange *range)
 * Validate each files in the backup with its size.
 */
 void
-pgBackupValidate(pgBackup *backup, bool size_only, bool for_get_timeline, bool with_database)
+pgBackupValidate(pgBackup *backup,
+				 bool size_only,
+				 bool for_get_timeline)
 {
 	char	timestamp[100];
 	char	base_path[MAXPGPATH];
@ -76,19 +78,27 @@ pgBackupValidate(pgBackup *backup, bool size_only, bool for_get_timeline, bool w
 	bool	corrupted = false;

 	time2iso(timestamp, lengthof(timestamp), backup->start_time);
-	if(!for_get_timeline){
-		if (with_database)
-			elog(INFO, "validate: %s backup and archive log files by %s", timestamp, (size_only ? "SIZE" : "CRC"));
-		else{
+	if (!for_get_timeline)
+	{
+		if (backup->backup_mode == BACKUP_MODE_FULL ||
+			backup->backup_mode == BACKUP_MODE_INCREMENTAL)
+			elog(INFO, "validate: %s backup and archive log files by %s",
+				 timestamp, (size_only ? "SIZE" : "CRC"));
+		else
+		{
 			if (backup->backup_mode == BACKUP_MODE_ARCHIVE)
-				elog(INFO, "validate: %s archive log files by %s", timestamp, (size_only ? "SIZE" : "CRC"));
+				elog(INFO, "validate: %s archive log files by %s",
+					 timestamp, (size_only ? "SIZE" : "CRC"));
 			else if (backup->with_serverlog)
-				elog(INFO, "validate: %s server log files by %s", timestamp, (size_only ? "SIZE" : "CRC"));
+				elog(INFO, "validate: %s server log files by %s",
+					 timestamp, (size_only ? "SIZE" : "CRC"));
 		}
 	}

-	if(!check){
-		if (HAVE_DATABASE(backup))
+	if (!check)
+	{
+		if (backup->backup_mode == BACKUP_MODE_FULL ||
+			backup->backup_mode == BACKUP_MODE_INCREMENTAL)
 		{
 			elog(LOG, "database files...");
 			pgBackupGetPath(backup, base_path, lengthof(base_path), DATABASE_DIR);
@ -100,17 +110,17 @@ pgBackupValidate(pgBackup *backup, bool size_only, bool for_get_timeline, bool w
 			parray_walk(files, pgFileFree);
 			parray_free(files);
 		}
-		if (HAVE_ARCLOG(backup))
-		{
-			elog(LOG, "archive WAL files...");
-			pgBackupGetPath(backup, base_path, lengthof(base_path), ARCLOG_DIR);
-			pgBackupGetPath(backup, path, lengthof(path), ARCLOG_FILE_LIST);
-			files = dir_read_file_list(base_path, path);
-			if (!pgBackupValidateFiles(files, base_path, size_only))
-				corrupted = true;
-			parray_walk(files, pgFileFree);
-			parray_free(files);
-		}
+
+		/* WAL archives are present for all modes */
+		elog(LOG, "archive WAL files...");
+		pgBackupGetPath(backup, base_path, lengthof(base_path), ARCLOG_DIR);
+		pgBackupGetPath(backup, path, lengthof(path), ARCLOG_FILE_LIST);
+		files = dir_read_file_list(base_path, path);
+		if (!pgBackupValidateFiles(files, base_path, size_only))
+			corrupted = true;
+		parray_walk(files, pgFileFree);
+		parray_free(files);
+
 		if (backup->with_serverlog)
 		{
 			elog(LOG, "server log files...");