Re: Online enabling of checksums

Daniel Gustafsson <daniel@yesql.se>

From: Daniel Gustafsson <daniel@yesql.se>
To: Michael Banck <michael.banck@credativ.de>
Cc: Magnus Hagander <magnus@hagander.net>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, Robert Haas <robertmhaas@gmail.com>, Greg Stark <stark@mit.edu>, PostgreSQL-development <pgsql-hackers@postgresql.org>
Date: 2018-03-04T22:30:26Z
Lists: pgsql-hackers
> On 04 Mar 2018, at 15:24, Michael Banck <michael.banck@credativ.de> wrote:

>> +		csum = pg_checksum_page(buf, blockno + segmentno*RELSEG_SIZE);
>> +		if (csum != header->pd_checksum)
>> +		{
>> +			if (ControlFile->data_checksum_version == PG_DATA_CHECKSUM_VERSION)
>> +				fprintf(stderr, _("%s: %s, block %d, invalid checksum in file %X, calculated %X\n"),
>> +						progname, fn, blockno, header->pd_checksum, csum);
> 
> The error message sounds a bit strange to me, I would expect the
> filename after "in file [...]", but you print the expected checksum.
> Also, 'invalid' sounds  a bit like we found something which is malformed
> checksum (no hex), so maybe "checksum mismatch in file, expected %X,
> found %X" or something?

Agreed.  Looking at our current error messages, “in file” is conventionally
followed by the filename.  I do however think “calculated” is better than
“expected” since it conveys clearly that the compared checksum is calculated by
pg_verify_checksum and not read from somewhere.

How about something like this?

_(“%s: checksum mismatch in file \”%s\”, block %d: calculated %X, found %X”),
	progname, fn, blockno, csum, header->pd_checksum);

cheers ./daniel

Commits

  1. Online enabling and disabling of data checksums

  2. Deactive flapping checksum isolation tests.

  3. Add support for coordinating record typmods among parallel workers.