Improve comments. Add pgBackupGetBackupMode()

backup.c

@@ -26,7 +26,6 @@
 #include "streamutil.h"
 #include "receivelog.h"
 
-static const char *backupModes[] = {"", "PAGE", "PTRACK", "FULL"};
 static int	standby_message_timeout = 10 * 1000;	/* 10 sec = default */
 static XLogRecPtr stop_backup_lsn = InvalidXLogRecPtr;
 const char *progname = "pg_probackup";
@@ -242,7 +241,14 @@ do_backup_database(parray *backup_list)
 		make_pagemap_from_ptrack(backup_files_list);
 	}
 
-	/* Sort pathname ascending TODO What for?*/
+	/*
+	 * Sort pathname ascending. It is necessary to create intermediate
+	 * directories sequentially.
+	 *
+	 * For example:
+	 * 1 - create 'base'
+	 * 2 - create 'base/1'
+	 */
 	parray_qsort(backup_files_list, pgFileComparePath);
 
 	/*
@@ -416,7 +422,7 @@ do_backup(void)
 	check_system_identifiers();
 
 	elog(LOG, "Backup start. backup-mode = %s+%s",
-		backupModes[current.backup_mode], current.stream?"STREAM":"ARCHIVE");
+		pgBackupGetBackupMode(&current), current.stream?"STREAM":"ARCHIVE");
 
 	/* Start backup. Update backup status. */
 	current.status = BACKUP_STATUS_RUNNING;
@@ -1495,7 +1501,10 @@ make_pagemap_from_ptrack(parray *files)
 }
 
 
