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
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 |
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 |