From ea3ed9fa171f5807bdb4790f8b64a0211cb729a7 Mon Sep 17 00:00:00 2001 From: Arthur Zakirov Date: Thu, 8 Nov 2018 15:04:55 +0300 Subject: [PATCH] Instead of current global backup variable use function local argument, which is sent by callers --- src/data.c | 59 +++++++++++++++++++++++-------- src/merge.c | 9 +++-- src/pg_probackup.h | 7 ++-- src/restore.c | 3 +- src/validate.c | 11 +++--- tests/expected/option_version.out | 2 +- 6 files changed, 63 insertions(+), 28 deletions(-) diff --git a/src/data.c b/src/data.c index a8378f09..5edeefa4 100644 --- a/src/data.c +++ b/src/data.c @@ -57,7 +57,7 @@ zlib_decompress(void *dst, size_t dst_size, void const *src, size_t src_size) */ static int32 do_compress(void* dst, size_t dst_size, void const* src, size_t src_size, - CompressAlg alg, int level) + CompressAlg alg, int level, const char **errormsg) { switch (alg) { @@ -66,7 +66,13 @@ do_compress(void* dst, size_t dst_size, void const* src, size_t src_size, return -1; #ifdef HAVE_LIBZ case ZLIB_COMPRESS: - return zlib_compress(dst, dst_size, src, src_size, level); + { + int32 ret; + ret = zlib_compress(dst, dst_size, src, src_size, level); + if (ret != Z_OK && errormsg) + *errormsg = zError(ret); + return ret; + } #endif case PGLZ_COMPRESS: return pglz_compress(src, src_size, dst, PGLZ_strategy_always); @@ -81,7 +87,7 @@ do_compress(void* dst, size_t dst_size, void const* src, size_t src_size, */ static int32 do_decompress(void* dst, size_t dst_size, void const* src, size_t src_size, - CompressAlg alg) + CompressAlg alg, const char **errormsg) { switch (alg) { @@ -90,7 +96,13 @@ do_decompress(void* dst, size_t dst_size, void const* src, size_t src_size, return -1; #ifdef HAVE_LIBZ case ZLIB_COMPRESS: - return zlib_decompress(dst, dst_size, src, src_size); + { + int32 ret; + ret = zlib_decompress(dst, dst_size, src, src_size); + if (ret != Z_OK && errormsg) + *errormsg = zError(ret); + return ret; + } #endif case PGLZ_COMPRESS: return pglz_decompress(src, src_size, dst, dst_size); @@ -110,7 +122,7 @@ do_decompress(void* dst, size_t dst_size, void const* src, size_t src_size, * But at least we will do this check only for pages which will no pass validation step. */ static bool -page_may_be_compressed(Page page, CompressAlg alg) +page_may_be_compressed(Page page, CompressAlg alg, uint32 backup_version) { PageHeader phdr; @@ -127,7 +139,7 @@ page_may_be_compressed(Page page, CompressAlg alg) phdr->pd_special == MAXALIGN(phdr->pd_special))) { /* ... end only if it is invalid, then do more checks */ - if (parse_program_version(current.program_version) >= 20023) + if (backup_version >= 20023) { /* Versions 2.0.23 and higher don't have such bug */ return false; @@ -434,9 +446,16 @@ compress_and_backup_page(pgFile *file, BlockNumber blknum, } else { + const char *errormsg = NULL; + /* The page was not truncated, so we need to compress it */ header.compressed_size = do_compress(compressed_page, BLCKSZ, - page, BLCKSZ, calg, clevel); + page, BLCKSZ, calg, clevel, + &errormsg); + /* Something went wrong and errormsg was assigned, throw a warning */ + if (header.compressed_size < 0 && errormsg != NULL) + elog(WARNING, "An error occured during compressing block %u of file \"%s\": %s", + blknum, file->path, errormsg); file->compress_alg = calg; file->read_size += BLCKSZ; @@ -473,7 +492,7 @@ compress_and_backup_page(pgFile *file, BlockNumber blknum, fclose(in); fclose(out); - elog(ERROR, "File: %s, cannot write backup at block %u : %s", + elog(ERROR, "File: %s, cannot write backup at block %u: %s", file->path, blknum, strerror(errno_tmp)); } @@ -661,7 +680,7 @@ backup_data_file(backup_files_arg* arguments, */ void restore_data_file(const char *to_path, pgFile *file, bool allow_truncate, - bool write_header) + bool write_header, uint32 backup_version) { FILE *in = NULL; FILE *out = NULL; @@ -766,14 +785,19 @@ restore_data_file(const char *to_path, pgFile *file, bool allow_truncate, blknum, file->path, read_len, header.compressed_size); if (header.compressed_size != BLCKSZ - || page_may_be_compressed(compressed_page.data, file->compress_alg)) + || page_may_be_compressed(compressed_page.data, file->compress_alg, + backup_version)) { int32 uncompressed_size = 0; + const char *errormsg = NULL; uncompressed_size = do_decompress(page.data, BLCKSZ, compressed_page.data, header.compressed_size, - file->compress_alg); + file->compress_alg, &errormsg); + if (uncompressed_size < 0 && errormsg != NULL) + elog(WARNING, "An error occured during decompressing block %u of file \"%s\": %s", + blknum, file->path, errormsg); if (uncompressed_size != BLCKSZ) elog(ERROR, "page of file \"%s\" uncompressed to %d bytes. != BLCKSZ", @@ -1578,7 +1602,8 @@ validate_one_page(Page page, pgFile *file, /* Valiate pages of datafile in backup one by one */ bool -check_file_pages(pgFile *file, XLogRecPtr stop_lsn, uint32 checksum_version) +check_file_pages(pgFile *file, XLogRecPtr stop_lsn, uint32 checksum_version, + uint32 backup_version) { size_t read_len = 0; bool is_valid = true; @@ -1645,14 +1670,20 @@ check_file_pages(pgFile *file, XLogRecPtr stop_lsn, uint32 checksum_version) blknum, file->path, read_len, header.compressed_size); if (header.compressed_size != BLCKSZ - || page_may_be_compressed(compressed_page.data, file->compress_alg)) + || page_may_be_compressed(compressed_page.data, file->compress_alg, + backup_version)) { int32 uncompressed_size = 0; + const char *errormsg = NULL; uncompressed_size = do_decompress(page.data, BLCKSZ, compressed_page.data, header.compressed_size, - file->compress_alg); + file->compress_alg, + &errormsg); + if (uncompressed_size < 0 && errormsg != NULL) + elog(WARNING, "An error occured during decompressing block %u of file \"%s\": %s", + blknum, file->path, errormsg); if (uncompressed_size != BLCKSZ) { diff --git a/src/merge.c b/src/merge.c index 2464199f..137f1acd 100644 --- a/src/merge.c +++ b/src/merge.c @@ -457,14 +457,16 @@ merge_files(void *arg) file->path = to_path_tmp; /* Decompress first/target file */ - restore_data_file(tmp_file_path, file, false, false); + restore_data_file(tmp_file_path, file, false, false, + parse_program_version(to_backup->program_version)); file->path = prev_path; } /* Merge second/source file with first/target file */ restore_data_file(tmp_file_path, file, from_backup->backup_mode == BACKUP_MODE_DIFF_DELTA, - false); + false, + parse_program_version(from_backup->program_version)); elog(VERBOSE, "Compress file and save it to the directory \"%s\"", argument->to_root); @@ -496,7 +498,8 @@ merge_files(void *arg) /* We can merge in-place here */ restore_data_file(to_path_tmp, file, from_backup->backup_mode == BACKUP_MODE_DIFF_DELTA, - true); + true, + parse_program_version(from_backup->program_version)); /* * We need to calculate write_size, restore_data_file() doesn't diff --git a/src/pg_probackup.h b/src/pg_probackup.h index 7bd87e56..182a647b 100644 --- a/src/pg_probackup.h +++ b/src/pg_probackup.h @@ -517,7 +517,8 @@ extern bool backup_data_file(backup_files_arg* arguments, CompressAlg calg, int clevel); extern void restore_data_file(const char *to_path, pgFile *file, bool allow_truncate, - bool write_header); + bool write_header, + uint32 backup_version); extern bool copy_file(const char *from_root, const char *to_root, pgFile *file); extern void move_file(const char *from_root, const char *to_root, pgFile *file); extern void push_wal_file(const char *from_path, const char *to_path, @@ -526,8 +527,8 @@ extern void get_wal_file(const char *from_path, const char *to_path); extern bool calc_file_checksum(pgFile *file); -extern bool check_file_pages(pgFile* file, - XLogRecPtr stop_lsn, uint32 checksum_version); +extern bool check_file_pages(pgFile* file, XLogRecPtr stop_lsn, + uint32 checksum_version, uint32 backup_version); /* parsexlog.c */ extern void extractPageMap(const char *archivedir, diff --git a/src/restore.c b/src/restore.c index c08e647c..439f3c4e 100644 --- a/src/restore.c +++ b/src/restore.c @@ -621,7 +621,8 @@ restore_files(void *arg) file->path + strlen(from_root) + 1); restore_data_file(to_path, file, arguments->backup->backup_mode == BACKUP_MODE_DIFF_DELTA, - false); + false, + parse_program_version(arguments->backup->program_version)); } else copy_file(from_root, pgdata, file); diff --git a/src/validate.c b/src/validate.c index 4fa7d78b..5cbb9b26 100644 --- a/src/validate.c +++ b/src/validate.c @@ -28,6 +28,7 @@ typedef struct bool corrupted; XLogRecPtr stop_lsn; uint32 checksum_version; + uint32 backup_version; /* * Return value from the thread. @@ -92,11 +93,6 @@ pgBackupValidate(pgBackup *backup) pg_atomic_clear_flag(&file->lock); } - /* - * We use program version to calculate checksum in pgBackupValidateFiles() - */ - validate_backup_version = parse_program_version(backup->program_version); - /* init thread args with own file lists */ threads = (pthread_t *) palloc(sizeof(pthread_t) * num_threads); threads_args = (validate_files_arg *) @@ -111,6 +107,7 @@ pgBackupValidate(pgBackup *backup) arg->corrupted = false; arg->stop_lsn = backup->stop_lsn; arg->checksum_version = backup->checksum_version; + arg->backup_version = parse_program_version(backup->program_version); /* By default there are some error */ threads_args[i].ret = 1; @@ -233,7 +230,9 @@ pgBackupValidateFiles(void *arg) /* validate relation blocks */ if (file->is_datafile) { - if (!check_file_pages(file, arguments->stop_lsn, arguments->checksum_version)) + if (!check_file_pages(file, arguments->stop_lsn, + arguments->checksum_version, + arguments->backup_version)) arguments->corrupted = true; } } diff --git a/tests/expected/option_version.out b/tests/expected/option_version.out index cb0a30d4..6a0391c2 100644 --- a/tests/expected/option_version.out +++ b/tests/expected/option_version.out @@ -1 +1 @@ -pg_probackup 2.0.22 \ No newline at end of file +pg_probackup 2.0.23 \ No newline at end of file