On 26/09/12 05:26, Daniel Kinzler wrote:
- Database::commit() should be called after completion of a sequence
of write operations where atomicity is desired, or at the earliest opportunity when contended locks are held.
Ok, so it's basically a savepoint.
Savepoints don't release locks.
In cases where transactions end up being too short due to the need for a called function to commit a transaction when the caller already has a transaction open, it is the responsibility of the callers to introduce some suitable abstraction for serializing the transactions.
In the presence of hooks implemented by extensions, this frankly seems impossible. Also, it would require functions to document exactly if and when they are using transactions, and hooks have to document whether implementors can use transactions.
Presumably there is some particular case where you have encountered this problem. What is it?
When transactions too long, you hit performance problems due to lock contention.
Yes... which makes me wonder why it's a good idea to start a transaction upon the first select, even for requests that do not write to the database at all.
Ordinary select queries do not acquire locks. They just open a snapshot.
For the semantics you propose, begin() and commit() seem to be misleading names
- flush() would be more descriptive of what is going on, and implies no nesting.
begin() and commit() are named after the queries that they unconditionally issue. A BEGIN query causes an implicit COMMIT of the preceding transaction, if there is any. The Database class is redundantly performing the same operation, not implementing some novel concept.
So, how about this:
- normally, flush() will commit any currently open DBO_TRX transaction (and the
next query will start a new one). In cli (auto-commit) mode, it would do nothing.
- begin() and end() (!) use counted levels. In auto-commit mode, the outer-most
level of them starts and commits a transaction.
- Between begin and end, flush does nothing.
This would allow us to use the "earliest commit" semantics, but also control blocks of DB operations that should be consistent and atomic. And it would make it explicit to programmers what they are doing.
I think that to avoid confusion, begin() and commit() should continue to issue the queries they are named for.
You shouldn't use the terms "CLI" and "autocommit" interchangeably. Autocommit is enabled when DBO_TRX is off and $db->mTrxLevel is zero. There are several callers that set up this situation during a web request, and there are many CLI scripts that start transactions explicitly.
Your scheme does not appear to provide a method for hooks to release highly contended locks that they may acquire. Lock contention is usually the most important reason for calling commit(). Holding a contended lock for an excessive amount of time has often brought the site down. Imagine if someone wrapped a hook that writes to site_stats with begin/end. The code would work just fine in testing, and then instantly bring the site down when it was deployed.
-- Tim Starling