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

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com>
Cc: Peter Geoghegan <pg(at)bowt(dot)ie>, 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-06 18:32:03
Message-ID: CA+TgmobtAq9sNa0Z4QNx0xiP4NAobuGOnOirTWdJajkTRFvXNw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On Wed, Oct 6, 2021 at 1:57 PM Mark Dilger <mark(dot)dilger(at)enterprisedb(dot)com> wrote:
> > To me #2 sounds like a tautology. It could almost be phrased as
> > "pg_amcheck does not check the things that it cannot check".
>
> I totally disagree. It is uncomfortable to me that `pg_amcheck --parent-check` will now silently not perform the parent check that was explicitly requested. That reported an error before, and now it just downgrades the check. This is hardly tautological. I'm only willing to post a patch with that change because I can see a practical argument that somebody might run that as a cron job and they don't want the cron job failing when the database happens to go into recovery. But again, not at all tautological.

Yeah, I don't think that's OK. -1 from me on making any such change.
If I say pg_amcheck --heapallindexed, I expect it to pass
heapallindexed = true to bt_index_check(). I don't expect it to make a
decision internally whether I really meant it when I said I wanted
--heapallindexed checking.

All of the decisions we're talking about here really have to do with
determining the user's intent. I think that if the user says
pg_amcheck --all, there's a good argument that they don't want us to
check unlogged relations on a standby which will never be valid, or
failed index builds which need not be valid. But even that is not
necessarily true. If the user typed pg_amcheck -i
some_index_that_failed_to_build, there is a pretty strong argument
that they want us to check that index and maybe fail, not skip
checking that index and report success without doing anything. I think
it's reasonable to accept that unfortunate deviation from the user's
intent in order to get the benefit of not failing for silly reasons
when, as will normally be the case, somebody just tries to check the
entire database, or some subset of tables and their corresponding
indexes. In those cases the user pretty clearly only wants to check
the valid things. So I agree, with some reservations, that excluding
unlogged relations while in recovery and invalid indexes is probably
the thing which is most likely to give the users what they want.

But how can we possibly say that a user who specifies --heapallindexed
doesn't really mean what they said?

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

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Peter Geoghegan 2021-10-06 18:55:59 Re: BUG #17212: pg_amcheck fails on checking temporary relations
Previous Message Peter Geoghegan 2021-10-06 18:20:08 Re: BUG #17212: pg_amcheck fails on checking temporary relations

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2021-10-06 18:48:10 Re: Running tests under valgrind is getting slower at an alarming pace
Previous Message Mark Dilger 2021-10-06 18:29:48 Re: Role Self-Administration