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