In message <1357499750.81.0.443026588007.issue2550731@psf.upfronthosting.co.za>
 <1357499750.81.0.443026588007.issue2550731@psf.upfronthosting.co.za>,
Bernhard Reiter writes:
>Bernhard Reiter added the comment:
>Yes,I believe the observed race condition is to blame for at least a part
>of the problem. If this hypothesis holds, then if would mean this is
>independent from your patch, though. We'e probably should bring it
>up in a different issue and find a way to demonstrate it with a
>regular version of roundup.

Agreed. I think interleving two commits that change the same fiend in
the same issue with a mysql/postgresql back would do it.

>Note that this race condition is in rdbms_common.py,
>but back_mysql.py overrides the function and seems to deal 
>at least with one concurrent write situation.

Actually it looks like back_mysql deals only with the case where a new
object/node is created. An example race is:

  tx 1 checks to see if an issue with the new issue name/handle is present
    in the db. The handle is available.
  tx 2 checks to see if an issue with the new issue name/handle is present
    in the db. The handle is available.
  tx 1 create a new issue using the name/handle. Success.
  tx 2 creates a new issue using the handle, fails because the handle
    was created by tx1.

I think this case will result in a traceback in the postgres case, but
that depends on the underlying sql that is produced. If the generated
does an insert rather than an insert and on error update, I hope it
will boow up in psycop2. Creating an issue to verify that this case is
handled "correctly" by failing and then making sure a clean
non-traceback result is returned to the user would be good.

>It would mean that some decisions based on auditors or reactors may be wrong
>in a concurrent edit situation for postgresql.
>
>Still I believe this code is strange:
>            current = node.get(propname, None)
>            if value == current:
>                del propvalues[propname]
>                continue
>            journalvalues[propname] = current

I was looking at that code as a possible race condition as the
node.get may get a value that changes during the lifetime of the
transaction. Consider:

 database has prop 1 set to A 
 tx 1: sets prop 1 to B
 tx 2: doesn't change prop 1 keeping it at A

 what happens after tx 2 completes?

 if tx1 commits before tx2 hits this code, it looks like tx 2
   changed prop 1 from B to A. The end state is prop 1 = A.

 if tx2 passes through this code before tx1 hits this code we end up
   with prop 1 changed to B on tx 1. The end state is prop 1 = B

I am not sure what should happen there exactly.

>shouldn't it be 
>  journalvalues[propname] = value 
>instead of "current"?

I think journalvalues is recording the old values and passed as props
on a call to self.db.addjournal -> doSaveJournal. If not journaling
would be totally broken and this part of code has been around since
the 0.5 release according to hg blame and hg log.

Also the same code shows up in back_anydbm.py.