-/* TODO Add comment */
+/*
+ * Stop WAL streaming if current 'xlogpos' exceeds 'stop_backup_lsn', which is
+ * set by pg_stop_backup().
+ */
 static bool
 stop_streaming(XLogRecPtr xlogpos, uint32 timeline, bool segment_finished)
 {

catalog.c

@@ -1,6 +1,6 @@
 /*-------------------------------------------------------------------------
  *
- * catalog.c: backup catalog opration
+ * catalog.c: backup catalog operation
  *
  * Portions Copyright (c) 2009-2011, NIPPON TELEGRAPH AND TELEPHONE CORPORATION
  * Portions Copyright (c) 2015-2017, Postgres Professional
@@ -222,6 +222,15 @@ read_backup(time_t timestamp)
 	return readBackupControlFile(conf_path);
 }
 
+/*
+ * Get backup_mode in string representation.
+ */
+const char *
+pgBackupGetBackupMode(pgBackup *backup)
+{
+	return backupModes[backup->backup_mode];
+}
+
 static bool
 IsDir(const char *dirpath, const char *entry)
 {
@@ -370,7 +379,6 @@ pgBackupCreateDir(pgBackup *backup)
 
 /*
  * Write information about backup.in to stream "out".
- * TODO improve comments
  */
 void
 pgBackupWriteControl(FILE *out, pgBackup *backup)
@@ -378,7 +386,7 @@ pgBackupWriteControl(FILE *out, pgBackup *backup)
 	char		timestamp[20];
 
 	fprintf(out, "#Configuration\n");
-	fprintf(out, "backup-mode = %s\n", backupModes[backup->backup_mode]);
+	fprintf(out, "backup-mode = %s\n", pgBackupGetBackupMode(backup));
 	fprintf(out, "stream = %s\n", backup->stream?"true":"false");
 	
 	fprintf(out, "\n#Compatibility\n");
@@ -388,9 +396,11 @@ pgBackupWriteControl(FILE *out, pgBackup *backup)
 
 	fprintf(out, "\n#Result backup info\n");
 	fprintf(out, "timelineid = %d\n", backup->tli);
+	/* LSN returned by pg_start_backup */
 	fprintf(out, "start-lsn = %x/%08x\n",
 			(uint32) (backup->start_lsn >> 32),
 			(uint32) backup->start_lsn);
+	/* LSN returned by pg_stop_backup */
 	fprintf(out, "stop-lsn = %x/%08x\n",
 			(uint32) (backup->stop_lsn >> 32),
 			(uint32) backup->stop_lsn);
@@ -409,11 +419,16 @@ pgBackupWriteControl(FILE *out, pgBackup *backup)
 		fprintf(out, "recovery-time = '%s'\n", timestamp);
 	}
 
-	/* TODO rename the field? */
+	/*
+	 * Size of PGDATA directory. The size does not include size of related
+	 * WAL segments in archive 'wal' directory.
+	 */
 	if (backup->data_bytes != BYTES_INVALID)
 		fprintf(out, "data-bytes = " INT64_FORMAT "\n", backup->data_bytes);
 
 	fprintf(out, "status = %s\n", status2str(backup->status));
+
+	/* 'parent_backup' is set if it is incremental backup */
 	if (backup->parent_backup != 0)
 	{
 		char	   *parent_backup = base36enc(backup->parent_backup);

data.c

@@ -54,9 +54,9 @@ parse_page(const DataPage *page, XLogRecPtr *lsn)
  */
 static void
 backup_data_page(pgFile *file, XLogRecPtr prev_backup_start_lsn,
-				BlockNumber blknum, BlockNumber nblocks,
-				FILE *in, FILE *out,
-				pg_crc32 *crc)
+				 BlockNumber blknum, BlockNumber nblocks,
+				 FILE *in, FILE *out,
+				 pg_crc32 *crc)
 {
 	BackupPageHeader	header;
 	off_t				offset;
@@ -77,7 +77,7 @@ backup_data_page(pgFile *file, XLogRecPtr prev_backup_start_lsn,
 		{
 			/* TODO Should we check specific error code here? */
 			if (verbose)
-				elog(WARNING, "File: %s, could not seek to block %u."
+				elog(WARNING, "File: %s, could not seek to block %u. "
 					 "Probably the file was truncated after backup start.",
 					 file->path, blknum);
 			return;
@@ -98,7 +98,7 @@ backup_data_page(pgFile *file, XLogRecPtr prev_backup_start_lsn,
 		 * If after several attempts page header is still invalid, throw an error.
 		 * The same idea is applied to checksum verification.
 		 */
-		
+
 		/*
 		 * TODO Should we show a hint about possible false positives suggesting to
 		 * decrease concurrent load? Or we can just copy this page and rely on

dir.c

@@ -339,7 +339,6 @@ dir_list_file(parray *files, const char *root, bool exclude, bool omit_symlink,
 
 /*
  * TODO Add comment, review
- * TODO if met file of unusual format throw a WARNING and don't add to list.
  */
 static void
 dir_list_file_internal(parray *files, const char *root, bool exclude,
@@ -358,9 +357,13 @@ dir_list_file_internal(parray *files, const char *root, bool exclude,
 		return;
 	}
 
+	/*
+	 * Add to files list only files, links and directories. Skip sockets and
+	 * other unexpected file formats.
+	 */
 	if (!S_ISDIR(file->mode) && !S_ISLNK(file->mode) &&	!S_ISREG(file->mode))
 	{
-		elog(WARNING, "Skip file \"%s\": Unexpected file format", file->path);
+		elog(WARNING, "Skip file \"%s\": unexpected file format", file->path);
 		return;
 	}
 

pg_probackup.h

@@ -301,6 +301,7 @@ extern void pgBackupValidate(pgBackup* backup);
 
 /* in catalog.c */
 extern pgBackup *read_backup(time_t timestamp);
+extern const char *pgBackupGetBackupMode(pgBackup *backup);
 
 extern parray *catalog_get_backup_list(time_t requested_backup_id);
 extern pgBackup *catalog_get_last_data_backup(parray *backup_list,

restore.c

@@ -338,8 +338,14 @@ restore_backup(pgBackup *backup)
 	parray_walk(files, pgFileFree);
 	parray_free(files);
 
-	/* TODO print backup name */
-	elog(LOG, "restore backup completed");
+	if (verbose)
+	{
+		char	   *backup_id;
+
+		backup_id = base36enc(backup->start_time);
+		elog(LOG, "restore %s backup completed", backup_id);
+		free(backup_id);
+	}
 }
 
 /*

show.c

@@ -11,7 +11,6 @@
 #include "pg_probackup.h"
 #include <time.h>
 
-static const char *backupModes[] = {"", "PAGE", "PTRACK", "FULL"};
 static void show_backup_list(FILE *out, parray *backup_list);
 static void show_backup_detail(FILE *out, pgBackup *backup);
 
@@ -201,7 +200,7 @@ show_backup_list(FILE *out, parray *backup_list)
 		fprintf(out, "%-8s %-19s  %s%-9s %2d / %d               %5s  %6s  %2X/%08X  %2X/%08X  %-8s\n",
 				backup_id,
 				timestamp,
-				backupModes[backup->backup_mode],
+				pgBackupGetBackupMode(backup),
 				backup->stream ? "+STREAM": "+ARCHIVE",
 				backup->tli,
 				parent_tli,