On 03/02/2015 11:21 PM, Peter Geoghegan wrote:
> On Mon, Mar 2, 2015 at 12:15 PM, Heikki Linnakangas <hlinnaka@iki.fi> wrote:
>> Hmm. I used a b-tree to estimate the effect that the locking would have in
>> the UPSERT case, for UPSERT into a table with a b-tree index. But you're
>> right that for the question of whether this is acceptable for the case of
>> regular insert into a table with exclusion constraints, other indexams are
>> more interesting. In a very quick test, the overhead with a single GiST
>> index seems to be about 5%. IMHO that's still a noticeable slowdown, so
>> let's try to find a way to eliminate it.
>
> Honestly, I don't know why you're so optimistic that this can be
> fixed, even with that heavyweight lock (for regular inserters +
> exclusion constraints).
We already concluded that it can be fixed, with the heavyweight lock.
See http://www.postgresql.org/message-id/54F4A0E0.4070602@iki.fi. Do you
see some new problem with that that hasn't been discussed yet? To
eliminate the heavy-weight lock, we'll need some new ideas, but it
doesn't seem that hard.
> My experimental branch, which I showed you privately shows big
> problems when I simulate upserting with exclusion constraints (so we
> insert first, handle exclusion violations using the traditional upsert
> subxact pattern that does work with B-Trees). It's much harder to back
> out of a heap_update() than it is to back out of a heap_insert(),
> since we might well be updated a tuple visible to some other MVCC
> snapshot. Whereas we can super delete a tuple knowing that only a
> dirty snapshot might have seen it, which bounds the complexity (only
> wait sites for B-Trees + exclusion constraints need to worry about
> speculative insertion tokens and so on).
>
> My experimental branch works just fine (with a variant jjanes_upsert
> with subxact looping), until I need to restart an update after a
> "failed" heap_update() that still returned HeapTupleMayBeUpdated
> (having super deleted within an ExecUpdate() call). There is no good
> way to do that for ExecUpdate() that I can see, because an existing,
> visible row is affected (unlike with ExecInsert()). Even if it was
> possible, it would be hugely invasive to already very complicated code
> paths.
Ah, so we can't easily use super-deletion to back out an UPDATE. I had
not considered that.
> I continue to believe that the best way forward is to incrementally
> commit the work by committing ON CONFLICT IGNORE first. That way,
> speculative tokens will remain strictly the concern of UPSERTers or
> sessions doing INSERT ... ON CONFLICT IGNORE.
Ok, let's try that. Can you cut a patch that does just ON CONFLICT
IGNORE, please?
- Heikki