Re: [BUG] failed assertion in EnsurePortalSnapshotExists()

Tom Lane <tgl@sss.pgh.pa.us>

From: Tom Lane <tgl@sss.pgh.pa.us>
To: "Drouvot, Bertrand" <bdrouvot@amazon.com>
Cc: Alvaro Herrera <alvherre@2ndquadrant.com>, Ranier Vilela <ranier.vf@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2021-09-30T17:16:22Z
Lists: pgsql-hackers

Attachments

"Drouvot, Bertrand" <bdrouvot@amazon.com> writes:
> [ v2-0003-EnsurePortalSnapshotExists-failed-assertion.patch ]

Looking through this, I think you were overenthusiastic about applying
PushActiveSnapshotWithLevel.  We don't really need to use it except in
the places where we're setting portalSnapshot, because other than those
cases we don't have a risk of portalSnapshot becoming a dangling pointer.
Also, I'm quite nervous about applying PushActiveSnapshotWithLevel to
snapshots that aren't created by the portal machinery itself, because
we don't know very much about where passed-in snapshots came from or
what the caller thinks their lifespan is.

The attached revision therefore backs off to only using the new code
in the two places where we really need it.  I made a number of
more-cosmetic revisions too.  Notably, I think it's useful to frame
the testing shortcoming as "we were not testing COMMIT/ROLLBACK
inside a plpgsql exception block".  So I moved the test code to the
plpgsql tests and made it check ROLLBACK too.

			regards, tom lane

PS: Memo to self: in the back branches, the new field has to be
added at the end of struct Portal.

Commits

  1. Fix Portal snapshot tracking to handle subtransactions properly.