Re: [BUG] failed assertion in EnsurePortalSnapshotExists()

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: "Drouvot, Bertrand" <bdrouvot(at)amazon(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Ranier Vilela <ranier(dot)vf(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: [BUG] failed assertion in EnsurePortalSnapshotExists()
Date: 2021-09-30 17:16:22
Message-ID: 3135025.1633022182@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

"Drouvot, Bertrand" <bdrouvot(at)amazon(dot)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.

Attachment Content-Type Size
v3-EnsurePortalSnapshotExists-failed-assertion.patch text/x-diff 8.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jaime Casanova 2021-09-30 17:25:07 Re: Numeric x^y for negative x
Previous Message Jaime Casanova 2021-09-30 16:45:34 Re: Diagnostic comment in LogicalIncreaseXminForSlot