Skip to content

bugzilla
bugzilla
Commit c0b4d49d authored by lpsolit%gmail.com's avatar lpsolit%gmail.com
Browse files
Options

Bug 121069: Remove $dbh->bz_(un)lock_tables from process_bug.cgi and Error.pm in…

Bug 121069: Remove $dbh->bz_(un)lock_tables from process_bug.cgi and Error.pm in favor of DB transactions. These methods are no longer used and are completely removed now - Patch by Frédéric Buclin <LpSolit@gmail.com> r/a=mkanat
parent ce3089f5
Hide whitespace changes
Inline Side-by-side
Showing with 13 additions and 208 deletions
Bugzilla/Constants.pm
View file @ c0b4d49d
......@@ -91,7 +91,6 @@ use File::Basename;
CMT_POPULAR_VOTES
CMT_MOVED_TO
UNLOCK_ABORT
THROW_ERROR
RELATIONSHIPS
......@@ -269,10 +268,6 @@ use constant CMT_HAS_DUPE => 2;
use constant CMT_POPULAR_VOTES => 3;
use constant CMT_MOVED_TO => 4;
# used by Bugzilla::DB to indicate that tables are being unlocked
# because of error
use constant UNLOCK_ABORT => 1;
# Determine whether a validation routine should return 0 or throw
# an error when the validation fails.
use constant THROW_ERROR => 1;
......
Bugzilla/DB.pm
View file @ c0b4d49d
......@@ -273,8 +273,7 @@ EOT
# List of abstract methods we are checking the derived class implements
our @_abstract_methods = qw(REQUIRED_VERSION PROGRAM_NAME DBD_VERSION
new sql_regexp sql_not_regexp sql_limit sql_to_days
sql_date_format sql_interval
bz_lock_tables bz_unlock_tables);
sql_date_format sql_interval);
# This overridden import method will check implementation of inherited classes
# for missing implementation of abstract methods
......@@ -301,62 +300,6 @@ sub import {
$Exporter::ExportLevel-- if $is_exporter;
}
sub bz_lock_tables {
my ($self, @tables) = @_;
my $list = join(', ', @tables);
# Check first if there was no lock before
if ($self->{private_bz_tables_locked}) {
ThrowCodeError("already_locked", { current => $self->{private_bz_tables_locked},
new => $list });
} else {
my %read_tables;
my %write_tables;
foreach my $table (@tables) {
$table =~ /^([\d\w]+)([\s]+AS[\s]+[\d\w]+)?[\s]+(WRITE|READ)$/i;
my $table_name = $1;
if ($3 =~ /READ/i) {
if (!exists $read_tables{$table_name}) {
$read_tables{$table_name} = undef;
}
}
else {
if (!exists $write_tables{$table_name}) {
$write_tables{$table_name} = undef;
}
}
}
# Begin Transaction
$self->bz_start_transaction();
Bugzilla->dbh->do('LOCK TABLE ' . join(', ', keys %read_tables) .
' IN ROW SHARE MODE') if keys %read_tables;
Bugzilla->dbh->do('LOCK TABLE ' . join(', ', keys %write_tables) .
' IN ROW EXCLUSIVE MODE') if keys %write_tables;
$self->{private_bz_tables_locked} = $list;
}
}
sub bz_unlock_tables {
my ($self, $abort) = @_;
# Check first if there was previous matching lock
if (!$self->{private_bz_tables_locked}) {
# Abort is allowed even without previous lock for error handling
return if $abort;
ThrowCodeError("no_matching_lock");
} else {
$self->{private_bz_tables_locked} = "";
# End transaction, tables will be unlocked automatically
if ($abort) {
$self->bz_rollback_transaction();
} else {
$self->bz_commit_transaction();
}
}
}
sub sql_istrcmp {
my ($self, $left, $right, $op) = @_;
$op ||= "=";
......@@ -1949,63 +1892,6 @@ Formatted SQL for the C<IN> operator.
=back
=item C<bz_lock_tables>
=over
=item B<Description>
Performs a table lock operation on specified tables. If the underlying
database supports transactions, it should also implicitly start a new
transaction.
Abstract method, should be overridden by database specific code.
=item B<Params>
=over
=item C<@tables> - list of names of tables to lock in MySQL
notation (ex. 'bugs AS bugs2 READ', 'logincookies WRITE')
=back
=item B<Returns> (nothing)
=back
=item C<bz_unlock_tables>
=over
=item B<Description>
Performs a table unlock operation.
If the underlying database supports transactions, it should also implicitly
commit or rollback the transaction.
Also, this function should allow to be called with the abort flag
set even without locking tables first without raising an error
to simplify error handling.
Abstract method, should be overridden by database specific code.
=item B<Params>
=over
=item C<$abort> - C<UNLOCK_ABORT> if the operation on locked tables
failed (if transactions are supported, the action will be rolled
back). No param if the operation succeeded. This is only used by
L<Bugzilla::Error/throw_error>.
=back
=item B<Returns> (none)
=back
=back
......
Bugzilla/DB/Mysql.pm
View file @ c0b4d49d
......@@ -200,44 +200,6 @@ sub sql_group_by {
}
sub bz_lock_tables {
my ($self, @tables) = @_;
my $list = join(', ', @tables);
# Check first if there was no lock before
if ($self->{private_bz_tables_locked}) {
ThrowCodeError("already_locked", { current => $self->{private_bz_tables_locked},
new => $list });
} else {
$self->bz_start_transaction();
$self->do('LOCK TABLE ' . $list);
$self->{private_bz_tables_locked} = $list;
}
}
sub bz_unlock_tables {
my ($self, $abort) = @_;
if ($self->bz_in_transaction) {
if ($abort) {
$self->bz_rollback_transaction();
}
else {
$self->bz_commit_transaction();
}
}
# Check first if there was previous matching lock
if (!$self->{private_bz_tables_locked}) {
# Abort is allowed even without previous lock for error handling
return if $abort;
ThrowCodeError("no_matching_lock");
} else {
$self->do("UNLOCK TABLES");
$self->{private_bz_tables_locked} = "";
}
}
sub _bz_get_initial_schema {
my ($self) = @_;
return $self->_bz_build_schema_from_disk();
......
Bugzilla/Error.pm
View file @ c0b4d49d
......@@ -46,16 +46,15 @@ sub _in_eval {
sub _throw_error {
my ($name, $error, $vars) = @_;
my $dbh = Bugzilla->dbh;
$vars ||= {};
$vars->{error} = $error;
# Make sure any locked tables are unlocked
# and the transaction is rolled back (if supported)
# If we are within an eval(), do not unlock tables as we are
# Make sure any transaction is rolled back (if supported).
# If we are within an eval(), do not roll back transactions as we are
# eval'uating some test on purpose.
Bugzilla->dbh->bz_unlock_tables(UNLOCK_ABORT) unless _in_eval();
$dbh->bz_rollback_transaction() if ($dbh->bz_in_transaction() && !_in_eval());
my $datadir = bz_locations()->{'datadir'};
# If a writable $datadir/errorlog exists, log error details there.
......@@ -124,10 +123,10 @@ sub ThrowCodeError {
sub ThrowTemplateError {
my ($template_err) = @_;
my $dbh = Bugzilla->dbh;
# Make sure any locked tables are unlocked
# and the transaction is rolled back (if supported)
Bugzilla->dbh->bz_unlock_tables(UNLOCK_ABORT);
# Make sure the transaction is rolled back (if supported).
$dbh->bz_rollback_transaction() if $dbh->bz_in_transaction();
my $vars = {};
if (Bugzilla->error_mode == ERROR_MODE_DIE) {
......
process_bug.cgi
View file @ c0b4d49d
......@@ -398,17 +398,7 @@ if ($move_action eq Bugzilla->params->{'move-button-text'}) {
$user->is_mover || ThrowUserError("auth_failure", {action => 'move',
object => 'bugs'});
my @multi_select_locks = map {'bug_' . $_->name . " WRITE"}
Bugzilla->get_fields({ custom => 1, type => FIELD_TYPE_MULTI_SELECT,
obsolete => 0 });
$dbh->bz_lock_tables('bugs WRITE', 'bugs_activity WRITE', 'duplicates WRITE',
'longdescs WRITE', 'profiles READ', 'groups READ',
'bug_group_map READ', 'group_group_map READ',
'user_group_map READ', 'classifications READ',
'products READ', 'components READ', 'votes READ',
'cc READ', 'fielddefs READ', 'bug_status READ',
'status_workflow READ', 'resolution READ', @multi_select_locks);
$dbh->bz_start_transaction();
# First update all moved bugs.
foreach my $bug (@bug_objects) {
......@@ -430,7 +420,7 @@ if ($move_action eq Bugzilla->params->{'move-button-text'}) {
$bug->_clear_dup_id;
}
$_->update() foreach @bug_objects;
$dbh->bz_unlock_tables();
$dbh->bz_commit_transaction();
# Now send emails.
foreach my $bug (@bug_objects) {
......@@ -510,28 +500,9 @@ foreach my $b (@bug_objects) {
# Do Actual Database Updates #
##############################
foreach my $bug (@bug_objects) {
my $write = "WRITE"; # Might want to make a param to control
# whether we do LOW_PRIORITY ...
my @multi_select_locks = map {'bug_' . $_->name . " $write"}
Bugzilla->get_fields({ custom => 1, type => FIELD_TYPE_MULTI_SELECT,
obsolete => 0 });
$dbh->bz_lock_tables("bugs $write", "bugs_activity $write", "cc $write",
"profiles READ", "dependencies $write", "votes $write",
"products READ", "components READ", "milestones READ",
"keywords $write", "longdescs $write", "fielddefs READ",
"bug_group_map $write", "flags $write", "duplicates $write",
"user_group_map READ", "group_group_map READ", "flagtypes READ",
"flaginclusions AS i READ", "flagexclusions AS e READ",
"keyworddefs READ", "groups READ", "attachments READ",
"bug_status READ", "group_control_map AS oldcontrolmap READ",
"group_control_map AS newcontrolmap READ",
"group_control_map READ", "email_setting READ",
"classifications READ", @multi_select_locks);
my $timestamp = $dbh->selectrow_array(q{SELECT NOW()});
$dbh->bz_start_transaction();
my $timestamp = $dbh->selectrow_array(q{SELECT NOW()});
my $changes = $bug->update($timestamp);
my %notify_deps;
......@@ -581,7 +552,7 @@ foreach my $bug (@bug_objects) {
# Set and update flags.
Bugzilla::Flag::process($bug, undef, $timestamp, $cgi, $vars);
$dbh->bz_unlock_tables();
$dbh->bz_commit_transaction();
###############
# Send Emails #
......
template/en/default/global/code-error.html.tmpl
View file @ c0b4d49d
......@@ -416,14 +416,6 @@
[% ELSIF error == "not_in_transaction" %]
Attempted to end transaction without starting one first.
[% ELSIF error == "already_locked" %]
Attempted to lock a table without releasing previous lock first:
<p>Tables already locked:<br>[% current FILTER html %]
<p>Tables requesting locking:<br>[% new FILTER html %]
[% ELSIF error == "no_matching_lock" %]
Attempted to unlock tables without locking them first.
[% ELSIF error == "comma_operator_deprecated" %]
[% title = "SQL query generator internal error" %]
There is an internal error in the SQL query generation code,
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment