At 2010-08-31 19:09, Aryeh Gregor wrote:
On Tue, Aug 31, 2010 at 11:08 AM, PlatonidesPlatonides@gmail.com wrote:
I propose changing Database::begin() / commit() / rollback() to keep the count in mTrxLevel and perform a savepoint instead of a BEGIN should it be called inside another one.
Savepoints are a way to perform partial rollbacks inside a bigger transaction. They are supported by all our databases [5] [6] [7] [8] [9] [10]. We shouldn't even have problems with our 4.0.30 servers, since its support was added in mysql 4.0.14 [11].
I think there would be performance issues with this. Specifically, there are a number of places where we do a commit because we need to release locks immediately to avoid contention. It looks like the locks are held until the transaction commit when savepoints are used, so this might make us hold a lot of locks for much too long. DatabaseBase should definitely be DBMS-neutral here, though, and should not depend on MySQL's behavior. I suggest that begin() check the transaction level, and if there's an open transaction, it should do an explicit commit() first and wfWarn(). This kind of thing is a bug -- it will lead to less atomicity than it looks like we have from casual code inspection, and these cases should be looked at and fixed if possible.
I think it still should be conscious decision and so those functions could use their first... hm... second parameter as the transaction name. For MS SQL you can simply use BEGIN TRANSACTION [name] (I think it would be more natural), for MySQL I guess savepoints should be used.
Regards, Nux.