Enable transaction code and use it in some installation places

RESOLVED FIXED in Bugzilla 3.2

Status

()

Bugzilla
Database
--
enhancement
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: Max Kanat-Alexander, Assigned: Max Kanat-Alexander)

Tracking

Bugzilla 3.2
Bug Flags:
approval +

Details

Attachments

(1 attachment)

v1
7.78 KB, patch
Max Kanat-Alexander
: review+
Details | Diff | Splinter Review
(Assignee)

Description

11 years ago
Right now transactions are "disabled" manually in the Bugzilla code (we throw errors if somebody calls bz_start_transaction, I believe).

Also, I have to figure out a bit more about what I want the default transaction behavior to be. As I recall, there are only two ANSI transaction modes that are supported by both MySQL and PostgreSQL.

The first script that we'll implement them in is editkeywords.cgi, since that's the easiest.

Transaction code should happen in CGIs, not in modules, I'm pretty sure.
(Assignee)

Comment 1

11 years ago
Okay, so when I went to add them to editkeywords.cgi, I realized the best place to add them would actually just be right inside of Bugzilla::Object itself. Since I made transactions nestable, it's no problem at all.
Summary: Enable transaction code and have editkeywords.cgi use it → Enable transaction code and use them inside of Bugzilla::Object
(Assignee)

Comment 2

11 years ago
Okay, actually, I'll split the Bugzilla::Object code out into another patch, because that part will require review (since I don't own that). I'll just use it at some simple places in Bugzilla::Install for now, since I own that.
Summary: Enable transaction code and use them inside of Bugzilla::Object → Enable transaction code and use them in some installation places
(Assignee)

Comment 3

11 years ago
Created attachment 258624 [details] [diff] [review]
v1

Okay! This has a change to Bugzilla.pm, but it's arguably a DB change. There's also a slight change to mod_perl.pl, but it's a DB change and I'm probably the mod_perl "owner" anyhow, if there was such a thing. :-) I think it's OK to check in as module owner.
Assignee: database → mkanat
Status: NEW → ASSIGNED
Attachment #258624 - Flags: review+
(Assignee)

Updated

11 years ago
Summary: Enable transaction code and use them in some installation places → Enable transaction code and use it in some installation places
(Assignee)

Comment 4

11 years ago
Checking in Bugzilla.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla.pm,v  <--  Bugzilla.pm
new revision: 1.56; previous revision: 1.55
done
Checking in mod_perl.pl;
/cvsroot/mozilla/webtools/bugzilla/mod_perl.pl,v  <--  mod_perl.pl
new revision: 1.6; previous revision: 1.5
done
Checking in Bugzilla/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB.pm,v  <--  DB.pm
new revision: 1.96; previous revision: 1.95
done
Checking in Bugzilla/DB/Mysql.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/DB/Mysql.pm,v  <--  Mysql.pm
new revision: 1.53; previous revision: 1.52
done
Checking in Bugzilla/Install/DB.pm;
/cvsroot/mozilla/webtools/bugzilla/Bugzilla/Install/DB.pm,v  <--  DB.pm
new revision: 1.29; previous revision: 1.28
done
Status: ASSIGNED → RESOLVED
Last Resolved: 11 years ago
Flags: approval+
Resolution: --- → FIXED

Comment 5

11 years ago
Comment on attachment 258624 [details] [diff] [review]
v1

Unfortunately, it seems that start, commit and rollback should be implemented at the MySQL/PgSQL layer.  MySQL has the ability to create save points, making it possible to roll back to a particular part of a transaction.  So, that effectively makes it possible to "nest" transactions but only if they're named.  The problem with nesting in this particular method is it violates ACID compliance, specifically - I - one transaction does not affect another.  What if this particular commit fails?  We have no way of notifying "others" that our commitment failed.

Rollback in this case, could potentially roll back parts of a transaction that the nested start didn't intend to roll back.  By using MySQL save points, this can be avoided and would restore most of the ACID compliance.  Technically, it still wouldn't be ACID, however, because a failure at any of the nested transaction levels would cause a failure for the higher-up levels of the transaction.
Attachment #258624 - Flags: review-
(Assignee)

Comment 6

11 years ago
Comment on attachment 258624 [details] [diff] [review]
v1

We aren't actually nesting transactions, Kevin. Did you read the code?
Attachment #258624 - Flags: review-
You need to log in before you can comment on or make changes to this bug.