begin_sql, commit_sql: ADODB smart transactions?

begin_sql, commit_sql: ADODB smart transactions?

sam marshall -
回帖数：7
Core developers的头像 Peer reviewers的头像 Plugin developers的头像
We've changed datalib.php's begin_sql, commit_sql, rollback_sql on our codebase to use ADODB's StartTrans and FinishTrans instead of Postgres-specific SQL code. (Though, for the moment, we left in the 'if' so it still only applies to Postgres. Removing that would be a rather more significant/risky change.)

if ($CFG->dbtype === 'postgres7') {
$db->StartTrans();
}

It's a one-line change and doesn't seem to have broken anything. There are significant advantages to the StartTrans approach, particularly that you can nest calls - very useful in ensuring transactions either happen or (if there's an error) don't.

For example, consider a method add_frog, which needs to add rows to several tables. If it fails, it should fail 'cleanly' without leaving the frog half-added, so you put in begin_sql and commit_sql before and after. OK cool but now you want to make a new method add_frogs_nest which calls add_frog multiple times. You now want the whole nest to be either added or (if there's an error) not added. So you add begin_sql and commit_sql... but this will just mess it up, under the current situation, because you can't nest the calls. [It'll still 'work', but if it fails midway through, the nest will be left midway added.] Making begin_sql use StartTrans would allow this without any other code changes.

See the explanation here:

http://phplens.com/lens/adodb/docs-adodb.htm#ex11

Is there any reason Moodle isn't using it? For example, perhaps Moodle is using an older version of ADODB and a newer ADODB can't be installed because it requires newer PHP version, or something like that? Or some more fundamental reason why it would be a problem? Otherwise I think it would be a good change...

--sam

PS As you can guess this is another of the small changes that I would like to have in core.

PPS No I don't think frog's nest is the right word. 微笑
平均分： -
回复sam marshall

Re: begin_sql, commit_sql: ADODB smart transactions?

John Papaioannou -
I don't think there's any difference, or subtle reason why we 're not using StartTrans(). Possibly the NVZLE people just used what they were familiar with in Postgres; since MySQL has an issue with transactions (see below) they didn't need to think about it, so there you go.

About the "issue": MyISAM tables don't support transactions, period. So you have the dilemma of either forcing the move to e.g. InnoDB (slower, uses more space, is it even enabled if you don't have control of the MySQL server?) or being in the position where you are supposed to have transaction facilities, but you don't really have any. That's not a nice situation; seeing what looks like transaction support can very well lead to developers having a false sense of security about it.

Jon
平均分： -
回复John Papaioannou

Re: begin_sql, commit_sql: ADODB smart transactions?

sam marshall -
Core developers的头像 Peer reviewers的头像 Plugin developers的头像
I really hate MySQL - the whole table type thing is a mess, I don't understand why anybody would use it...

Anyhow, leaving that aside - basically I think it would probably be safe to make the change I outlined (leaving in the if postgres check). A more risky change would be to just make the call for all databases and assume ADODB will handle it. I don't know what happens if you do that in MySQL in cases where (a) you're using table types that support it, or (b) you're not.

Seems to me like Moodle should definitely be moving toward a world in which transactions work, but that doesn't mean it's necessary to absolutely require people use an appropriate database/table type.

--sam
平均分： -
回复sam marshall

Re: begin_sql, commit_sql: ADODB smart transactions?

John Papaioannou -
I believe (but am not 100% positive) that there's no harm done without the if. Most probably ADODB's mysql driver simply does nothing when the transaction functions are called (there's a separate "mysqlt" driver for MySQL with transaction support enabled, so I guess there must be some difference there wink). I don't see why the BEGIN TRANSACTION query would fail anyway since transaction stuff is handled by the storage driver, which is pretty low level.

Again, the problem with transactions is this: what do you do if at some point in your code you decide you need to roll the transaction back? If you clean up the database manually, you might just as well ditch transactions. If you don't, then it's left in a half-baked state. But this is definitely a problem, because two sentences ago you decided you need to roll the transaction back.

So as I see it it's one of these:
  1. Ditch non-transaction MySQL (probably impossible)
  2. Have Moodle not be consistent in its error-handling across databases (definitely undesirable from Moodle's point of view) or
  3. Just live with it and clean up manually (the poor developer gets the shaft again, but acceptable)

平均分： -
回复sam marshall

Re: begin_sql, commit_sql: ADODB smart transactions?

Martín Langhoff -
hi! Right now we cannot count on transactions working, because the main RDBMS platform for Moodle is MySQL (this has two aspects actually: MySQL's default table type doesn't support transactions, and the InnoDB engine doesn't scale very well AFAICS). So Moodle cannot count on transactions today, and probably for a long time.

So we have to code defensively in that sense. Having transactions would make thing easier, but... tough. 伤心

The transaction support (begin_sql(), etc) is there mainly to speed up bulk changes that are very slow under Postgres due to each UPDATE or INSERT being ACID. We can replace it with AdoDB's BeginTrans() -- but I would _not_ use that smart transactions thing because it masks buggy code that does nested transactions.

One day we when we can truly count on transactions (and on the RDBMS being _fast_ even with transactions), we'll probably start reworking the core and key modules to take advantage of transactions. Doing that will require refactoring some bits of code to make sure control over the transaction lies within one function with a clear logic flow.

Unfortunately, it won't happen until the MySQL guys either sort their stuff out, or they mess it up so much that everyone migrates to Pg 眨眼
平均分： -
回复Martín Langhoff

Re: begin_sql, commit_sql: ADODB smart transactions?

sam marshall -
Core developers的头像 Peer reviewers的头像 Plugin developers的头像
OK... so, here we use nested transactions (mostly in OU-only code) and consequently need ADODB's smart transactions. Should we just call $db->StartTrans() directly rather than going through datalib? (No, we aren't going to change it to not use transactions.)

Also if the slowness in Postgres is caused by UPDATE/INSERTs being ACID, wouldn't it also apply to MySQL under InnoDB - which some people have selected as default, I know I certainly did whenever I used to use MySQL - meaning that begin_sql should use BeginTrans and apply to all databases? Or is the slowness caused by something more specific to Postgres.

By the way, I agree that accidentally nesting transactions by virtue of forgetting to close one can cause bugs but the current situation has absolutely no protection against that; there isn't any error that occurs when you do that. So I think it's inaccurate to suggest that the current situation is any safer than smart transactions. In fact, smart transactions are much less likely to mask bugs - if you forget to close a transaction, your database change won't happen at all. So the current situation masks bugs, and smart transactions wouldn't...

--sam
平均分： -
回复sam marshall

Re: begin_sql, commit_sql: ADODB smart transactions?

Andrew McMillan -
I tend to agree, Sam.

The other thing to consider is that PostgreSQL 8.2 will start to support nested transactions, or something on the way to that.  Eventually those ADODB functions will very likely Do The Right Thing for nesting.

And as it stands I would say that they do a better thing than the default low-level SQL behaviour - at least at an application behaviour level they do - and something could always be added to warn about stuff where nested transactions are used, but not actually achievable within the backend.

It would also be good to see some testing on "what happens if you use the ADODB transaction functions against a MySQL database that doesn't support them, or (shudder!) partially supports them".  If someone could do that testing and get some facts about this then people might be able to have some confidence to use transactions at appropriate points in the code.

Regards,
Andrew McMillan.
平均分： -
回复Andrew McMillan

Re: begin_sql, commit_sql: ADODB smart transactions?

sam marshall -
Core developers的头像 Peer reviewers的头像 Plugin developers的头像
Bit of an old thread, but for info we have just changed back the functions in our version of datalib, since this change wasn't accepted into core and we wanted to reduce our difference. We are now calling the ADODB methods directly through $db.
平均分： -