Re: Potential G2-item cycles under serializable isolation

Thomas Munro <thomas.munro@gmail.com>

From: Thomas Munro <thomas.munro@gmail.com>
To: Peter Geoghegan <pg@bowt.ie>
Cc: Andres Freund <andres@anarazel.de>, Kevin Grittner <kgrittn@gmail.com>, Jeff Davis <pgsql@j-davis.com>, PostgreSQL mailing lists <pgsql-bugs@lists.postgresql.org>, Kyle Kingsbury <aphyr@jepsen.io>
Date: 2020-06-11T03:16:13Z
Lists: pgsql-bugs
On Thu, Jun 11, 2020 at 1:10 PM Peter Geoghegan <pg@bowt.ie> wrote:
> Attached is v3, which has an isolationtester test.

Nice repro.  We have:

foo happened before bar, because it can't see bar_insert
bar happened before foo, because it can't see foo_insert (!)

Confirmed here that this anomaly is missed in master and detected with
the patch as shown, and check-world passes.

Explaining it to myself like I'm five:

The tuple foo_insert was concurrently inserted *and* deleted/updated.
Our coding screwed that case up.  Your fix teaches the
HEAPTUPLE_DELETE_IN_PROGRESS case to check if it's also an insert we
can't see, and then handles it the same as non-visible HEAPTUPLE_LIVE
or HEAPTUPLE_INSERT_IN_PROGRESS.  That is, by taking the xmin
(inserter), not the xmax (updater/deleter) for conflict-out checking.
(As mentioned on IM, I wondered for a bit why it's OK that we now lack
a check for conflict out to the deleter in the insert+delete case, but
I think that's OK because it certainly happened after the insert
committed.)

Then you realised that HEAPTUPLE_RECENTLY_DEAD has no reason to be
treated differently.

Minor nits (already mentioned via IM but FTR): bar_first isn't
necessary in the test, s/foo-insert/foo_insert/

Looks pretty good to me.  Tricky problem, really excellent work.



Commits

  1. Doc: Add references for SI and SSI.

  2. Improve comments for [Heap]CheckForSerializableConflictOut().

  3. Avoid update conflict out serialization anomalies.

  4. Implement genuine serializable isolation level.