pg.ddx.io pgsql-hackers@postgresql.org mailing list archive
help / color / mirror / Atom feed From: Mark Dilger <mark.dilger@enterprisedb.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Peter Geoghegan <pg@bowt.ie>
Cc: Andres Freund <andres@anarazel.de>
Cc: PostgreSQL-development <pgsql-hackers@postgresql.org>
Subject: Re: new heapcheck contrib module
Date: Thu, 30 Jul 2020 13:18:01 -0700
Message-ID: <DCD52DE8-C9EF-4C10-BFCC-5293E4742C30@enterprisedb.com> (raw )
In-Reply-To: <CA+TgmoaX7sPUKyHf+1bec_OJgDhvPBw_9Jt9XbEAPxhcF1YVVw@mail.gmail.com >
References: <12ED3DA8-25F0-4B68-937D-D907CFBF08E7@enterprisedb.com >
<20200420194210.sevnpi4lyghoanxi@alap3.anarazel.de >
<23E60C3C-C20A-4508-84AC-E0CEA59E784A@enterprisedb.com >
<CA+TgmobAm8vB1MNQ1taO1et+HxTN8ULnQvEG+grNDqNQTBjbtA@mail.gmail.com >
<F845AC50-A479-43F3-ADFB-861A4D2AD287@enterprisedb.com >
<5F743835-3399-419C-8324-2D424237E999@enterprisedb.com >
<CAFiTN-syyHc3jZoou51v0SR8z0POoNfktqEO6MaGig4YS8mosA@mail.gmail.com >
<E6F4258A-B661-4F2C-8A35-278C9436930A@enterprisedb.com >
<CAFiTN-s-VX3DLfqv7OOo0zGCTOZ3xVU9UZ6GgzBZdJTPNnYFNg@mail.gmail.com >
<885D2EC9-7D06-4903-A17D-72C184A1E1C5@enterprisedb.com >
<CAFiTN-uYg_PiZ854=5R-D9skratapAHKhaDAG7fhJv-Vn5UhaQ@mail.gmail.com >
<5CC85F28-F210-47AC-8DA8-EC4E82AF3FD5@enterprisedb.com >
<CAFiTN-sP5iFJGERTq4a4OLTCzjZKhLC5P6_k2bxQKXnuvdmzGA@mail.gmail.com >
<CAFiTN-tTg0R5AHvXC1ro9XphroX1wUtc5t91hU_+7DeGTpE5VQ@mail.gmail.com >
<0C0349EB-DD85-4DA8-82C7-DF70AB714592@enterprisedb.com >
<CAFiTN-uD3WSeggU20yU6voGou6BDb-cQcfkE8B1rAzWKZVK-Bw@mail.gmail.com >
<BA6E870F-241C-4DDB-B92D-DB39E679C7D4@enterprisedb.com >
<CA+TgmobQUdfM2Q7WAR606RR2YehCZ5GHRKBc-VWfRj-1ZUKqDA@mail.gmail.com >
<C4A3E5AE-CBB2-4E3F-8BAE-29EE216762C2@enterprisedb.com >
<CA+TgmoaX7sPUKyHf+1bec_OJgDhvPBw_9Jt9XbEAPxhcF1YVVw@mail.gmail.com >
> On Jul 29, 2020, at 12:52 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Mon, Jul 20, 2020 at 5:02 PM Mark Dilger
> <mark.dilger@enterprisedb.com> wrote:
>> I've made the options 'all-visible', 'all-frozen', and 'none'. It defaults to 'none'.
>
> That looks nice.
>
>>> I guess that
>>> could still be expensive if there's a lot of them, but needing
>>> ShareUpdateExclusiveLock rather than only AccessShareLock is a little
>>> unfortunate.
>>
>> I welcome strategies that would allow for taking a lesser lock.
>
> I guess I'm not seeing why you need any particular strategy here. Say
> that at the beginning you note the starting relfrozenxid of the table
> -- I think I would lean toward just ignoring datfrozenxid and the
> cluster-wide value completely. You also note the current value of the
> transaction ID counter. Those are the two ends of the acceptable
> range.
>
> Let's first consider the oldest acceptable XID, bounded by
> relfrozenxid. If you see a value that is older than the relfrozenxid
> value that you noted at the start, it is definitely invalid. If you
> see a newer value, it could still be older than the table's current
> relfrozenxid, but that doesn't seem very worrisome. If the user
> vacuumed the table while they were running this tool, they can always
> run the tool again afterward if they wish. Forcing the vacuum to wait
> by taking ShareUpdateExclusiveLock doesn't actually solve anything
> anyway: you STILL won't notice any problems the vacuum introduces, and
> in fact you are now GUARANTEED not to notice them, plus now the vacuum
> happens later.
>
> Now let's consider the newest acceptable XID, bounded by the value of
> the transaction ID counter. Any time you see a newer XID than the last
> value of the transaction ID counter that you observed, you go observe
> it again. If the value from the table still looks invalid, then you
> complain about it. Either way, you remember the new observation and
> check future tuples against that value. I think the patch is already
> doing this anyway; if it weren't, you'd need an even stronger lock,
> one sufficient to prevent any insert/update/delete activity on the
> table altogether.
>
> Maybe I'm just being dense here -- exactly what problem are you worried about?
Per tuple, tuple_is_visible() potentially checks whether the xmin or xmax committed via TransactionIdDidCommit. I am worried about concurrent truncation of clog entries causing I/O errors on SLRU lookup when performing that check. The three strategies I had for dealing with that were taking the XactTruncationLock (formerly known as CLogTruncationLock, for those reading this thread from the beginning), locking out vacuum, and the idea upthread from Andres about setting PROC_IN_VACUUM and such. Maybe I'm being dense and don't need to worry about this. But I haven't convinced myself of that, yet.
—
Mark Dilger
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
view thread (218+ messages) latest in thread
Message-ID: <DCD52DE8-C9EF-4C10-BFCC-5293E4742C30@enterprisedb.com>
Permalink: ../DCD52DE8-C9EF-4C10-BFCC-5293E4742C30@enterprisedb.com/
Also on: postgresql.org/message-id/DCD52DE8-C9EF-4C10-BFCC-5293E4742C30@enterprisedb.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-hackers@postgresql.org
Cc: mark.dilger@enterprisedb.com, robertmhaas@gmail.com, dilipbalaut@gmail.com, pg@bowt.ie, andres@anarazel.de
Subject: Re: new heapcheck contrib module
In-Reply-To: <DCD52DE8-C9EF-4C10-BFCC-5293E4742C30@enterprisedb.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