On Thu, 2007-01-04 at 12:13 -0500, Tom Lane wrote:
> "Simon Riggs" <[EMAIL PROTECTED]> writes:
> > On Thu, 2007-01-04 at 11:09 -0500, Tom Lane wrote:
> >> "It works most of the time" doesn't exactly satisfy me.
>
> > It seemed safer to allow a very rare error through to the next level of
> > error checking rather than to close the door so tight that recovery
> > would not be possible in a very rare case.
>
> If a DBA is turning checksums off at all, he's already bought into the
> assumption that he's prepared to recover from backups. What you don't
> seem to get here is that this "feature" is pretty darn questionable in
> the first place, and for it to have a side effect of poking a hole in
> the system's reliability even when it's off is more than enough to get
> it rejected outright. It's just a No Sale.
I get it, and I listened. I'm was/am happy to do it the way you
suggested; I was merely explaining that I had considered the issue.
New patch enclosed.
--
Simon Riggs
EnterpriseDB http://www.enterprisedb.com
Index: src/backend/access/transam/xlog.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/access/transam/xlog.c,v
retrieving revision 1.259
diff -c -r1.259 xlog.c
*** src/backend/access/transam/xlog.c 8 Dec 2006 19:50:53 -0000 1.259
--- src/backend/access/transam/xlog.c 4 Jan 2007 17:34:13 -0000
***************
*** 137,142 ****
--- 137,143 ----
char *XLOG_sync_method = NULL;
const char XLOG_sync_method_default[] = DEFAULT_SYNC_METHOD_STR;
bool fullPageWrites = true;
+ bool compute_wal_crc = true;
#ifdef WAL_DEBUG
bool XLOG_DEBUG = false;
***************
*** 607,613 ****
*/
doPageWrites = fullPageWrites || Insert->forcePageWrites;
! INIT_CRC32(rdata_crc);
len = 0;
for (rdt = rdata;;)
{
--- 608,614 ----
*/
doPageWrites = fullPageWrites || Insert->forcePageWrites;
! INIT_CRC32(rdata_crc);
len = 0;
for (rdt = rdata;;)
{
***************
*** 615,621 ****
{
/* Simple data, just include it */
len += rdt->len;
! COMP_CRC32(rdata_crc, rdt->data, rdt->len);
}
else
{
--- 616,623 ----
{
/* Simple data, just include it */
len += rdt->len;
! if (compute_wal_crc)
! COMP_CRC32(rdata_crc, rdt->data, rdt->len);
}
else
{
***************
*** 630,636 ****
else if (rdt->data)
{
len += rdt->len;
! COMP_CRC32(rdata_crc, rdt->data, rdt->len);
}
break;
}
--- 632,639 ----
else if (rdt->data)
{
len += rdt->len;
! if (compute_wal_crc)
! COMP_CRC32(rdata_crc, rdt->data, rdt->len);
}
break;
}
***************
*** 647,653 ****
else if (rdt->data)
{
len += rdt->len;
! COMP_CRC32(rdata_crc, rdt->data, rdt->len);
}
break;
}
--- 650,657 ----
else if (rdt->data)
{
len += rdt->len;
! if (compute_wal_crc)
! COMP_CRC32(rdata_crc, rdt->data, rdt->len);
}
break;
}
***************
*** 662,701 ****
rdt = rdt->next;
}
! /*
! * Now add the backup block headers and data into the CRC
! */
! for (i = 0; i < XLR_MAX_BKP_BLOCKS; i++)
! {
! if (dtbuf_bkp[i])
! {
! BkpBlock *bkpb = &(dtbuf_xlg[i]);
! char *page;
! COMP_CRC32(rdata_crc,
! (char *) bkpb,
! sizeof(BkpBlock));
! page = (char *) BufferGetBlock(dtbuf[i]);
! if (bkpb->hole_length == 0)
! {
! COMP_CRC32(rdata_crc,
! page,
! BLCKSZ);
! }
! else
! {
! /* must skip the hole */
! COMP_CRC32(rdata_crc,
! page,
! bkpb->hole_offset);
! COMP_CRC32(rdata_crc,
! page + (bkpb->hole_offset + bkpb->hole_length),
! BLCKSZ - (bkpb->hole_offset + bkpb->hole_length));
! }
! }
! }
!
! /*
* NOTE: We disallow len == 0 because it provides a useful bit of extra
* error checking in ReadRecord. This means that all callers of
* XLogInsert must supply at least some not-in-a-buffer data. However, we
--- 666,708 ----
rdt = rdt->next;
}
! if (compute_wal_crc)
! {
! /*
! * Now add the backup block headers and data into the CRC
! */
! for (i = 0; i < XLR_MAX_BKP_BLOCKS; i++)
! {
! if (dtbuf_bkp[i])
! {
! BkpBlock *bkpb = &(dtbuf_xlg[i]);
! char *page;
!
! COMP_CRC32(rdata_crc,
! (char *) bkpb,
! sizeof(BkpBlock));
! page = (char *) BufferGetBlock(dtbuf[i]);
! if (bkpb->hole_length == 0)
! {
! COMP_CRC32(rdata_crc,
! page,
! BLCKSZ);
! }
! else
! {
! /* must skip the hole */
! COMP_CRC32(rdata_crc,
! page,
! bkpb->hole_offset);
! COMP_CRC32(rdata_crc,
! page + (bkpb->hole_offset + bkpb->hole_length),
! BLCKSZ - (bkpb->hole_offset + bkpb->hole_length));
! }
! }
! }
! }
! /*
* NOTE: We disallow len == 0 because it provides a useful bit of extra
* error checking in ReadRecord. This means that all callers of
* XLogInsert must supply at least some not-in-a-buffer data. However, we
***************
*** 919,928 ****
record->xl_rmid = rmid;
/* Now we can finish computing the record's CRC */
! COMP_CRC32(rdata_crc, (char *) record + sizeof(pg_crc32),
! SizeOfXLogRecord - sizeof(pg_crc32));
! FIN_CRC32(rdata_crc);
! record->xl_crc = rdata_crc;
#ifdef WAL_DEBUG
if (XLOG_DEBUG)
--- 926,940 ----
record->xl_rmid = rmid;
/* Now we can finish computing the record's CRC */
! if (compute_wal_crc)
! {
! COMP_CRC32(rdata_crc, (char *) record + sizeof(pg_crc32),
! SizeOfXLogRecord - sizeof(pg_crc32));
! FIN_CRC32(rdata_crc);
! record->xl_crc = rdata_crc;
! }
! else
! record->xl_crc = 0;
#ifdef WAL_DEBUG
if (XLOG_DEBUG)
***************
*** 2783,2791 ****
BkpBlock bkpb;
char *blk;
/* First the rmgr data */
! INIT_CRC32(crc);
! COMP_CRC32(crc, XLogRecGetData(record), len);
/* Add in the backup blocks, if any */
blk = (char *) XLogRecGetData(record) + len;
--- 2795,2806 ----
BkpBlock bkpb;
char *blk;
+ if (!compute_wal_crc)
+ return true;
+
/* First the rmgr data */
! INIT_CRC32(crc);
! COMP_CRC32(crc, XLogRecGetData(record), len);
/* Add in the backup blocks, if any */
blk = (char *) XLogRecGetData(record) + len;
***************
*** 2805,2811 ****
return false;
}
blen = sizeof(BkpBlock) + BLCKSZ - bkpb.hole_length;
! COMP_CRC32(crc, blk, blen);
blk += blen;
}
--- 2820,2826 ----
return false;
}
blen = sizeof(BkpBlock) + BLCKSZ - bkpb.hole_length;
! COMP_CRC32(crc, blk, blen);
blk += blen;
}
***************
*** 4146,4157 ****
record->xl_rmid = RM_XLOG_ID;
memcpy(XLogRecGetData(record), &checkPoint, sizeof(checkPoint));
! INIT_CRC32(crc);
! COMP_CRC32(crc, &checkPoint, sizeof(checkPoint));
! COMP_CRC32(crc, (char *) record + sizeof(pg_crc32),
! SizeOfXLogRecord - sizeof(pg_crc32));
! FIN_CRC32(crc);
! record->xl_crc = crc;
/* Create first XLOG segment file */
use_existent = false;
--- 4161,4177 ----
record->xl_rmid = RM_XLOG_ID;
memcpy(XLogRecGetData(record), &checkPoint, sizeof(checkPoint));
! if (compute_wal_crc)
! {
! INIT_CRC32(crc);
! COMP_CRC32(crc, &checkPoint, sizeof(checkPoint));
! COMP_CRC32(crc, (char *) record + sizeof(pg_crc32),
! SizeOfXLogRecord - sizeof(pg_crc32));
! FIN_CRC32(crc);
! record->xl_crc = crc;
! }
! else
! record->xl_crc = 0;
/* Create first XLOG segment file */
use_existent = false;
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/utils/misc/guc.c,v
retrieving revision 1.364
diff -c -r1.364 guc.c
*** src/backend/utils/misc/guc.c 30 Dec 2006 21:21:54 -0000 1.364
--- src/backend/utils/misc/guc.c 4 Jan 2007 17:34:18 -0000
***************
*** 548,553 ****
--- 548,563 ----
true, NULL, NULL
},
{
+ {"wal_checksum", PGC_POSTMASTER, WAL_SETTINGS,
+ gettext_noop("Calculates CRC checksum for all WAL records."),
+ gettext_noop("This option calculates checksums for all WAL records, so that the "
+ "checksum can be rechecked during recovery following a system crash."),
+ GUC_NOT_IN_SAMPLE
+ },
+ &compute_wal_crc,
+ true, NULL, NULL
+ },
+ {
{"silent_mode", PGC_POSTMASTER, LOGGING_WHEN,
gettext_noop("Runs the server silently."),
gettext_noop("If this parameter is set, the server will automatically run in the "
Index: src/include/access/xlog.h
===================================================================
RCS file: /projects/cvsroot/pgsql/src/include/access/xlog.h,v
retrieving revision 1.75
diff -c -r1.75 xlog.h
*** src/include/access/xlog.h 5 Nov 2006 22:42:10 -0000 1.75
--- src/include/access/xlog.h 4 Jan 2007 17:34:19 -0000
***************
*** 140,145 ****
--- 140,146 ----
extern int XLOGbuffers;
extern char *XLogArchiveCommand;
extern int XLogArchiveTimeout;
+ extern bool compute_wal_crc;
extern char *XLOG_sync_method;
extern const char XLOG_sync_method_default[];
---------------------------(end of broadcast)---------------------------
TIP 2: Don't 'kill -9' the postmaster
