alexmv at bestpractical
Mar 16, 2012, 2:22 PM
Views: 107
Permalink
|
|
rt branch, 4.0/lock-transaction-updates, created. rt-4.0.5-93-gc4b74be
|
|
The branch, 4.0/lock-transaction-updates has been created
at c4b74bef625c0489bd3b715d6fd9017dba4c57cc (commit)
- Log -----------------------------------------------------------------
commit c4b74bef625c0489bd3b715d6fd9017dba4c57cc
Author: Alex Vandiver <alexmv [at] bestpractical>
Date: Fri Mar 16 16:57:07 2012 -0400
Lock transaction updates so scrips get a consistent snapshot
Previously, nothing prevented multiple transactions from being run on
the system concurrently, and making identical changes. This could lead
to multiple Corrrespondences, followed by multiple "Status changed from
new to open" transactions. Prevent this by always running
->_NewTransaction in a database transaction, and ensuring that it takes
a write lock on the row before running scrips and purges the cache.
This ensures a coherent and serial execution of scrips.
diff --git a/lib/RT/Record.pm b/lib/RT/Record.pm
index 29cff47..60ef4b3 100644
--- a/lib/RT/Record.pm
+++ b/lib/RT/Record.pm
@@ -1422,6 +1422,20 @@ Private function to create a new RT::Transaction object for this ticket update
=cut
+sub LockForUpdate {
+ my $self = shift;
+ return if RT->Config->Get('DatabaseType') eq "SQLite";
+
+ my $pk = $self->_PrimaryKey;
+ my $id = @_ ? $_[0] : $self->$pk;
+ $self->_expire if $self->isa("DBIx::SearchBuilder::Record::Cachable");
+ return $self->_LoadFromSQL(
+ "SELECT * FROM ".$self->Table
+ ." WHERE $pk = ? FOR UPDATE",
+ $id,
+ );
+}
+
sub _NewTransaction {
my $self = shift;
my %args = (
@@ -1441,6 +1455,11 @@ sub _NewTransaction {
@_
);
+ my $in_txn = RT->DatabaseHandle->TransactionDepth;
+ RT->DatabaseHandle->BeginTransaction unless $in_txn;
+
+ $self->LockForUpdate;
+
my $old_ref = $args{'OldReference'};
my $new_ref = $args{'NewReference'};
my $ref_type = $args{'ReferenceType'};
@@ -1487,6 +1506,9 @@ sub _NewTransaction {
if ( RT->Config->Get('UseTransactionBatch') and $transaction ) {
push @{$self->{_TransactionBatch}}, $trans if $args{'CommitScrips'};
}
+
+ RT->DatabaseHandle->Commit unless $in_txn;
+
return ( $transaction, $msg, $trans );
}
diff --git a/lib/RT/Ticket.pm b/lib/RT/Ticket.pm
index 3f2e94c..79fa036 100644
--- a/lib/RT/Ticket.pm
+++ b/lib/RT/Ticket.pm
@@ -2095,14 +2095,16 @@ sub Comment {
}
$args{'NoteType'} = 'Comment';
+ $RT::Handle->BeginTransaction();
if ($args{'DryRun'}) {
- $RT::Handle->BeginTransaction();
$args{'CommitScrips'} = 0;
}
my @results = $self->_RecordNote(%args);
if ($args{'DryRun'}) {
$RT::Handle->Rollback();
+ } else {
+ $RT::Handle->Commit();
}
return(@results);
@@ -2141,10 +2143,10 @@ sub Correspond {
or ( $self->CurrentUserHasRight('ModifyTicket') ) ) {
return ( 0, $self->loc("Permission Denied"), undef );
}
+ $args{'NoteType'} = 'Correspond';
- $args{'NoteType'} = 'Correspond';
+ $RT::Handle->BeginTransaction();
if ($args{'DryRun'}) {
- $RT::Handle->BeginTransaction();
$args{'CommitScrips'} = 0;
}
@@ -2161,6 +2163,8 @@ sub Correspond {
if ($args{'DryRun'}) {
$RT::Handle->Rollback();
+ } else {
+ $RT::Handle->Commit();
}
return (@results);
diff --git a/t/ticket/race.t b/t/ticket/race.t
new file mode 100644
index 0000000..5d59e5c
--- /dev/null
+++ b/t/ticket/race.t
@@ -0,0 +1,47 @@
+use strict;
+use warnings;
+
+use RT::Test tests => 1;
+
+use constant KIDS => 10;
+
+my $id;
+
+{
+ my $t = RT::Ticket->new( RT->SystemUser );
+ ($id) = $t->Create(
+ Queue => "General",
+ Subject => "Race $$",
+ );
+}
+
+diag "Created ticket $id";
+RT->DatabaseHandle->Disconnect;
+
+my @kids;
+for (1..KIDS) {
+ if (my $pid = fork()) {
+ push @kids, $pid;
+ next;
+ }
+
+ # In the kid, load up the ticket and correspond
+ RT->ConnectToDatabase;
+ my $t = RT::Ticket->new( RT->SystemUser );
+ $t->Load( $id );
+ $t->Correspond( Content => "Correspondence from PID $$" );
+ undef $t;
+ exit 0;
+}
+
+
+diag "Forked @kids";
+waitpid $_, 0 for @kids;
+diag "All kids finished corresponding";
+
+RT->ConnectToDatabase;
+my $t = RT::Ticket->new( RT->SystemUser );
+$t->Load($id);
+my $txns = $t->Transactions;
+$txns->Limit( FIELD => 'Type', VALUE => 'Status' );
+is($txns->Count, 1, "Only one transaction change recorded" );
-----------------------------------------------------------------------
_______________________________________________
Rt-commit mailing list
Rt-commit [at] lists
http://lists.bestpractical.com/cgi-bin/mailman/listinfo/rt-commit
|
rt branch, 4.0/lock-transaction-updates, created. rt-4.0.5-93-gc4b74be