Re: Make pg_checksums complain if compiled BLCKSZ and data folder's block size differ

From: Magnus Hagander <magnus(at)hagander(dot)net>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Michael Banck <michael(dot)banck(at)credativ(dot)de>, Sergei Kornilov <sk(at)zsrv(dot)org>
Subject: Re: Make pg_checksums complain if compiled BLCKSZ and data folder's block size differ
Date: 2019-03-16 10:18:17
Message-ID: CABUevExpjqHZppyO8sOe7k4LpFPq+v3Gs--n8Y9mOqjCOqRf4g@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Sat, Mar 16, 2019 at 2:22 AM Michael Paquier <michael(at)paquier(dot)xyz> wrote:

> Hi all,
> (related folks in CC)
>
> Sergei Kornilov has reported here an issue with pg_checksums:
>
> https://www.postgresql.org/message-id/5217311552474471@myt2-66bcb87429e6.qloud-c.yandex.net
>
> If the block size the tool is compiled with does not match the data
> folder block size, then users would get incorrect checksums failures,
> which is confusing. As pg_checksum_block() uses directly the block
> size, this cannot really be made dynamic yet, so we had better issue
> an error on that. Michael Banck has sent a patch for that:
> https://www.postgresql.org/message-id/1552476561.4947.67.camel@credativ.de
>
> The error message proposed is like that:
> + if (ControlFile->blcksz != BLCKSZ)
> + {
> + fprintf(stderr, _("%s: data directory block size %d is different
> to compiled-in block size %d.\n"),
> + progname, ControlFile->blcksz, BLCKSZ);
> + exit(1);
> + }
> Still I think that we could do better.
>
> Here is a proposal of message which looks more natural to me, and more
> consistent with what xlog.c complains about:
> database files are incompatible with pg_checksums.
> The database cluster was initialized with BLCKSZ %d, but pg_checksums
> was compiled with BLCKSZ %d.
>

BLCKSZ is very much an internal term. The exposed name through pg_settings
is block_size, so I think the original was better. Combining that one with
yours into "initialized with block size %d" etc, makes it a lot nicer.

The "incompatible with pg_checksums" part may be a bit redundant with the
commandname at the start as well, as I now realized Fabien pointed out
downthread. But I would suggest just cutting it and saying "%s: database
files are incompatible" or maybe "%s: data directory is incompatible" even?

--
Magnus Hagander
Me: https://www.hagander.net/ <http://www.hagander.net/>
Work: https://www.redpill-linpro.com/ <http://www.redpill-linpro.com/>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Stephen Frost 2019-03-16 10:21:10 Re: [PATCH v20] GSSAPI encryption support
Previous Message Fabien COELHO 2019-03-16 10:13:58 Re: seems like a bug in pgbench -R