Re: [BUG] failed assertion in EnsurePortalSnapshotExists()

On 2021-Sep-28, Tom Lane wrote:

> 1. Provide a variant of PushActiveSnapshot that allows the caller
> to specify the as_level to use, and then have
> EnsurePortalSnapshotExists, as well as other places that create
> Portal-associated snapshots, use this with as_level equal to the
> Portal's createSubid. The main hazard I see here is that this could
> theoretically allow the ActiveSnapshot stack to end up with
> out-of-order as_level values, causing issues. For the moment we
> could probably just add assertions to check that the passed as_level
> is >= next level, or maybe even that this variant is only called with
> empty ActiveSnapshot stack.

I don't see anything wrong with this idea offhand.

I didn't try to create scenarios with out-of-order as_level active
snapshots, but with all the creativity out there in the world, and
considering that it's possible to write procedures in C, I think that
asserting that the order is maintained is warranted.

Now if we do meet a case with out-of-order levels, what do we do? I
suppose we'd need to update AtSubCommit_Snapshot and AtSubAbort_Snapshot
to cope with that (but by all means let's wait until we have a test case
where that happens ...)

> 2. Provide a way for AtSubAbort_Portals to detect whether a
> portalSnapshot pointer points to a snap that's going to be
> deleted by AtSubAbort_Snapshot, and then just have it clear any
> portalSnapshots that are about to become dangling. (This'd amount
> to accepting the possibility that portalSnapshot is of a different
> subxact level from the portal, and dealing with the situation.)
>
> I initially thought #2 would be the way to go, but it turns out
> to be a bit messy since what we have is a Snapshot pointer not an
> ActiveSnapshotElt pointer. We'd have to do something like search the
> ActiveSnapshot stack looking for pointer equality to the caller's
> Snapshot pointer, which seems fragile --- do we assume as_snap is
> unique for any other purpose?

I don't remember what patch it was, but I remember contemplating
sometime during the past year the possibility of snapshots being used
twice in the active stack. Now maybe this is not possible in practice
because in most cases we create a copy, but I couldn't swear that that's
always the case. I wouldn't rely on that.

--
Álvaro Herrera Valdivia, Chile — https://www.EnterpriseDB.com/
"Las cosas son buenas o malas segun las hace nuestra opinión" (Lisias)