From: Heikki Linnakangas <hlinnaka@iki.fi>
To: Thomas Munro <thomas.munro@gmail.com>
To: Dmitry Dolgov <9erthalion6@gmail.com>
Cc: artem.anisimov.255@gmail.com, pgsql-bugs@lists.postgresql.org, Teodor Sigaev <teodor@sigaev.ru>
Subject: Re: BUG #17949: Adding an index introduces serialisation anomalies.
Date: Mon, 26 Jun 2023 11:04:26 +0300
Message-ID: <4638fef6-ea51-0e0e-d463-d5269575b021@iki.fi> (raw)
In-Reply-To: <CA+hUKGKmR57CgZAPRjeSQfXb_oxjSUx7673SqDRw9RF2FfCw=A@mail.gmail.com>
References: <CA+hUKG+zAbVgU91ZjkaCd=1AFOaMRbOp0=E9DP0BwippqqGx5g@mail.gmail.com>
<CA+hUKG+3B7uBkJd4rr2FXhTc+95F3RvRovbhBM2UDt68aL5a-g@mail.gmail.com>
<CA+hUKGJATT51T0Vkg0FDkQa5boQv-WoqL1_KM=313ySu0MVqig@mail.gmail.com>
<20230619121757.gjvuvypp25ngwkux@ddolgov.remote.csb>
<CA+hUKGJP3g6PF4vES0X0zy34uuSvHxHUhoq65A_WtzqxPpJ_6g@mail.gmail.com>
<CA+hUKGJQ5ijziJtnrZfo8-8dmfiCU8nJBTnpkVweBp2krN_6Ng@mail.gmail.com>
<CA+hUKGKRD5UM6mEDkre-yhm9Smr4ampf-5=NeFkAqi-jRt4RYQ@mail.gmail.com>
<20230621090421.4rr55ybndttng7xl@ddolgov.remote.csb>
<CA+hUKGJKTCGShnj0ZRhHsmOuHtZ-udZu8jN+GJ2-7YCSGnDYzg@mail.gmail.com>
<20230623140542.5tbeg5ikz3cupag3@ddolgov.remote.csb>
<20230624135910.zufuollwp57otawo@ddolgov.remote.csb>
<CA+hUKGKmR57CgZAPRjeSQfXb_oxjSUx7673SqDRw9RF2FfCw=A@mail.gmail.com>
On 26/06/2023 03:25, Thomas Munro wrote:
> On Sun, Jun 25, 2023 at 1:59 AM Dmitry Dolgov <9erthalion6@gmail.com> wrote:
>> I've managed to resolve it, or at least reduce the chances for the issue to
>> appear, via semi-randomly adding more CheckForSerializableConflictIn /
>> PredicateLock around the new sublist that has to be created in
>> ginHeapTupleFastInsert. I haven't seen the reproducer failing with this
>> changeset after running it multiple times for a couple of minutes, where on the
>> main branch, with the two fixes from Thomas included, it was failing within a
>> couple of seconds.
>
> Ahh, right, thanks. I don't think we need to lock all those pages as
> you showed, as this whole "fast" path is supposed to be covered by the
> meta page (in other words, GIN is expected to have the highest
> possible serialisation failure rate under SSI unless you turn fast
> updates off). But there is an ordering bug with the existing
> predicate lock in that code, which allows this to happen:
>
> S1: CheckForSerializableConflictIn(meta)
>
> S2: PredicateLockPage(meta)
> S2: scan, find no tuples
>
> S1: BufferLock(EXCLUSIVE
> S1: modify stuff...
>
> CheckForSerializableConflictIn() was written with the assumption that
> you're inserting a tuple (ie you have the page containing the tuple
> locked), so you'll either conflict with a reader who already has a
> predicate lock at that point OR you'll insert first and then the
> reader will see your (invisible-to-snapshot) tuples, but here we're
> doing some fancy footwork with a meta page, and we screwed up the
> ordering and left a window where neither of those things happens.
> Perhaps it was coded that way because there is drop-then-reacquire
> dance, but it's easy enough to move the check in both branches. Does
> that make sense?
Yes, +1 on the patches. Any chance of constructing test cases for these?
The above race condition is hard to reach, but some of these other bugs
seem more testable.
Some minor nits: In
v3-0001-Fix-race-in-SSI-interaction-with-empty-btrees.patch:
> /*
> - * We only get here if the index is completely empty. Lock relation
> - * because nothing finer to lock exists.
> + * Since we have no pages locked, it's possible for another
> + * transaction to insert data between _bt_search() and
> + * PredicateLockRelation(). We have to try again after taking a
> + * relation-level predicate lock, to close a narrow window where we
> + * wouldn't scan concurrently inserted tuples, but the writer wouldn't
> + * see our predicate lock.
> */
I'd like to keep the old comment here, it's good context, and add the
new text in addition to the old.
v3-0002-Fix-race-in-SSI-interaction-with-bitmap-heap-scan.patch: Can we
keep the optimization when not using SSI?
--
Heikki Linnakangas
Neon (https://neon.tech)
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: hlinnaka@iki.fi, thomas.munro@gmail.com, 9erthalion6@gmail.com, teodor@sigaev.ru
Subject: Re: BUG #17949: Adding an index introduces serialisation anomalies.
In-Reply-To: <4638fef6-ea51-0e0e-d463-d5269575b021@iki.fi>
* 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