Andres Freund <andres@anarazel.de> writes:
> On 2016-05-10 16:14:50 -0400, Tom Lane wrote:
>> I think the problem is pretty hypothetical until you get to consuming a
>> substantial part of the OID space within any one toast table, at which
>> point you're going to need 8-byte toast OIDs. Improving that situation
>> seems like something we can define as a future feature.
> Doesn't really have to be within a single toast table. As the oid
> counter is global, it just needs to advance quickly enough globally.
I don't think that alters the conclusion. To get rapid advancement
through the whole 4G OID space, you'd need densely packed existing
OIDs in all of the toast tables being inserted into. And the more
toast tables in play, the less likely any one of them is densely
packed, because it's likely some OIDs were assigned into other
toast tables.
Now having said that, you don't actually need *rapid* advancement
of the OID counter to have a potential problem. Imagine that some
transaction inserts a TOAST value and later fails, so that you have
a dead-but-unhinted toast tuple sitting there. If no VACUUM comes
along to clean it up (quite possible if there's low activity on
the parent table), then after 4G uses of OIDs, you could be unlucky
enough that the next attempted insertion into that TOAST table chooses
the same OID. Now, that choosing would be done by GetNewOidWithIndex
which would have to have seen the dead toast tuple; but if its hinting of
the tuple got lost somehow then you could have two tuples with the same
OID and neither hinted as dead. Then subsequent TOAST fetches have
a problem that they might take the wrong one.
Having said that, I still say that changing HeapTupleSatisfiesToast
is the wrong thing. It can't go deciding not to return toast values
because they're committed dead --- the parent tuple could easily be
committed dead as well, and yet still be visible to our query's
snapshot. Nor is it attractive to introduce more hint-bit-setting
requirements for toast tuples; that just increases the amount of
hint-bit-setting disk traffic that must occur subsequently to
any update involving toast values.
Maybe the right fix here is for toast OID assignment to use
GetNewOidWithIndex with SnapshotToast, or some special-purpose
snapshot that does the right thing, so that it would see such a
dead toast row as being a conflict. (If you got enough such
dead rows, this might make it hard to find a free OID; but hopefully
vacuum would come along and fix things before it got to that.)
regards, tom lane