PGPRO-2096: Use CRC-32 instead of CRC-32C

za-arthur · za-arthur · commit 23cf692428c0 · 2018-10-26T17:49:39.000+03:00
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.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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);
 
