From 23cf692428c0c55ea56f45f0d37ef14585a8744a Mon Sep 17 00:00:00 2001 From: Arthur Zakirov Date: Fri, 26 Oct 2018 17:49:39 +0300 Subject: [PATCH] PGPRO-2096: Use CRC-32 instead of CRC-32C Using CRC-32C to calculate checksum of pg_control gives same value for different backups. It might be because pg_control stores its content plus checksum of the content. --- .gitignore | 1 + Makefile | 33 +++++++++++++++++++++------------ src/backup.c | 12 ++++++------ src/data.c | 26 +++++++++++++------------- src/dir.c | 8 ++++---- 5 files changed, 45 insertions(+), 35 deletions(-) diff --git a/.gitignore b/.gitignore index 02d1512a..4fc21d91 100644 --- a/.gitignore +++ b/.gitignore @@ -33,6 +33,7 @@ /tests/helpers/*pyc # Extra files +/src/pg_crc.c /src/datapagemap.c /src/datapagemap.h /src/logging.h diff --git a/Makefile b/Makefile index 80b907be..55d4428c 100644 --- a/Makefile +++ b/Makefile @@ -1,16 +1,23 @@ PROGRAM = pg_probackup -OBJS = src/backup.o src/catalog.o src/configure.o src/data.o \ - src/delete.o src/dir.o src/fetch.o src/help.o src/init.o \ - src/pg_probackup.o src/restore.o src/show.o \ - src/util.o src/validate.o src/datapagemap.o src/parsexlog.o \ - src/xlogreader.o src/streamutil.o src/receivelog.o \ - src/archive.o src/utils/parray.o src/utils/pgut.o src/utils/logger.o \ - src/utils/json.o src/utils/thread.o src/merge.o -EXTRA_CLEAN = src/datapagemap.c src/datapagemap.h src/xlogreader.c \ - src/receivelog.c src/receivelog.h src/streamutil.c src/streamutil.h src/logging.h +# utils +OBJS = src/utils/json.o src/utils/logger.o src/utils/parray.o \ + src/utils/pgut.o src/utils/thread.o -INCLUDES = src/datapagemap.h src/logging.h src/receivelog.h src/streamutil.h +OBJS += src/archive.o src/backup.o src/catalog.o src/configure.o src/data.o \ + src/delete.o src/dir.o src/fetch.o src/help.o src/init.o src/merge.o \ + src/parsexlog.o src/pg_probackup.o src/restore.o src/show.o src/util.o \ + src/validate.o + +# borrowed files +OBJS += src/pg_crc.o src/datapagemap.o src/receivelog.o src/streamutil.o \ + src/xlogreader.o + +EXTRA_CLEAN = src/pg_crc.c src/datapagemap.c src/datapagemap.h src/logging.h \ + src/receivelog.c src/receivelog.h src/streamutil.c src/streamutil.h \ + src/xlogreader.c + +INCLUDES = src/datapagemap.h src/logging.h src/streamutil.h src/receivelog.h ifdef USE_PGXS PG_CONFIG = pg_config @@ -46,14 +53,14 @@ all: checksrcdir $(INCLUDES); $(PROGRAM): $(OBJS) -src/xlogreader.c: $(top_srcdir)/src/backend/access/transam/xlogreader.c - rm -f $@ && $(LN_S) $(srchome)/src/backend/access/transam/xlogreader.c $@ src/datapagemap.c: $(top_srcdir)/src/bin/pg_rewind/datapagemap.c rm -f $@ && $(LN_S) $(srchome)/src/bin/pg_rewind/datapagemap.c $@ src/datapagemap.h: $(top_srcdir)/src/bin/pg_rewind/datapagemap.h rm -f $@ && $(LN_S) $(srchome)/src/bin/pg_rewind/datapagemap.h $@ src/logging.h: $(top_srcdir)/src/bin/pg_rewind/logging.h rm -f $@ && $(LN_S) $(srchome)/src/bin/pg_rewind/logging.h $@ +src/pg_crc.c: $(top_srcdir)/src/backend/utils/hash/pg_crc.c + rm -f $@ && $(LN_S) $(srchome)/src/backend/utils/hash/pg_crc.c $@ src/receivelog.c: $(top_srcdir)/src/bin/pg_basebackup/receivelog.c rm -f $@ && $(LN_S) $(srchome)/src/bin/pg_basebackup/receivelog.c $@ src/receivelog.h: $(top_srcdir)/src/bin/pg_basebackup/receivelog.h @@ -62,6 +69,8 @@ src/streamutil.c: $(top_srcdir)/src/bin/pg_basebackup/streamutil.c rm -f $@ && $(LN_S) $(srchome)/src/bin/pg_basebackup/streamutil.c $@ src/streamutil.h: $(top_srcdir)/src/bin/pg_basebackup/streamutil.h rm -f $@ && $(LN_S) $(srchome)/src/bin/pg_basebackup/streamutil.h $@ +src/xlogreader.c: $(top_srcdir)/src/backend/access/transam/xlogreader.c + rm -f $@ && $(LN_S) $(srchome)/src/backend/access/transam/xlogreader.c $@ ifeq (,$(filter 9.5 9.6,$(MAJORVERSION))) diff --git a/src/backup.c b/src/backup.c index 1956569e..1d58d9f2 100644 --- a/src/backup.c +++ b/src/backup.c @@ -306,7 +306,7 @@ remote_copy_file(PGconn *conn, pgFile* file) to_path, strerror(errno_tmp)); } - INIT_CRC32C(file->crc); + INIT_TRADITIONAL_CRC32(file->crc); /* read from stream and write to backup file */ while (1) @@ -332,14 +332,14 @@ remote_copy_file(PGconn *conn, pgFile* file) { write_buffer_size = Min(row_length, sizeof(buf)); memcpy(buf, copybuf, write_buffer_size); - COMP_CRC32C(file->crc, buf, write_buffer_size); + COMP_TRADITIONAL_CRC32(file->crc, buf, write_buffer_size); /* TODO calc checksum*/ if (fwrite(buf, 1, write_buffer_size, out) != write_buffer_size) { errno_tmp = errno; /* oops */ - FIN_CRC32C(file->crc); + FIN_TRADITIONAL_CRC32(file->crc); fclose(out); PQfinish(conn); elog(ERROR, "cannot write to \"%s\": %s", to_path, @@ -363,7 +363,7 @@ remote_copy_file(PGconn *conn, pgFile* file) } file->write_size = (int64) file->read_size; - FIN_CRC32C(file->crc); + FIN_TRADITIONAL_CRC32(file->crc); fclose(out); } @@ -2137,7 +2137,7 @@ backup_files(void *arg) continue; } } - else + else { bool skip = false; @@ -2147,7 +2147,7 @@ backup_files(void *arg) { calc_file_checksum(file); /* ...and checksum is the same... */ - if (EQ_CRC32C(file->crc, (*prev_file)->crc)) + if (EQ_TRADITIONAL_CRC32(file->crc, (*prev_file)->crc)) skip = true; /* ...skip copying file. */ } if (skip || diff --git a/src/data.c b/src/data.c index 50b39566..e8b7171f 100644 --- a/src/data.c +++ b/src/data.c @@ -416,7 +416,7 @@ compress_and_backup_page(pgFile *file, BlockNumber blknum, blknum, header.compressed_size, write_buffer_size); */ /* Update CRC */ - COMP_CRC32C(*crc, write_buffer, write_buffer_size); + COMP_TRADITIONAL_CRC32(*crc, write_buffer, write_buffer_size); /* write data page */ if(fwrite(write_buffer, 1, write_buffer_size, out) != write_buffer_size) @@ -476,13 +476,13 @@ backup_data_file(backup_files_arg* arguments, /* reset size summary */ file->read_size = 0; file->write_size = 0; - INIT_CRC32C(file->crc); + INIT_TRADITIONAL_CRC32(file->crc); /* open backup mode file for read */ in = fopen(file->path, PG_BINARY_R); if (in == NULL) { - FIN_CRC32C(file->crc); + FIN_TRADITIONAL_CRC32(file->crc); /* * If file is not found, this is not en error. @@ -587,7 +587,7 @@ backup_data_file(backup_files_arg* arguments, to_path, strerror(errno)); fclose(in); - FIN_CRC32C(file->crc); + FIN_TRADITIONAL_CRC32(file->crc); /* * If we have pagemap then file in the backup can't be a zero size. @@ -839,7 +839,7 @@ copy_file(const char *from_root, const char *to_root, pgFile *file) struct stat st; pg_crc32 crc; - INIT_CRC32C(crc); + INIT_TRADITIONAL_CRC32(crc); /* reset size summary */ file->read_size = 0; @@ -849,7 +849,7 @@ copy_file(const char *from_root, const char *to_root, pgFile *file) in = fopen(file->path, PG_BINARY_R); if (in == NULL) { - FIN_CRC32C(crc); + FIN_TRADITIONAL_CRC32(crc); file->crc = crc; /* maybe deleted, it's not error */ @@ -898,7 +898,7 @@ copy_file(const char *from_root, const char *to_root, pgFile *file) strerror(errno_tmp)); } /* update CRC */ - COMP_CRC32C(crc, buf, read_len); + COMP_TRADITIONAL_CRC32(crc, buf, read_len); file->read_size += read_len; } @@ -925,14 +925,14 @@ copy_file(const char *from_root, const char *to_root, pgFile *file) strerror(errno_tmp)); } /* update CRC */ - COMP_CRC32C(crc, buf, read_len); + COMP_TRADITIONAL_CRC32(crc, buf, read_len); file->read_size += read_len; } file->write_size = (int64) file->read_size; /* finish CRC calculation and store into pgFile */ - FIN_CRC32C(crc); + FIN_TRADITIONAL_CRC32(crc); file->crc = crc; /* update file permission */ @@ -1350,7 +1350,7 @@ calc_file_checksum(pgFile *file) pg_crc32 crc; Assert(S_ISREG(file->mode)); - INIT_CRC32C(crc); + INIT_TRADITIONAL_CRC32(crc); /* reset size summary */ file->read_size = 0; @@ -1360,7 +1360,7 @@ calc_file_checksum(pgFile *file) in = fopen(file->path, PG_BINARY_R); if (in == NULL) { - FIN_CRC32C(crc); + FIN_TRADITIONAL_CRC32(crc); file->crc = crc; /* maybe deleted, it's not error */ @@ -1387,7 +1387,7 @@ calc_file_checksum(pgFile *file) break; /* update CRC */ - COMP_CRC32C(crc, buf, read_len); + COMP_TRADITIONAL_CRC32(crc, buf, read_len); file->write_size += read_len; file->read_size += read_len; @@ -1402,7 +1402,7 @@ calc_file_checksum(pgFile *file) } /* finish CRC calculation and store into pgFile */ - FIN_CRC32C(crc); + FIN_TRADITIONAL_CRC32(crc); file->crc = crc; fclose(in); diff --git a/src/dir.c b/src/dir.c index c211cc32..60f13705 100644 --- a/src/dir.c +++ b/src/dir.c @@ -274,20 +274,20 @@ pgFileGetCRC(const char *file_path) file_path, strerror(errno)); /* calc CRC of backup file */ - INIT_CRC32C(crc); + INIT_TRADITIONAL_CRC32(crc); while ((len = fread(buf, 1, sizeof(buf), fp)) == sizeof(buf)) { if (interrupted) elog(ERROR, "interrupted during CRC calculation"); - COMP_CRC32C(crc, buf, len); + COMP_TRADITIONAL_CRC32(crc, buf, len); } errno_tmp = errno; if (!feof(fp)) elog(WARNING, "cannot read \"%s\": %s", file_path, strerror(errno_tmp)); if (len > 0) - COMP_CRC32C(crc, buf, len); - FIN_CRC32C(crc); + COMP_TRADITIONAL_CRC32(crc, buf, len); + FIN_TRADITIONAL_CRC32(crc); fclose(fp);