Re: pgsql: Remove the restriction that the relmap must be 512 bytes.

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pgsql: Remove the restriction that the relmap must be 512 bytes.
Date: 2022-07-27 19:39:03
Message-ID: CA+TgmoZ4PLbVX0xrxf+Vg9+-btv+_7G6WoSwQby4TPZ+JDexJQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On Wed, Jul 27, 2022 at 3:17 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
> >> Hmm, but that doesn't actually produce nice behavior. It just goes
> >> into an infinite loop, like this:
> >> So now I'm back to being unsure what to do here.
>
> > I vote to go for the catversion bump for now.
>
> What this is showing us is that any form of corruption in the relmapper
> file causes very unpleasant behavior. We probably had better do something
> about that, independently of this issue.

I'm not sure how important that is, but it certainly wouldn't hurt.

> In the meantime, I still think bumping the file magic number is a better
> answer. It won't make the behavior any worse for un-updated code than
> it is already.

But it also won't make it any better, so why even bother? The goal of
catversion bumps is to replace crashes or unpredictable misbehavior
with a nice error message that tells you exactly what the problem is.
Here we'd just be replacing an infinite series of crashes with an
infinite series of crashes with a slightly different error message.
It's probably worth comparing those error messages:

FATAL: could not read file "global/pg_filenode.map": read 512 of 524
FATAL: relation mapping file "global/pg_filenode.map" contains invalid data

The first message is what you get now. The second message is what you
get with the proposed change to the magic number. I would argue that
the second message is actually worse than the first one, because the
first one actually gives you some hint what the problem is, whereas
the second one really just says that an unspecified bad thing
happened.

In short, I think Alvaro's idea is unprincipled but solves the problem
of making it clear that a new initdb is required. Your idea is
principled but does not solve any problem.

--
Robert Haas
EDB: http://www.enterprisedb.com

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2022-07-27 20:05:16 Re: pgsql: Remove the restriction that the relmap must be 512 bytes.
Previous Message Tom Lane 2022-07-27 19:17:36 Re: pgsql: Remove the restriction that the relmap must be 512 bytes.

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2022-07-27 19:59:18 Re: Schema variables - new implementation for Postgres 15
Previous Message Tom Lane 2022-07-27 19:36:26 Re: Allow pageinspect's bt_page_stats function to return a set of rows instead of a single row