Re: BUG #17212: pg_amcheck fails on checking temporary relations

From: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: Robert Haas <robertmhaas(at)gmail(dot)com>, Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com>, Alexander Lakhin <exclusion(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: BUG #17212: pg_amcheck fails on checking temporary relations
Date: 2021-10-12 02:22:14
Message-ID: C26F0D76-7C5E-43FD-810C-32459B1104EC@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

> On Oct 11, 2021, at 5:37 PM, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
> Cool. I pushed just the amcheck changes a moment ago. I attach the
> remaining changes from your v3, with a new draft commit message (no
> real changes). I didn't push the rest (what remains in the attached
> revision) just yet because I'm not quite sure about the approach used
> to exclude temp tables.

Thanks for that.

> Do we really need the redundancy between prepare_btree_command(),
> prepare_heap_command(), and compile_relation_list_one_db()? All three
> exclude temp relations, plus you have extra stuff in
> prepare_btree_command(). There is some theoretical value in delaying
> the index specific stuff until the query actually runs, at least in
> theory. But it also seems unlikely to make any appreciable difference
> to the overall level of coverage in practice.

I agree that it is unlikely to make much difference in practice. Another session running reindex concurrently is, I think, the most likely to conflict, but it is just barely imaginable that a relation will be dropped, and its OID reused for something unrelated, by the time the check command gets run. The new object might be temporary where the old object was not. On a properly functioning database, that may be too remote a possibility to be worth worrying about, but on a corrupt database, most bets are off, and I can't really tell you if that's a likely scenario, because it is hard to think about all the different ways corruption might cause a database to behave. On the other hand, the join against pg_class might fail due to unspecified corruption, so my attempt to play it safe may backfire.

I don't feel strongly about this. If you'd like me to remove those checks, I can do so. These are just my thoughts on the subject.

> Would it be simpler to do it all together, in
> compile_relation_list_one_db()? Were you concerned about things
> changing when parallel workers are run? Or something else?

Yeah, I was contemplating things changing by the time the parallel workers run the command. I don't know how important that is.


Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Suhonen Reijo (Valtori) 2021-10-12 07:47:12 VS: BUG #17218: Cluster recovery is not working properly.
Previous Message Zhihong Zhang 2021-10-12 00:48:54 Re: Epoch from age is incorrect

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2021-10-12 02:33:10 Re: pg14 psql broke \d datname.nspname.relname
Previous Message Michael Paquier 2021-10-12 02:20:31 Re: More business with $Test::Builder::Level in the TAP tests