public inbox for [email protected]
help / color / mirror / Atom feedFrom: Andres Freund <[email protected]>
To: [email protected]
To: Peter Geoghegan <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Masahiko Sawada <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations
Date: Sat, 19 Feb 2022 16:12:31 -0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <CAH2-WzkiB-qcsBmWrpzP0nxvrQExoUts1d7TYShg_DrkOHeg4Q@mail.gmail.com>
References: <CAH2-WzmT9N_42b=yuT-B03E=R48ECA6iEvR48305CSfwg=WUjw@mail.gmail.com>
<CA+TgmoYhXCi6qWa6sU+6KFSD_Ha=PVJC7nPeXvhMF1poo2D=xg@mail.gmail.com>
<CAH2-Wz=9Jr+s-hUbq=RxW8xRR8Ygb6OjpUDOVOK4z0qZe9Xopw@mail.gmail.com>
<CA+Tgmobr_95jt2GwLoDYbE=t6ChXBvv3U48F8HJyjYUikY2Gnw@mail.gmail.com>
<CAH2-Wzn11=HUK6gmTd1EjtZmEuXOYyifojpzPaVdVrxWFufTYA@mail.gmail.com>
<CAH2-Wz=iLnf+0CsaB37efXCGMRJO1DyJw5HMzm7tp1AxG1NR2g@mail.gmail.com>
<CAH2-WzkUBbg9SSL7bFWT_Y86027GAUJEgqwgbe2YjOLEpw9ysQ@mail.gmail.com>
<CAH2-Wzk=uUHMPx3Zs41ouPBgxX-5BbwSuPeOBLyKt3y5jt-sgw@mail.gmail.com>
<CA+TgmoZMKKO_dGBhEzZ5T8jbmL+3kxSuLoQx_pyDfHKutiifCg@mail.gmail.com>
<CAH2-WzmfyfmmANV6RRchHmUBgjU9QYBUP_6CoFuQ3d23T4E6nQ@mail.gmail.com>
<[email protected]>
<CAH2-Wz=DHb2hXGqcvuVyy0qC88VcYoaNjy2FFtb-xjQMOZTQqw@mail.gmail.com>
<CAH2-WzkiB-qcsBmWrpzP0nxvrQExoUts1d7TYShg_DrkOHeg4Q@mail.gmail.com>
Hi,
(On phone, so crappy formatting and no source access)
On February 19, 2022 3:08:41 PM PST, Peter Geoghegan <[email protected]> wrote:
>On Fri, Feb 18, 2022 at 5:00 PM Peter Geoghegan <[email protected]> wrote:
>> Another testing strategy occurs to me: we could stress-test the
>> implementation by simulating an environment where the no-cleanup-lock
>> path is hit an unusually large number of times, possibly a fixed
>> percentage of the time (like 1%, 5%), say by making vacuumlazy.c's
>> ConditionalLockBufferForCleanup() call return false randomly. Now that
>> we have lazy_scan_noprune for the no-cleanup-lock path (which is as
>> similar to the regular lazy_scan_prune path as possible), I wouldn't
>> expect this ConditionalLockBufferForCleanup() testing gizmo to be too
>> disruptive.
>
>I tried this out, using the attached patch. It was quite interesting,
>even when run against HEAD. I think that I might have found a bug on
>HEAD, though I'm not really sure.
>
>If you modify the patch to simulate conditions under which
>ConditionalLockBufferForCleanup() fails about 2% of the time, you get
>much better coverage of lazy_scan_noprune/heap_tuple_needs_freeze,
>without it being so aggressive as to make "make check-world" fail --
>which is exactly what I expected. If you are much more aggressive
>about it, and make it 50% instead (which you can get just by using the
>patch as written), then some tests will fail, mostly for reasons that
>aren't surprising or interesting (e.g. plan changes). This is also
>what I'd have guessed would happen.
>
>However, it gets more interesting. One thing that I did not expect to
>happen at all also happened (with the current 50% rate of simulated
>ConditionalLockBufferForCleanup() failure from the patch): if I run
>"make check" from the pg_surgery directory, then the Postgres backend
>gets stuck in an infinite loop inside lazy_scan_prune, which has been
>a symptom of several tricky bugs in the past year (not every time, but
>usually). Specifically, the VACUUM statement launched by the SQL
>command "vacuum freeze htab2;" from the file
>contrib/pg_surgery/sql/heap_surgery.sql, at line 54 leads to this
>misbehavior.
>This is a temp table, which is a choice made by the tests specifically
>because they need to "use a temp table so that vacuum behavior doesn't
>depend on global xmin". This is convenient way of avoiding spurious
>regression tests failures (e.g. from autoanalyze), and relies on the
>GlobalVisTempRels behavior established by Andres' 2020 bugfix commit
>94bc27b5.
We don't have a blocking path for cleanup locks of temporary buffers IIRC (normally not reachable). So I wouldn't be surprised if a cleanup lock failing would cause some odd behavior.
>It's quite possible that this is nothing more than a bug in my
>adversarial gizmo patch -- since I don't think that
>ConditionalLockBufferForCleanup() can ever fail with a temp buffer
>(though even that's not completely clear right now). Even if the
>behavior that I saw does not indicate a bug on HEAD, it still seems
>informative. At the very least, it wouldn't hurt to Assert() that the
>target table isn't a temp table inside lazy_scan_noprune, documenting
>our assumptions around temp tables and
>ConditionalLockBufferForCleanup().
Definitely worth looking into more.
This reminds me of a recent thing I noticed in the aio patch. Spgist can end up busy looping when buffers are locked, instead of blocking. Not actually related, of course.
Andres
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
view thread (20+ messages) latest in thread
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Removing more vacuumlazy.c special cases, relfrozenxid optimizations
In-Reply-To: <[email protected]>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox