From: | "Drouvot, Bertrand" <bdrouvot(at)amazon(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
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 18:07:01 |
Message-ID: | d66da5f7-eb6c-4753-84a2-0c3811f93e5a@amazon.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 9/30/21 7:16 PM, Tom Lane wrote:
> "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.
Oh right, I did not think about it, thanks!
>
> The attached revision therefore backs off to only using the new code
> in the two places where we really need it.
Ok, so in PortalRunUtility() and EnsurePortalSnapshotExists().
> I made a number of
> more-cosmetic revisions too.
thanks!
> 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.
Indeed, makes sense.
>
> regards, tom lane
>
> PS: Memo to self: in the back branches, the new field has to be
> added at the end of struct Portal.
out of curiosity, why?
Thanks
Bertrand
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2021-09-30 18:17:16 | Re: [EXTERNAL] Re: Add ETIMEDOUT to ALL_CONNECTION_FAILURE_ERRNOS |
Previous Message | James Coleman | 2021-09-30 17:37:44 | Re: Document spaces in .pgpass need to be escaped |