On Mon, Jun 20, 2016 at 5:59 PM, Andres Freund <and...@anarazel.de> wrote: > On 2016-06-20 17:55:19 -0400, Robert Haas wrote: >> On Mon, Jun 20, 2016 at 4:24 PM, Andres Freund <and...@anarazel.de> wrote: >> > On 2016-06-20 16:10:23 -0400, Robert Haas wrote: >> >> I >> >> mean, suppose we just don't do any of that before we go off to do >> >> toast_insert_or_update and RelationGetBufferForTuple. Eventually, >> >> when we reacquire the page lock, we might find that somebody else has >> >> already updated the tuple, but couldn't that be handled by >> >> (approximately) looping back up to l2 just as we do in several other >> >> cases? >> > >> > We'd potentially have to undo a fair amount more work: the toasted data >> > would have to be deleted and such, just to retry. Which isn't going to >> > super easy, because all of it will be happening with the current cid (we >> > can't just increase CommandCounterIncrement() for correctness reasons). >> >> Why would we have to delete the TOAST data? AFAIUI, the tuple points >> to the TOAST data, but not the other way around. So if we change our >> mind about where to put the tuple, I don't think that requires >> re-TOASTing. > > Consider what happens if we, after restarting at l2, notice that we > can't actually insert, but return in the !HeapTupleMayBeUpdated > branch. If the caller doesn't error out - and there's certainly callers > doing that - we'd "leak" a toasted datum. Unless the transaction aborts, > the toasted datum would never be cleaned up, because there's no datum > pointing to it, so no heap_delete will ever recurse into the toast > datum (via toast_delete()).
OK, I see what you mean. Still, that doesn't seem like such a terrible cost. If you try to update a tuple and if it looks like you can update it but then after TOASTing you find that the status of the tuple has changed such that you can't update it after all, then you might need to go set xmax = MyTxid() on all of the TOAST tuples you created (whose CTIDs we could save someplace, so that it's just a matter of finding them by CTID to kill them). That's not likely to happen particularly often, though, and when it does happen it's not insanely expensive. We could also reduce the cost by letting the caller of heap_update() decide whether to back out the work; if the caller intends to throw an error anyway, then there's no point. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
