[Issue #169] minor improvements

Author: gsmolk

src/data.c

@@ -237,7 +237,7 @@ read_page_from_file(pgFile *file, BlockNumber blknum,
 	{
 		int i;
 		/* Check if the page is zeroed. */
-		for(i = 0; i < BLCKSZ && page[i] == 0; i++);
+		for (i = 0; i < BLCKSZ && page[i] == 0; i++);
 
 		/* Page is zeroed. No need to check header and checksum. */
 		if (i == BLCKSZ)
@@ -271,9 +271,10 @@ read_page_from_file(pgFile *file, BlockNumber blknum,
  * should be a pointer to allocated BLCKSZ of bytes.
  *
  * Prints appropriate warnings/errors/etc into log.
- * Returns 0 if page was successfully retrieved
- *         SkipCurrentPage(-3) if we need to skip this page
+ * Returns:
+ *                 PageIsOk(0) if page was successfully retrieved
  *         PageIsTruncated(-2) if the page was truncated
+ *         SkipCurrentPage(-3) if we need to skip this page
  *         PageIsCorrupted(-4) if the page check mismatch
  */
 static int32
@@ -312,9 +313,8 @@ prepare_page(ConnectionArgs *conn_arg,
 			switch (result)
 			{
 				case 2:
-					page_is_valid = true;
 					elog(VERBOSE, "File: \"%s\" blknum %u, empty page", from_fullpath, blknum);
-					break;
+					return PageIsOk;
 
 				case 1:
 					page_is_valid = true;
@@ -393,6 +393,12 @@ prepare_page(ConnectionArgs *conn_arg,
 		}
 	}
 
+	/* Get page via ptrack interface from PostgreSQL shared buffer.
+	 * We do this in following cases:
+	 * 1. PTRACK backup of 1.x versions
+	 * 2. During backup, regardless of backup mode, of PostgreSQL instance
+	 *    with ptrack support we encountered invalid page.
+	 */
 	if ((backup_mode == BACKUP_MODE_DIFF_PTRACK
 		&& (ptrack_version_num >= 15 && ptrack_version_num < 20))
 			|| !page_is_valid)
@@ -434,7 +440,6 @@ prepare_page(ConnectionArgs *conn_arg,
 			!parse_page(page, &page_lsn))
 				elog(ERROR, "Cannot parse page after pg_ptrack_get_block. "
 								"Possible risk of a memory corruption");
-
 	}
 
 	if (page_is_truncated)
@@ -1260,22 +1265,18 @@ create_empty_file(fio_location from_location, const char *to_root,
 	/* open file for write  */
 	join_path_components(to_path, to_root, file->rel_path);
 	out = fio_fopen(to_path, PG_BINARY_W, to_location);
+
 	if (out == NULL)
-	{
-		elog(ERROR, "cannot open destination file \"%s\": %s",
+		elog(ERROR, "Cannot open destination file \"%s\": %s",
 			 to_path, strerror(errno));
-	}
 
 	/* update file permission */
 	if (fio_chmod(to_path, file->mode, to_location) == -1)
-	{
-		fio_fclose(out);
-		elog(ERROR, "cannot change mode of \"%s\": %s", to_path,
+		elog(ERROR, "Cannot change mode of \"%s\": %s", to_path,
 			 strerror(errno));
-	}
 
 	if (fio_fclose(out))
-		elog(ERROR, "cannot close \"%s\": %s", to_path, strerror(errno));
+		elog(ERROR, "Cannot close \"%s\": %s", to_path, strerror(errno));
 
 	return true;
 }

src/restore.c

@@ -697,6 +697,11 @@ restore_chain(pgBackup *dest_backup, parray *parent_chain,
 			if (S_ISDIR(dest_file->mode))
 				continue;
 
+			/* skip external files if ordered to do so */
+			if (dest_file->external_dir_num > 0 &&
+				params->skip_external_dirs)
+				continue;
+
 			/* construct fullpath */
 			if (dest_file->external_dir_num == 0)
 			{
@@ -708,7 +713,7 @@ restore_chain(pgBackup *dest_backup, parray *parent_chain,
 			}
 			else
 			{
-				char	*external_path = parray_get(external_dirs, dest_file->external_dir_num - 1);
+				char *external_path = parray_get(external_dirs, dest_file->external_dir_num - 1);
 				join_path_components(to_fullpath, external_path, dest_file->rel_path);
 			}
 
@@ -831,8 +836,10 @@ restore_files(void *arg)
 				 to_fullpath, strerror(errno_tmp));
 		}
 
-		if (!fio_is_remote_file(out))
-			setbuf(out, buffer);
+		/* update file permission */
+		if (fio_chmod(to_fullpath, dest_file->mode, FIO_DB_HOST) == -1)
+			elog(ERROR, "Cannot change mode of \"%s\": %s", to_fullpath,
+				 strerror(errno));
 
 		if (!dest_file->is_datafile || dest_file->is_cfs)
 			elog(VERBOSE, "Restoring non-data file: \"%s\"", to_fullpath);
@@ -843,6 +850,9 @@ restore_files(void *arg)
 		if (dest_file->write_size == 0)
 			goto done;
 
+		if (!fio_is_remote_file(out))
+			setbuf(out, buffer);
+
 		/* Restore destination file */
 		if (dest_file->is_datafile && !dest_file->is_cfs)
 			/* Destination file is data file */
@@ -853,30 +863,7 @@ restore_files(void *arg)
 			arguments->restored_bytes += restore_non_data_file(arguments->parent_chain,
 										arguments->dest_backup, dest_file, out, to_fullpath);
 
-		/*
-		 * Destination file is data file.
-		 * Iterate over incremental chain and lookup given destination file.
-		 * Apply changed blocks to destination file from every backup in parent chain.
-		 */
-
-		done:
-
-		/* Truncate file up to n_blocks. NOTE: no need, we just should not write
-		 * blocks that are exceeding n_blocks.
-		 * But for this to work, n_blocks should be trusted.
-		 */
-
-		/* update file permission
-		 * TODO: chmod must be done right after fopen()
-		 */
-		if (fio_chmod(to_fullpath, dest_file->mode, FIO_DB_HOST) == -1)
-		{
-			int errno_tmp = errno;
-			fio_fclose(out);
-			elog(ERROR, "Cannot change mode of \"%s\": %s", to_fullpath,
-				 strerror(errno_tmp));
-		}
-
+done:
 		/* close file */
 		if (fio_fclose(out) != 0)
 			elog(ERROR, "Cannot close file \"%s\": %s", to_fullpath,