On Thu, 2013-04-11 at 20:12 +0100, Simon Riggs wrote: > So, if we apply a patch like the one attached, we then end up with the > WAL checksum using the page checksum as an integral part of its > calculation. (There is no increase in code inside WALInsertLock, > nothing at all touched in that area). > > > Then all we need to do is make PageSetChecksumInplace() use Ants' algo > and we're done. > > > Only point worth discussing is that this change would make backup > blocks be covered by a 16-bit checksum, not the CRC-32 it is now. i.e. > the record header is covered by a CRC32 but the backup blocks only by > 16-bit.
FWIW, that's fine with me. > (Attached patch is discussion only. Checking checksum in recovery > isn't coded at all.) I like it. A few points: * Given that setting the checksum is unconditional in a backup block, do we want to zero the checksum field when the backup block is restored if checksums are disabled? Otherwise we would have a strange situation where some blocks have a checksum on disk even when checksums are disabled. * When we do PageSetChecksumInplace(), we need to be 100% sure that the hole is empty; otherwise the checksum will fail when we re-expand it. It might be worth a memset beforehand just to be sure. Regards, Jeff Davis -- Sent via pgsql-hackers mailing list (firstname.lastname@example.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers