Re: Enabling Checksums

On 26 November 2012 02:32, Jeff Davis <pgsql(at)j-davis(dot)com> wrote:
> Updated both patches.
>
> Changes:
> * Moved the changes to pageinspect into the TLI patch, because it
> makes more sense to be a part of that patch and it also reduces the size
> of the main checksums patch.
> * Fix off-by-one bug in checksum calculation
> * Replace "VerificationInfo" in the function names with "Checksum",
> which is shorter.
> * Make the checksum algorithm process 4 bytes at a time and sum into a
> signed 64-bit int, which is faster than byte-at-a-time. Also, forbid
> zero in either byte of the checksum, because that seems like a good
> idea.
>
> I've done quite a bit of testing at this point, and everything seems
> fine to me. I've tested various kinds of errors (bytes being modified or
> zeroed at various places of the header and data areas, transposed pages)
> at 8192 and 32768 page sizes. I also looked at the distribution of
> checksums in various ways (group by checksum % <prime> for various
> primes, and not seeing any skew), and I didn't see any worrying
> patterns.

I think the way forwards for this is...

1. Break out the changes around inCommit flag, since that is just
uncontroversial refactoring. I can do that. That reduces the noise
level in the patch and makes it easier to understand the meaningful
changes.

2. Produce an SGML docs page that describes how this works, what the
limitations and tradeoffs are. "Reliability & the WAL" could use an
extra section2 header called Checksums (wal.sgml). This is essential
for users AND reviewers to ensure everybody has understood this (heck,
I can't remember everything about this either...)

3. I think we need an explicit test of this feature (as you describe
above), rather than manual testing. corruptiontester?

4. We need some general performance testing to show whether this is
insane or not.

But this looks in good shape for commit otherwise.

--
Simon Riggs http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services