Hi,
On 2022-03-10 17:38:10 -0800, Peter Geoghegan wrote:
> On Thu, Mar 10, 2022 at 4:50 PM Andres Freund <andres@anarazel.de> wrote:
> > On 2022-03-07 10:44:23 +0100, hubert depesz lubaczewski wrote:
> > > TRAP: FailedAssertion("HaveRegisteredOrActiveSnapshot()", File: "toast_internals.c", Line: 670, PID: 526595)
>
> > That's a new assertion, but I put it in to detect bugs like what's fixed in
> > 7c38ef2a5d6cf6d8dc3834399d7a1c364d64ce64. Looks like something in the snapshot
> > mangement of cursors is busted...
> >
> > Any chance you could try to come up with a reproducer?
>
> Although it isn't related to this bug, I thought I'd still ask: where
> else we could add "Assert(HaveRegisteredOrActiveSnapshot())" like
> this, to get similar protection?
Probably in a bunch of places, yes.
> Isn't this exactly the kind of
> assertion that is contemplated by comments in GlobalVisTestFor() that
> you wrote (for the snapshot scalability work)?
IIRC we can't easily there [yet], because we're incredibly sloppy around
snapshots for catalog accesses in a number of places. I started a thread and
it didn't seem anybody was too concerned. I probably should pick it up again.
Another example of complete brokenness in the wider area is catalog accesses
happening after PROC_IN_VACUUM is set. Afaict it's entirely incorrect to do
any catalog accesses once set, because rows that are still needed for the
catalog access can just be removed.
Greetings,
Andres Freund
