On Fri, Apr 10, 2009 at 10:23:36PM +0200, Andreas Mock wrote: [...] >> The current implementation does a SELECT followed by an INSERT or UPDATE >> as appropriate. This introduces a race condition. The whole thing is a >> critical section and needs to be wrapped in a transaction or savepoint. > O.k. I asked why you have to use savepoints, because I thought you can > also use transactions.
Because I'm already using transactions in my higher-level code, and transactions can't be nested. Savepoints are how one performs nested transactions. At present, if two parallel writers are trying to update_or_create() using the same unique fields, and happen to interfere with each other on the critical window, one will succeed and the other will fail. This will also cause higher-level transactions, such as a txn_do block to fail, even if the code is otherwise correct. I don't think it should do that, especially given there's a relatively simple fix using savepoints. >> I implemented it by starting a savepoint, then just trying the INSERT. If >> that fails (normally due to duplicate UNIQUE keys) then the savepoint is >> rolled back and the SELECT and UPDATE is done as before. After the >> UPDATE, the savepoint is committed. > Why do you have to do a rollback when the update is rejected because of > dup key? Because it causes the transaction to fail, and no more queries will be executed until rollback. >> However, I'm not quite sure this trick completely retains the semantics >> of create_or_update, so it wouldn't be a drop-in replacement. > It would be interesting if you can make it really an atomic operation. I'm > asking because I'm not sure if transactions are enough. It is if you also use SELECT FOR UPDATE. >> I also suspect that while savepoints may be necessary, they're not >> sufficient, and one also needs SELECT FOR UPDATE. How might I express >> that in DBIC? > An interesting question. I would be also interested in that. Somebody else has kindly answered that, which I think gives me the final piece to allow me to craft a patch. _______________________________________________ List: http://lists.scsys.co.uk/cgi-bin/mailman/listinfo/dbix-class IRC: irc.perl.org#dbix-class SVN: http://dev.catalyst.perl.org/repos/bast/DBIx-Class/ Searchable Archive: http://www.grokbase.com/group/[email protected]
