From: | Peter Geoghegan <pg(at)bowt(dot)ie> |
---|---|
To: | Robert Haas <robertmhaas(at)gmail(dot)com> |
Cc: | "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: pgsql: amcheck: Fix verify_heapam for tuples where xmin or xmax is 0. |
Date: | 2023-03-27 18:34:16 |
Message-ID: | CAH2-WzmD0qbJpFNihJ5t-ESZBA5j6ukmO=VG1POG=nE_O82h-g@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-committers pgsql-hackers |
On Mon, Mar 27, 2023 at 10:17 AM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > What about aborted speculative insertions? See
> > heap_abort_speculative(), which directly sets the speculatively
> > inserted heap tuple's xmin to InvalidTransactionId/zero.
>
> Oh, dear. I didn't know about that case.
A big benefit of having extensive amcheck coverage is that it
effectively centralizes information about the on-disk format, in an
easy to understand way, and (over time) puts things on a more rigorous
footing. Now it'll be a lot harder for somebody else to overlook that
case in the future, which is good. Things are trending in the right
direction.
> > It probably does make sense to keep something close to this check --
> > it just needs to account for speculative insertions to avoid false
> > positive reports of corruption. We could perform cross-checks against
> > a tuple whose xmin is InvalidTransactionId/zero to verify that it
> > really is from an aborted speculative insertion, to the extent that
> > that's possible. For example, such a tuple can't be a heap-only tuple,
> > and it can't have any xmax value other than InvalidTransactionId/zero.
>
> Since this was back-patched, I think it's probably better to just
> remove the error. We can introduce new validation if we want, but that
> should probably be master-only.
That makes sense.
I don't think that it's particularly likely that having refined
aborted speculative insertion amcheck coverage will make a critical
difference to any user, at any time. But "amcheck as documentation of
the on-disk format" is reason enough to have it.
--
Peter Geoghegan
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2023-03-27 19:04:29 | pgsql: Reject attempts to alter composite types used in indexes. |
Previous Message | Robert Haas | 2023-03-27 17:41:32 | pgsql: amcheck: Generalize one of the recently-added update chain check |
From | Date | Subject | |
---|---|---|---|
Next Message | Alvaro Herrera | 2023-03-27 18:54:56 | Re: SQL/JSON revisited |
Previous Message | Melanie Plageman | 2023-03-27 18:12:03 | Re: Should vacuum process config file reload more often |