Re: TRAP: FailedAssertion("HaveRegisteredOrActiveSnapshot()", File: "toast_internals.c", Line: 670, PID: 19403)

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Thu, Apr 14, 2022 at 3:05 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> If you don't register it, then you need to also make sure that it's
>> destroyed whenever that older snapshot is.

> Well, if that's true, then I agree that it's a good argument against
> that approach. But I guess I'm confused as to why we'd end up in that
> situation. Suppose we do these two things:

> 1. Decree that SnapshotResetXmin calls InvalidateCatalogSnapshot. It's
> the other way around right now, but that's only because we're
> registering the catalog snapshot.
> 2. Bomb out in GetCatalogSnapshot if you don't have an active or
> registered snapshot already.

> Is there some reason we'd need any more infrastructure than that?

Yes.

1. Create snapshot 1 (beginning of transaction).
2. Create catalog snapshot (okay because of snapshot 1).
3. Create snapshot 2.
4. Destroy snapshot 1.
5. Catalog snapshot is still there and is now the oldest.

The implementation you propose would have to also forbid this sequence
of events, which is (a) difficult and (b) would add instability to the
system, since there's really no reason that this should be Not OK.

I'm basically not on board with adding complication to make the system
less performant and more brittle, and I don't see how the direction
you want to go isn't that.

(BTW, this thought experiment also puts a hole in the test added by
277692220: even if HaveRegisteredOrActiveSnapshot were doing what
it claims to do, it would allow use of the catalog snapshot for
detoasting after step 4, which I suppose is not what Andres intended.)

regards, tom lane