Merge pull request #75 from ossc-db/#72

Fix a bug which restores already deleted data.(#72)

data.c

@@ -189,17 +189,6 @@ typedef union DataPage
 	char				data[BLCKSZ];
 } DataPage;
 
-/*
- * Along with each data page, the following information is written to the
- * backup.
- */
-typedef struct BackupPageHeader
-{
-	BlockNumber	block;			/* block number */
-	uint16		hole_offset;	/* number of bytes before "hole" */
-	uint16		hole_length;	/* number of bytes in "hole" */
-} BackupPageHeader;
-
 /*
  * From the page header of the actual PostgreSQL data page, extract
  * information that is written to the backup as part of BackupPageHeader.
@@ -369,6 +358,7 @@ backup_data_file(const char *from_root,
 		int		upper_length;
 
 		header.block = blknum;
+		header.endpoint = false;
 
 		/*
 		 * If a invalid data page was found, fallback to simple copy to ensure
@@ -525,11 +515,56 @@ backup_data_file(const char *from_root,
 
 		file->read_size += read_len;
 	}
+	/*
+	 * In incremental backup mode, append a special page header, with
+	 * endpoint field set to true, to mark the end of relation as of this
+	 * backup.
+	 * This is used when restoring the backup, to truncate any subsequent
+	 * pages that may be present in the previous full backup.
+	 * In incremental backup mode, put the endpoint to remember the last
+	 * block number.
+	 */
+	if (current.backup_mode == BACKUP_MODE_INCREMENTAL)
+	{
+		header.block = ++blknum;
+		header.endpoint = true;
+
+#ifdef HAVE_LIBZ
+		if (compress)
+		{
+			doDeflate(&z, sizeof(header), sizeof(outbuf), &header, outbuf, in,
+					  out, &crc, &file->write_size, Z_NO_FLUSH);
+		}
+		else
+#endif
+		{
+		    if (fwrite(&header, 1, sizeof(header), out) != sizeof(header))
+			{
+				int errno_tmp = errno;
+				/* oops */
+				fclose(in);
+				fclose(out);
+				ereport(ERROR,
+					(errcode(ERROR_SYSTEM),
+					 errmsg("could not write at block %u of \"%s\": %s to put endpoint",
+						blknum, to_path, strerror(errno_tmp))));
+			}
+			PGRMAN_COMP_CRC32(crc, &header, sizeof(header));
+
+			file->write_size += sizeof(header);
+		}
+	}
 
 #ifdef HAVE_LIBZ
 	if (compress)
 	{
-		if (file->read_size > 0)
+		/*
+		 * finalize zstream.
+		 *
+		 * NOTE: We need to do this even if we didn't read anything from the
+		 * file but still had to write the special page header.
+		 */
+		if (file->read_size > 0 || header.endpoint)
 		{
 			while (doDeflate(&z, 0, sizeof(outbuf), NULL, outbuf, in, out, &crc,
 							 &file->write_size, Z_FINISH) != Z_STREAM_END)
@@ -685,7 +720,10 @@ restore_data_file(const char *from_root,
 		{
 			status = doInflate(&z, sizeof(inbuf), sizeof(header), inbuf,
 						&header, in, out, &crc, &read_size);
-			if (status == Z_STREAM_END)
+
+			/* when the stream ends, proceed to next block unless the block is
+			 * flagged as endpoint, which needs an additional truncation process.*/
+			if (status == Z_STREAM_END && !header.endpoint)
 			{
 				if (z.avail_out != sizeof(header))
 					ereport(ERROR,
@@ -721,6 +759,20 @@ restore_data_file(const char *from_root,
 							blknum, file->path, strerror(errno_tmp))));
 			}
 		}
+		if (header.endpoint)
+		{
+		/*
+		 * endpoint means there are no pages any more at the point of
+		 * incremental backup, so truncate it.
+		 * This process is necessary for preventing unexpected deleted
+		 * data comeback which happens when a vacuum shrink relations
+		 * between a full backup and an incremental backup.
+		 */
+		    blknum = header.block;
+			elog(DEBUG, "truncating file. %s blknum: %d", to_path, blknum);
+			truncate(to_path, (blknum - 1) * BLCKSZ);
+			break;
+		}
 
 		if (header.block < blknum || header.hole_offset > BLCKSZ ||
 			(int) header.hole_offset + (int) header.hole_length > BLCKSZ)

expected/restore.out

@@ -58,3 +58,15 @@ OK: hard-copy option works well.
 0
 0
 
+###### RESTORE COMMAND TEST-0011 ######
+###### vacuum shrinks a page between full and incremental backups ######
+0
+0
+0
+
+###### RESTORE COMMAND TEST-0012 ######
+###### vacuum shrinks a page between full and incremental backups(compressed) ######
+0
+0
+0
+

expected/restore_checksum.out

@@ -58,3 +58,15 @@ OK: hard-copy option works well.
 0
 0
 
+###### RESTORE COMMAND TEST-0011 ######
+###### vacuum shrinks a page between full and incremental backups ######
+0
+0
+0
+
+###### RESTORE COMMAND TEST-0012 ######
+###### vacuum shrinks a page between full and incremental backups(compressed) ######
+0
+0
+0
+

pg_rman.h

@@ -88,6 +88,21 @@ typedef struct pgBackupRange
 	time_t	end;			/* begin +1 when one backup is target */
 } pgBackupRange;
 
+/*
+ * Along with each data page, the following information is written to the
+ * backup.
+ */
+typedef struct BackupPageHeader
+{
+    BlockNumber block;          /* block number */
+    uint16      hole_offset;    /* number of bytes before "hole" */
+    uint16      hole_length;    /* number of bytes in "hole" */
+    bool        endpoint;       /* If set to true, this page marks the end
+                                   of relation, which means any subsequent
+                                   pages are truncated. */
+} BackupPageHeader;
+
+
 #define pgBackupRangeIsValid(range)	\
 	(((range)->begin != (time_t) 0) || ((range)->end != (time_t) 0))
 #define pgBackupRangeIsSingle(range) \

sql/restore.sh

@@ -307,6 +307,49 @@ psql --no-psqlrc -p ${TEST_PGPORT} -d db0010 -c "SELECT * FROM pgbench_branches;
 diff ${TEST_BASE}/TEST-0010-before.out ${TEST_BASE}/TEST-0010-after.out
 echo ''
 
+echo '###### RESTORE COMMAND TEST-0011 ######'
+echo '###### vacuum shrinks a page between full and incremental backups ######'
+init_backup
+pg_ctl start -w -t 600 > /dev/null 2>&1
+createdb db0011 -p ${TEST_PGPORT}
+psql --no-psqlrc -p ${TEST_PGPORT} -d db0011 -c "CREATE TABLE t0011(i int,j int,k varchar);" > /dev/null 2>&1
+psql --no-psqlrc -p ${TEST_PGPORT} -d db0011 -c "INSERT INTO t0011 (i,j,k) select generate_series(1,1000),1, repeat('a', 10);" > /dev/null 2>&1
+pg_rman backup -B ${BACKUP_PATH} -b full -p ${TEST_PGPORT} -d postgres --quiet;echo $?
+pg_rman validate -B ${BACKUP_PATH} --quiet
+psql --no-psqlrc -p ${TEST_PGPORT} -d db0011 -c "DELETE FROM t0011 WHERE i > 10;" > /dev/null 2>&1
+psql --no-psqlrc -p ${TEST_PGPORT} -d db0011 -c "VACUUM t0011;" > /dev/null 2>&1
+pg_rman backup -B ${BACKUP_PATH} -b incremental -p ${TEST_PGPORT} -d postgres --quiet;echo $?
+pg_rman validate -B ${BACKUP_PATH} --quiet
+psql --no-psqlrc -p ${TEST_PGPORT} -d db0011 -c "SELECT * FROM t0011;" > ${TEST_BASE}/TEST-0011-before.out
+pg_ctl stop -m fast > /dev/null 2>&1
+pg_rman restore -B ${BACKUP_PATH} --quiet;echo $?
+pg_ctl start -w -t 600 > /dev/null 2>&1
+sleep 1
+psql --no-psqlrc -p ${TEST_PGPORT} -d db0011 -c "SELECT * FROM t0011;" > ${TEST_BASE}/TEST-0011-after.out
+diff ${TEST_BASE}/TEST-0011-before.out ${TEST_BASE}/TEST-0011-after.out
+echo ''
+
+echo '###### RESTORE COMMAND TEST-0012 ######'
+echo '###### vacuum shrinks a page between full and incremental backups(compressed) ######'
+init_backup
+pg_ctl start -w -t 600 > /dev/null 2>&1
+createdb db0012 -p ${TEST_PGPORT}
+psql --no-psqlrc -p ${TEST_PGPORT} -d db0012 -c "CREATE TABLE t0012(i int,j int,k varchar);" > /dev/null 2>&1
+psql --no-psqlrc -p ${TEST_PGPORT} -d db0012 -c "INSERT INTO t0012 (i,j,k) select generate_series(1,1000),1, repeat('a', 10);" > /dev/null 2>&1
+pg_rman backup -B ${BACKUP_PATH} -b full -Z -p ${TEST_PGPORT} -d postgres --quiet;echo $?
+pg_rman validate -B ${BACKUP_PATH} --quiet
+psql --no-psqlrc -p ${TEST_PGPORT} -d db0012 -c "DELETE FROM t0012 WHERE i > 10;" > /dev/null 2>&1
+psql --no-psqlrc -p ${TEST_PGPORT} -d db0012 -c "VACUUM t0012;" > /dev/null 2>&1
+pg_rman backup -B ${BACKUP_PATH} -b incremental -Z -p ${TEST_PGPORT} -d postgres --quiet;echo $?
+pg_rman validate -B ${BACKUP_PATH} --quiet
+psql --no-psqlrc -p ${TEST_PGPORT} -d db0012 -c "SELECT * FROM t0012;" > ${TEST_BASE}/TEST-0012-before.out
+pg_ctl stop -m fast > /dev/null 2>&1
+pg_rman restore -B ${BACKUP_PATH} --quiet;echo $?
+pg_ctl start -w -t 600 > /dev/null 2>&1
+sleep 1
+psql --no-psqlrc -p ${TEST_PGPORT} -d db0012 -c "SELECT * FROM t0012;" > ${TEST_BASE}/TEST-0012-after.out
+diff ${TEST_BASE}/TEST-0012-before.out ${TEST_BASE}/TEST-0012-after.out
+echo ''
 
 # clean up the temporal test data
 pg_ctl stop -m immediate > /dev/null 2>&1