Re: [sqlsmith] Crash in GetOldestSnapshot() - Mailing list pgsql-hackers

From Tom Lane
Subject Re: [sqlsmith] Crash in GetOldestSnapshot()
Date
Msg-id 2850.1470592623@sss.pgh.pa.us
Whole thread Raw
In response to Re: [sqlsmith] Crash in GetOldestSnapshot()  (Andrew Gierth <andrew@tao11.riddles.org.uk>)
Responses Re: [sqlsmith] Crash in GetOldestSnapshot()  (Robert Haas <robertmhaas@gmail.com>)
List pgsql-hackers
Andrew Gierth <andrew@tao11.riddles.org.uk> writes:
> In a similar case in the past involving holdable cursors, the solution
> was to detoast _before_ storing in the tuplestore (see
> PersistHoldablePortal). I guess the question now is, under what
> circumstances is it now allowable to detoast a datum with no active
> snapshot? (Wouldn't it be necessary in such a case to know what the
> oldest snapshot ever used in the transaction was?)

After looking at this a bit, I think probably the appropriate solution
is to register the snapshot that was used by the query and store it as
a property of the Portal, releasing it when the Portal is destroyed.
Essentially, this views possession of a relevant snapshot as a resource
that is required to make toast dereferences safe.

I think there has been a bug here for awhile.  Consider a committed-dead
row with some associated toast data, and suppose the query's snapshot
was the last one that could see that row.  Once we destroy the query's
snapshot, there is nothing preventing a concurrent VACUUM from removing
the dead row and the toast data.  When the RETURNING code was originally
written, I think this was safe enough, because the bookkeeping that
determined when VACUUM could remove data was based on transactions'
advertised xmins, and those did not move once set for the life of the
transaction.  So dereferencing a toast pointer you'd fetched was safe
for the rest of the transaction.  But when we changed over to
oldest-snapshot-based xmin advertisement, and made it so that a
transaction holding no snapshots advertised no xmin, we created a hazard
for data held in portals.

In this view of things, flattening toast pointers in "held" tuplestores
is still necessary, but it's because their protective snapshot is going
away not because the transaction as a whole is going away.  But as long
as we hold onto the relevant snapshot, we don't have to do that for
portals used intra-transaction.

It's interesting to think about whether we could let snapshots outlive
transactions and thereby not need to flatten "held" tuplestores either.
I kinda doubt it's a good idea because of the potential bad effects
for vacuum not being able to remove dead rows for a long time; but
it seems at least possible to do it, in this world-view.
        regards, tom lane



pgsql-hackers by date:

Previous
From: Bruce Momjian
Date:
Subject: Re: Heap WARM Tuples - Design Draft
Next
From: Robert Haas
Date:
Subject: Re: [sqlsmith] Crash in GetOldestSnapshot()