pg.ddx.io pgsql-bugs@postgresql.org mailing list archive help / color / mirror / Atom feed From: Peter Geoghegan <pg@bowt.ie>
To: Mark Dilger <mark.dilger@enterprisedb.com>
Cc: Robert Haas <robertmhaas@gmail.com>
Cc: Pavel Borisov <pashkin.elfe@gmail.com>
Cc: Alexander Lakhin <exclusion@gmail.com>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: BUG #17212: pg_amcheck fails on checking temporary relations
Date: Mon, 11 Oct 2021 11:12:07 -0700
Message-ID: <CAH2-Wzne7L9fX2LdZ9enNZpO2wuqbrsubCw2thH__CC_nZazjw@mail.gmail.com> (raw )
In-Reply-To: <40DB088B-C96F-49B2-BC84-BDC8D35C7E97@enterprisedb.com >
References: <17212-34dd4a1d6bba98bf@postgresql.org >
<CAH2-Wz=bw-Uk2yoOq7yevC9hyMGgZ2Ud34nd9YJJPXJ3pkVcrw@mail.gmail.com >
<E85855BD-C063-4B24-9EAA-02DECC8F42B9@enterprisedb.com >
<E283B45F-5745-49C9-BBC7-99876E2FABD1@enterprisedb.com >
<f42de8ec-f219-0922-93b3-37a72bba1feb@gmail.com >
<28B36E41-1E8F-4DC1-A924-711621BA4972@enterprisedb.com >
<CAH2-Wz=v4py=c79J7wpdVmQMkd8rO7gj5Ur5Dp4E0y7aQUz6cg@mail.gmail.com >
<386F66EE-DEFD-42EA-9DB2-102BF3414669@enterprisedb.com >
<CAH2-Wzkwb+1GSONMrXByHhgkzD-N59A_kFcvsoA_+M9cNNHm5g@mail.gmail.com >
<CA+TgmobJnZE6WMw1y+N9ts36nsbL_un0CYB_khieg8rOa86RBw@mail.gmail.com >
<CAH2-Wzm02e=jAarpzeZGyK8JyXTvuHCfJ9KC5svosD72nROKrw@mail.gmail.com >
<B2BDB87C-8288-436E-B3DC-8AEB92FEB8F1@enterprisedb.com >
<CAH2-Wz=WYdmbc1k8Lr2P4eQeYkh1UYvyDigzrvukejcWc2jKOA@mail.gmail.com >
<c422a1aa-81c0-0762-0971-9a026cf2aa79@gmail.com >
<CAH2-WzndLb0-GZcshTJGw6u6SQrJ5puyFiFYh-DWR8ue2zQuvA@mail.gmail.com >
<CALT9ZEGhuXiH54VgyVJ65V+ccGqPqHTduBM4saPWSD2mrD5C6Q@mail.gmail.com >
<F824CD5F-C82F-451C-AE67-9AD31447424A@enterprisedb.com >
<CAH2-WznaYWowxxFaGK6Hv=SUQYS2-r7dQZJFWyDT_W6GGtuCjA@mail.gmail.com >
<63FF68F5-683D-423E-A667-9C00441101A5@enterprisedb.com >
<CAH2-Wz=8RP6izkLF=P_bFUK-CyTsO09hQ1-ZsC9P4Qiz3CughQ@mail.gmail.com >
<561CE3DB-5CBC-44BF-9A3E-1D7FAEB123E0@enterprisedb.com >
<CA+TgmobtAq9sNa0Z4QNx0xiP4NAobuGOnOirTWdJajkTRFvXNw@mail.gmail.com >
<CAH2-WzmJkVKFSqJd2_uy07jPg-LW0yCHh0fgA8rFkdyk6FZOnQ@mail.gmail.com >
<CA+TgmoYa5exExaONEnGBWRdi7A3=FKuKzewY8DXTj6+045wiFQ@mail.gmail.com >
<CAH2-WzkwRUayddmb3uxCadV_y--DHfY+UMmjtOokiAO2xNh5fA@mail.gmail.com >
<CA+Tgmoat1M4MAqUQM1H+bkWR=wXmi33bB0YRR=xaFk9ADs5Jeg@mail.gmail.com >
<CAH2-Wzk3NzXYXsUKWBFuBm6aiXTh-_L2Dm9wPqfFenUvG+VbRw@mail.gmail.com >
<543F3BF3-EDED-4C52-AF05-623921B6C409@enterprisedb.com >
<CAH2-WzkXphRHgbaUXqMgb7SBmc_VbwdyWGYqphV6KbOH7stfxA@mail.gmail.com >
<55A2BFE7-AAAF-4A64-BD3A-5C98EA982267@enterprisedb.com >
<CAH2-WzniogVqNuWzNVLOKjkd8FBcSry29hN9=wiROUPw92fmGg@mail.gmail.com >
<07159C9A-40DA-47BF-B2F4-203F5AC00890@enterprisedb.com >
<CAH2-Wzkt-umhXfR1nqy=ZDXMfoN3LDK4W5VtchfWyRAuPWxtnA@mail.gmail.com >
<40DB088B-C96F-49B2-BC84-BDC8D35C7E97@enterprisedb.com >
On Mon, Oct 11, 2021 at 10:46 AM Mark Dilger
<mark.dilger@enterprisedb.com> wrote:
> > On Oct 11, 2021, at 10:10 AM, Peter Geoghegan <pg@bowt.ie> wrote:
> > This mostly looks good to me. Just one thing occurs to me: I suspect
> > that we don't need to call pg_is_in_recovery() from SQL at all. What's
> > wrong with just letting verify_heapam() (the C function from amcheck
> > proper) show those notice messages where appropriate?
>
> I thought a big part of the debate upthread was over exactly this point, that pg_amcheck should not attempt to check (a) temporary relations, (b) indexes that are invalid or unready, and (c) unlogged relations during recovery.
Again, I consider (a) and (b) very similar to each other, but very
dissimilar to (c). Only (a) and (b) are *inherently* not verifiable by
amcheck.
To me, giving pg_amcheck responsibility for only calling amcheck
functions when (a) and (b) are sane is akin to expecting pg_amcheck to
only call bt_index_check() with a B-Tree index. Giving pg_amcheck
these responsibilities is not a case of "pg_amcheck presuming to know
what's best for the user, or too much about amcheck", because amcheck
itself pretty clearly expects this from the user (and always has). The
user is no worse off for having used pg_amcheck rather than calling
amcheck functions from SQL themselves. pg_amcheck is literally just
fulfilling basic expectations held by amcheck, that are pretty much
documented as such.
Sure, the user might not be happy with --parent-check throwing an
error on a replica. But in practice most users won't want to do that
anyway. Even on a primary it's usually not possible as a practical
matter, because the locking implications are *bad* -- it's just too
disruptive, for too little extra coverage. And so when --parent-check
fails on a replica, it really is very likely that the user should just
not do that. Which is easy: just remove --parent-check, and try again.
Most scenarios where --parent-check is useful involve the user already
knowing that there is some corruption. In other words, scenarios where
almost nothing could be considered overkill. Presumably this is very
rare.
> I don't like having pg_amcheck parse the error message that comes back from amcheck.
> How shall we proceed?
What's the problem with just having pg_amcheck pass through the notice
to the user, without it affecting anything else? Why should a simple
notice message need to affect its return code, or anything else?
It's not like I feel very strongly about this question. Ultimately it
probably doesn't matter very much -- if pg_amcheck just can't deal
with these notice messages for some reason, then I can let it go. But
what's the reason? If there is a good reason, then maybe we should
just not have the notice messages (so we would just remove the
existing one from verify_nbtree.c, while still interpreting the case
in the same way -- index has no storage to check, and so is trivially
verified).
--
Peter Geoghegan
view thread (83+ messages) latest in thread
Message-ID: <CAH2-Wzne7L9fX2LdZ9enNZpO2wuqbrsubCw2thH__CC_nZazjw@mail.gmail.com>
Permalink: ../CAH2-Wzne7L9fX2LdZ9enNZpO2wuqbrsubCw2thH__CC_nZazjw@mail.gmail.com/
Also on: postgresql.org/message-id/CAH2-Wzne7L9fX2LdZ9enNZpO2wuqbrsubCw2thH__CC_nZazjw@mail.gmail.com
copy link · copy postgr.es
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: pg@bowt.ie, mark.dilger@enterprisedb.com, robertmhaas@gmail.com, pashkin.elfe@gmail.com, exclusion@gmail.com, pgsql-hackers@postgresql.org
Subject: Re: BUG #17212: pg_amcheck fails on checking temporary relations
In-Reply-To: <CAH2-Wzne7L9fX2LdZ9enNZpO2wuqbrsubCw2thH__CC_nZazjw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox