Re: ask for review of MERGE

There are now two branches of MERGE code in review progress available.  
http://github.com/greg2ndQuadrant/postgres/tree/merge-unstable has the 
bit-rotted version that doesn't quite work against HEAD yet, while 
http://github.com/greg2ndQuadrant/postgres/tree/merge is what I'm still 
testing against until I get that sorted out.

Attached is a tar file containing an initial concurrent MERGE test 
case.  What it does is create is a pgbench database with a scale of 1.  
Then it runs a custom test that does an UPSERT using MERGE against 
pgbench_accounts, telling pgbench the database scale is actually 2.  
This means that approximately half of the statements executed will hit 
the MATCHED side of the merge and update existing rows, while half will 
hit the NOT MATCHED one and create new account records.  Did some small 
tests using pgbench's debug mode where you see all the statements it 
executes, and all the output looked correct.  Successive tests runs are 
not deterministically perfect performance comparisons, but with enough 
transactions I hope that averages out.

For comparison sake, there's an almost identical test case that does the 
same thing using the pl/pgsql example function from the PostgreSQL 
manual for the UPSERT instead also in there.  You just run test-merge.sh 
or test-function.sh and it runs the whole test, presuming you built and 
installed pgbench and don't mind that it will blow away any database 
named pgbench you already have.

Since the current MERGE implementation is known not to be optimized for 
concurrency, my main goal here wasn't to see how fast it was.  That 
number is interesting though.  With the sole postgresql.conf change of:

checkpoint_settings=32

And a debug/assertion build using 8 concurrent clients, I got 1607 TPS 
of UPSERT out of the trigger approach @ 6MB/s of writes to disk, while 
the MERGE one delivered 988 TPS @ 4.5MB/s of writes.  Will explore this 
more later.

This did seem to find a bug in the implementation after running for a while:

TRAP: FailedAssertion("!(epqstate->origslot != ((void *)0))", File: 
"execMain.c", Line: 1732)

Line number there is relative to what you can see at 
http://github.com/greg2ndQuadrant/postgres/blob/merge/src/backend/executor/execMain.c
and that's a null pointer check at the beginning of 
EvalPlanQualFetchRowMarks.  Haven't explored why this happened or how 
repeatable this is, but since Boxuan was looking for some bugs to chase 
I wanted to deliver him one to chew on.

While the performance doesn't need to be great in V1, there needs to be 
at least some minimal protection against concurrency issues before this 
is commit quality.  Will continue to shake this code out looking for 
them now that I have some basic testing that works for doing so.

-- 
Greg Smith   2ndQuadrant US    greg@2ndQuadrant.com   Baltimore, MD
PostgreSQL Training, Services and Support        www.2ndQuadrant.us