Re: INSERT ... ON CONFLICT {UPDATE | IGNORE}

On 28 August 2014 03:43, Peter Geoghegan <pg@heroku.com> wrote:

> "Value locking"
> ===========
>
> To date, on-list discussion around UPSERT has almost exclusively
> concerned what I've called "value locking"; the idea of locking values
> in unique indexes in the abstract (to establish the right to insert
> ahead of time). There was some useful discussion on this question
> between myself and Heikki back around December/January. Ultimately, we
> were unable to reach agreement on an approach and discussion tapered
> off. However, Heikki did understand the concerns that informed by
> design. He recognized the need to be able to easily *release* value
> locks, so as to avoid "unprincipled deadlocks", where under high
> concurrency there are deadlocks between sessions that only UPSERT a
> single row at a time. I'm not sure how widely appreciated this point
> is, but I believe that Heikki appreciates it. It is a very important
> point in my opinion. I don't want an implementation that is in any way
> inferior to the "UPSERT looping subxact" pattern does (i.e. the plpsql
> thing that the docs suggest).
>
> When we left off, Heikki continued to favor an approach that involved
> speculatively inserting heap tuples, and then deleting them in the
> event of a conflict. This design was made more complicated when the
> need to *release* value locks became apparent (Heikki ended up making
> some changes to HeapTupleSatisfiesDirty(), as well as sketching a
> design for what you might call a "super delete", where xmin can be set
> to InvalidTransactionId for speculatively-inserted heap tuples). After
> all, it wasn't as if we could abort a subxact to release locks, which
> is what the "UPSERT looping subxact" pattern does. I think it's fair
> to say that that design became more complicated than initially
> anticipated [4] [5].
>
> Anyway, the greater point here is that fundamentally, AFAICT Heikki
> and I were in agreement. Once you buy into the idea that we must avoid
> holding on to "value locks" of whatever form - as Heikki evidently did
> - then exactly what form they take is ultimately only a detail.
> Granted, it's a very important detail, but a detail nonetheless. It
> can be discussed entirely independently of all of this new stuff, and
> thank goodness for that.
>
> If anyone finds my (virtually unchanged) page heavyweight lock based
> value locking approach objectionable, I ask that the criticism be
> framed in a way that makes a sharp distinction between each of the
> following:
>
> 1. You don't accept that value locks must be easily released in the
> event of a conflict. Is anyone in this camp? It's far from obvious to
> me what side of this question Andres is on at this stage, for example.
> Robert might have something to say here too.
>
> 2. Having taken into account the experience of myself and Heikki, and
> all that is implied by taking that approach ***while avoiding
> unprincipled deadlocks***, you continue to believe that an approach
> based on speculative heap insertion, or some alternative scheme is
> better than what I have done to the nbtree code here, or you otherwise
> dislike something about the proposed value locking scheme. You accept
> that value locks must be released and released easily in the event of
> a conflict, but like Heikki you just don't like what I've done to get
> there.
>
> Since we can (I believe) talk about the value locking aspect and the
> rest of the patch independently, we should do so...unless you're in
> camp 1, in which case I guess that we'll have to thrash it out.

I'm trying to understand and help out with pushing this patch forwards
to completion.

Basically, I have absolutely no idea whether I object to or agree with
1) and don't know where to look to find out. We need a clear
exposition of design and the alternatives.

My approach would be to insert an index tuple for that value into the
index, but with the leaf ituple marked with an xid rather than a ctid.
If someone tries to insert into the index they would see this and wait
for the inserting transaction to end. The inserting transaction would
then resolve what happens in the heap (insert/update) and later
repoint the index tuple to the inserted/updated row version. I don't
see the need for page level locking since it would definitely result
in deadlocks (e.g. SQLServer).

-- Simon Riggs                   http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services