On 02/10/2015 02:21 AM, Peter Geoghegan wrote:
> On Fri, Feb 6, 2015 at 1:51 PM, Bruce Momjian <bruce@momjian.us> wrote:
>> Other than the locking part, the biggest part of this patch is adjusting
>> things so that an INSERT can change into an UPDATE.
>
> Thanks for taking a look at it. That's somewhat cleaned up in the
> attached patchseries - V2.2. This has been rebased to repair the minor
> bit-rot pointed out by Thom.
I don't really have the energy to review this patch in one batch, so I'd
really like to split this into two:
1. Solve the existing "problem" with exclusion constraints that if two
insertions happen concurrently, one of them might be aborted with
"deadlock detected" error, rather then "conflicting key violation"
error. That really wouldn't be worth fixing otherwise, but it happens to
be a useful stepping stone for this upsert patch.
2. All the rest.
I took a stab at extracting the parts needed to do 1. See attached. I
didn't modify ExecUpdate to use speculative insertions, so the issue
remains for UPDATEs, but that's easy to add.
I did not solve the potential for livelocks (discussed at
http://www.postgresql.org/message-id/CAM3SWZTfTt_fehet3tU3YKCpCYPYnNaUqUZ3Q+NAASnH-60teA@mail.gmail.com).
The patch always super-deletes the already-inserted tuple on conflict,
and then waits for the other inserter. That would be nice to fix, using
one of the ideas from that thread, or something else.
We never really debated the options for how to do the speculative
insertion and super-deletion. This patch is still based on the quick &
dirty demo patches I posted about a year ago, in response to issues you
found with earlier versions. That might be OK - maybe I really hit the
nail on designing those things and got them right on first try - but
more likely there are better alternatives.
Are we happy with acquiring the SpeculativeInsertLock heavy-weight lock
for every insertion? That seems bad for performance reasons. Also, are
we happy with adding the new fields to the proc array? Another
alternative that was discussed was storing the speculative insertion
token on the heap tuple itself. (See
http://www.postgresql.org/message-id/52D00D2D.6030307@vmware.com)
Are we happy with the way super-deletion works? Currently, the xmin
field is set to invalid to mark the tuple as super-deleted. That
required checks in HeapTupleSatisfies* functions. One alternative would
be to set xmax equal to xmin, and teach the code currently calls
XactLockTableWait() to check if xmax=xmin, and not consider the tuple as
conflicting.
- Heikki
