From c5464b5bb7dfece2bad2b8af9eba4d9b6d07d778 Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Fri, 31 Jan 2014 15:18:51 +0800 Subject: Bug 956233: enable USE_MEMCACHE on most objects r=dkl, a=glob --- Bugzilla/Attachment.pm | 15 ++++++++++++++- Bugzilla/Auth/Verify.pm | 1 + Bugzilla/Auth/Verify/DB.pm | 1 + Bugzilla/Bug.pm | 2 ++ Bugzilla/Classification.pm | 13 +++++++++++-- Bugzilla/Comment/TagWeights.pm | 3 +++ Bugzilla/Field.pm | 2 ++ Bugzilla/Flag.pm | 2 ++ Bugzilla/FlagType.pm | 9 +++++++-- Bugzilla/Install/Requirements.pm | 6 ++++++ Bugzilla/Memcached.pm | 27 +++++++++++++++++++++++---- Bugzilla/Milestone.pm | 1 + Bugzilla/Object.pm | 5 ++--- Bugzilla/Search/Recent.pm | 3 +++ Bugzilla/Search/Saved.pm | 1 + Bugzilla/User.pm | 1 + buglist.cgi | 1 + contrib/merge-users.pl | 5 +++++ editclassifications.cgi | 11 +++++++++-- editusers.cgi | 5 +++++ query.cgi | 9 ++++++--- sanitycheck.cgi | 26 +++++++++++++++++++++----- userprefs.cgi | 5 +++-- 23 files changed, 130 insertions(+), 24 deletions(-) diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index c0521c9b4..d0bd8e4c2 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -908,6 +908,9 @@ sub update { $dbh->do('UPDATE bugs SET delta_ts = ? WHERE bug_id = ?', undef, ($timestamp, $self->bug_id)); $self->{modification_time} = $timestamp; + # because we updated the attachments table after SUPER::update(), we + # need to ensure the cache is flushed. + Bugzilla->memcached->clear({ table => 'attachments', id => $self->id }); } return $changes; @@ -932,7 +935,10 @@ sub remove_from_db { my $dbh = Bugzilla->dbh; $dbh->bz_start_transaction(); - $dbh->do('DELETE FROM flags WHERE attach_id = ?', undef, $self->id); + my @flag_ids = $dbh->selectrow_array( + 'SELECT id FROM flags WHERE attach_id = ?', undef, $self->id); + $dbh->do('DELETE FROM flags WHERE ' . $dbh->sql_in('id', \@flag_ids)) + if @flag_ids; $dbh->do('DELETE FROM attach_data WHERE id = ?', undef, $self->id); $dbh->do('UPDATE attachments SET mimetype = ?, ispatch = ?, isobsolete = ? WHERE attach_id = ?', undef, ('text/plain', 0, 1, $self->id)); @@ -942,6 +948,13 @@ sub remove_from_db { if (-e $filename) { unlink $filename or warn "Couldn't unlink $filename: $!"; } + + # As we don't call SUPER->remove_from_db we need to manually clear + # memcached here. + Bugzilla->memcached->clear({ table => 'attachments', id => $self->id }); + foreach my $flag_id (@flag_ids) { + Bugzilla->memcached->clear({ table => 'flags', id => $flag_id }); + } } ############################### diff --git a/Bugzilla/Auth/Verify.pm b/Bugzilla/Auth/Verify.pm index 0c552f40b..ecb64e22a 100644 --- a/Bugzilla/Auth/Verify.pm +++ b/Bugzilla/Auth/Verify.pm @@ -89,6 +89,7 @@ sub create_or_update_user { if ($extern_id && $username_user_id && !$extern_user_id) { $dbh->do('UPDATE profiles SET extern_id = ? WHERE userid = ?', undef, $extern_id, $username_user_id); + Bugzilla->memcached->clear({ table => 'profiles', id => $username_user_id }); } # Finally, at this point, one of these will give us a valid user id. diff --git a/Bugzilla/Auth/Verify/DB.pm b/Bugzilla/Auth/Verify/DB.pm index 82fa662dc..a5b78797b 100644 --- a/Bugzilla/Auth/Verify/DB.pm +++ b/Bugzilla/Auth/Verify/DB.pm @@ -95,6 +95,7 @@ sub change_password { my $cryptpassword = bz_crypt($password); $dbh->do("UPDATE profiles SET cryptpassword = ? WHERE userid = ?", undef, $cryptpassword, $user->id); + Bugzilla->memcached->clear({ table => 'profiles', id => $user->id }); } 1; diff --git a/Bugzilla/Bug.pm b/Bugzilla/Bug.pm index 95bf293a5..3e48312a9 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -54,6 +54,8 @@ use constant LIST_ORDER => ID_FIELD; # Bugs have their own auditing table, bugs_activity. use constant AUDIT_CREATES => 0; use constant AUDIT_UPDATES => 0; +# This will be enabled later +use constant USE_MEMCACHED => 0; # This is a sub because it needs to call other subroutines. sub DB_COLUMNS { diff --git a/Bugzilla/Classification.pm b/Bugzilla/Classification.pm index 771a894b4..5d488e3b9 100644 --- a/Bugzilla/Classification.pm +++ b/Bugzilla/Classification.pm @@ -55,9 +55,18 @@ sub remove_from_db { ThrowUserError("classification_not_deletable") if ($self->id == 1); $dbh->bz_start_transaction(); + # Reclassify products to the default classification, if needed. - $dbh->do("UPDATE products SET classification_id = 1 - WHERE classification_id = ?", undef, $self->id); + my @product_ids = $dbh->selectrow_array( + "SELECT id FROM products WHERE classification_id = ?", + undef, $self->id); + if (@product_ids) { + $dbh->do("UPDATE products SET classification_id = 1 WHERE " . + $dbh->sql_in('id', \@product_ids)); + foreach my $id (@product_ids) { + Bugzilla->memcached->clear({ table => 'products', id => $id }); + } + } $self->SUPER::remove_from_db(); diff --git a/Bugzilla/Comment/TagWeights.pm b/Bugzilla/Comment/TagWeights.pm index 5835efbc4..f1a220a47 100644 --- a/Bugzilla/Comment/TagWeights.pm +++ b/Bugzilla/Comment/TagWeights.pm @@ -35,6 +35,9 @@ use constant NAME_FIELD => 'tag'; use constant LIST_ORDER => 'weight DESC'; use constant VALIDATORS => { }; +# There's no gain to caching these objects +use constant USE_MEMCACHED => 0; + sub tag { return $_[0]->{'tag'} } sub weight { return $_[0]->{'weight'} } diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm index 82c82161a..dce16f83b 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -1075,6 +1075,8 @@ sub create { # Restore the original obsolete state of the custom field. $dbh->do('UPDATE fielddefs SET obsolete = 0 WHERE id = ?', undef, $field->id) unless $is_obsolete; + + Bugzilla->memcached->clear({ table => 'fielddefs', id => $field->id }); } return $field; diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index 8931649e4..f365afd98 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -461,6 +461,7 @@ sub update { $dbh->do('UPDATE flags SET modification_date = ? WHERE id = ?', undef, ($timestamp, $self->id)); $self->{'modification_date'} = format_time($timestamp, '%Y.%m.%d %T'); + Bugzilla->memcached->clear({ table => 'flags', id => $self->id }); } return $changes; } @@ -607,6 +608,7 @@ sub force_retarget { if ($is_retargetted) { $dbh->do('UPDATE flags SET type_id = ? WHERE id = ?', undef, ($flag->type_id, $flag->id)); + Bugzilla->memcached->clear({ table => 'flags', id => $flag->id }); } else { # Track deleted attachment flags. diff --git a/Bugzilla/FlagType.pm b/Bugzilla/FlagType.pm index 5fee86b30..c70c1598c 100644 --- a/Bugzilla/FlagType.pm +++ b/Bugzilla/FlagType.pm @@ -185,8 +185,13 @@ sub update { # Silently remove requestees from flags which are no longer # specifically requestable. if (!$self->is_requesteeble) { - $dbh->do('UPDATE flags SET requestee_id = NULL WHERE type_id = ?', - undef, $self->id); + my @ids = $dbh->selectrow_array( + "SELECT id FROM flags WHERE type_id = ?", undef, $self->id); + $dbh->do("UPDATE flags SET requestee_id = NULL WHERE " + . $dbh->sql_in('type_id', \@ids)); + foreach my $id (@ids) { + Bugzilla->memcached->clear({ table => 'flags', id => $id }); + } } $dbh->bz_commit_transaction(); diff --git a/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm index cd8978eb1..e72129153 100644 --- a/Bugzilla/Install/Requirements.pm +++ b/Bugzilla/Install/Requirements.pm @@ -396,6 +396,12 @@ sub OPTIONAL_MODULES { }, # memcached + { + package => 'URI-Escape', + module => 'URI::Escape', + version => 0, + feature => ['memcached'], + }, { package => 'Cache-Memcached', module => 'Cache::Memcached', diff --git a/Bugzilla/Memcached.pm b/Bugzilla/Memcached.pm index b1b10311b..0752bcce9 100644 --- a/Bugzilla/Memcached.pm +++ b/Bugzilla/Memcached.pm @@ -14,6 +14,10 @@ use warnings; use Bugzilla::Error; use Bugzilla::Util qw(trick_taint); use Scalar::Util qw(blessed); +use URI::Escape; + +# memcached keys have a maximum length of 250 bytes +use constant MAX_KEY_LENGTH => 250; sub _new { my $invocant = shift; @@ -26,10 +30,11 @@ sub _new { && Bugzilla->params->{memcached_servers}) { require Cache::Memcached; + $self->{namespace} = Bugzilla->params->{memcached_namespace} || ''; $self->{memcached} = Cache::Memcached->new({ servers => [ split(/[, ]+/, Bugzilla->params->{memcached_servers}) ], - namespace => Bugzilla->params->{memcached_namespace} || '', + namespace => $self->{namespace}, }); } return bless($self, $class); @@ -155,6 +160,14 @@ sub _prefix { return $request_cache->{memcached_prefix}; } +sub _encode_key { + my ($self, $key) = @_; + $key = $self->_prefix . ':' . uri_escape_utf8($key); + return length($self->{namespace} . $key) > MAX_KEY_LENGTH + ? undef + : $key; +} + sub _set { my ($self, $key, $value) = @_; if (blessed($value)) { @@ -162,13 +175,17 @@ sub _set { ThrowCodeError('param_invalid', { function => "Bugzilla::Memcached::set", param => "value" }); } - return $self->{memcached}->set($self->_prefix . ':' . $key, $value); + $key = $self->_encode_key($key) + or return; + return $self->{memcached}->set($key, $value); } sub _get { my ($self, $key) = @_; - my $value = $self->{memcached}->get($self->_prefix . ':' . $key); + $key = $self->_encode_key($key) + or return; + my $value = $self->{memcached}->get($key); return unless defined $value; # detaint returned values @@ -187,7 +204,9 @@ sub _get { sub _delete { my ($self, $key) = @_; - return $self->{memcached}->delete($self->_prefix . ':' . $key); + $key = $self->_encode_key($key) + or return; + return $self->{memcached}->delete($key); } 1; diff --git a/Bugzilla/Milestone.pm b/Bugzilla/Milestone.pm index 6d6465e4d..b1783de88 100644 --- a/Bugzilla/Milestone.pm +++ b/Bugzilla/Milestone.pm @@ -113,6 +113,7 @@ sub update { $dbh->do('UPDATE products SET defaultmilestone = ? WHERE id = ? AND defaultmilestone = ?', undef, ($self->name, $self->product_id, $changes->{value}->[0])); + Bugzilla->memcached->clear({ table => 'produles', id => $self->product_id }); } $dbh->bz_commit_transaction(); diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index c4dfe8cf7..ff1454539 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -35,9 +35,8 @@ use constant AUDIT_UPDATES => 1; use constant AUDIT_REMOVES => 1; # When USE_MEMCACHED is true, the class is suitable for serialisation to -# Memcached. This will be flipped to true by default once the majority of -# Bugzilla Object have been tested with Memcached. -use constant USE_MEMCACHED => 0; +# Memcached. See documentation in Bugzilla::Memcached for more information. +use constant USE_MEMCACHED => 1; # This allows the JSON-RPC interface to return Bugzilla::Object instances # as though they were hashes. In the future, this may be modified to return diff --git a/Bugzilla/Search/Recent.pm b/Bugzilla/Search/Recent.pm index 6d349d129..cc1c3c582 100644 --- a/Bugzilla/Search/Recent.pm +++ b/Bugzilla/Search/Recent.pm @@ -42,6 +42,9 @@ use constant VALIDATORS => { use constant UPDATE_COLUMNS => qw(bug_list list_order); +# There's no gain to caching these objects +use constant USE_MEMCACHED => 0; + ################### # DB Manipulation # ################### diff --git a/Bugzilla/Search/Saved.pm b/Bugzilla/Search/Saved.pm index e81035ada..5dfce611b 100644 --- a/Bugzilla/Search/Saved.pm +++ b/Bugzilla/Search/Saved.pm @@ -187,6 +187,7 @@ sub rename_field_value { } $dbh->do("UPDATE $table SET query = ? WHERE $id_field = ?", undef, $query, $id); + Bugzilla->memcached->clear({ table => $table, id => $id }); } $dbh->bz_commit_transaction(); diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 5ded9f06e..872c742db 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -278,6 +278,7 @@ sub update_last_seen_date { # pending changes $dbh->do("UPDATE profiles SET last_seen_date = ? WHERE userid = ?", undef, $date, $self->id); + Bugzilla->memcached->clear({ table => 'profiles', id => $self->id }); } } diff --git a/buglist.cgi b/buglist.cgi index 281504c66..600e14ef7 100755 --- a/buglist.cgi +++ b/buglist.cgi @@ -382,6 +382,7 @@ if ($cmdtype eq "dorem") { $dbh->do('DELETE FROM namedquery_group_map WHERE namedquery_id = ?', undef, $query_id); + Bugzilla->memcached->clear({ table => 'namedqueries', id => $query_id }); } # Now reset the cached queries diff --git a/contrib/merge-users.pl b/contrib/merge-users.pl index 19cb8d4c9..eb205cac2 100755 --- a/contrib/merge-users.pl +++ b/contrib/merge-users.pl @@ -222,4 +222,9 @@ $user->derive_regexp_groups(); # Commit the transaction $dbh->bz_commit_transaction(); +# It's complex to determine which items now need to be flushed from memcached. +# As user merge is expected to be a rare event, we just flush the entire cache +# when users are merged. +Bugzilla->memcached->clear_all(); + print "Done.\n"; diff --git a/editclassifications.cgi b/editclassifications.cgi index 3d93057b9..d47853133 100755 --- a/editclassifications.cgi +++ b/editclassifications.cgi @@ -188,9 +188,10 @@ if ($action eq 'update') { if ($action eq 'reclassify') { my $classification = Bugzilla::Classification->check($class_name); - + my $sth = $dbh->prepare("UPDATE products SET classification_id = ? WHERE name = ?"); + my @names; if (defined $cgi->param('add_products')) { check_token_data($token, 'reclassify_classifications'); @@ -198,6 +199,7 @@ if ($action eq 'reclassify') { foreach my $prod ($cgi->param("prodlist")) { trick_taint($prod); $sth->execute($classification->id, $prod); + push @names, $prod; } } delete_token($token); @@ -206,7 +208,8 @@ if ($action eq 'reclassify') { if (defined $cgi->param('myprodlist')) { foreach my $prod ($cgi->param("myprodlist")) { trick_taint($prod); - $sth->execute(1,$prod); + $sth->execute(1, $prod); + push @names, $prod; } } delete_token($token); @@ -216,6 +219,10 @@ if ($action eq 'reclassify') { $vars->{'classification'} = $classification; $vars->{'token'} = issue_session_token('reclassify_classifications'); + foreach my $name (@names) { + Bugzilla->memcached->clear({ table => 'products', name => $name }); + } + LoadTemplate($action); } diff --git a/editusers.cgi b/editusers.cgi index f4e3c0841..83f364528 100755 --- a/editusers.cgi +++ b/editusers.cgi @@ -650,6 +650,11 @@ if ($action eq 'search') { $dbh->bz_commit_transaction(); delete_token($token); + # It's complex to determine which items now need to be flushed from + # memcached. As user deletion is expected to be a rare event, we just + # flush the entire cache when a user is deleted. + Bugzilla->memcached->clear_all(); + $vars->{'message'} = 'account_deleted'; $vars->{'otheruser'}{'login'} = $otherUser->login; $vars->{'restrictablegroups'} = $user->bless_groups(); diff --git a/query.cgi b/query.cgi index 65a7d3c3d..620cf2737 100755 --- a/query.cgi +++ b/query.cgi @@ -14,6 +14,7 @@ use Bugzilla; use Bugzilla::Bug; use Bugzilla::Constants; use Bugzilla::Search; +use Bugzilla::Search::Saved; use Bugzilla::User; use Bugzilla::Util; use Bugzilla::Error; @@ -76,9 +77,11 @@ if ($cgi->param('nukedefaultquery')) { if ($userid) { my $token = $cgi->param('token'); check_hash_token($token, ['nukedefaultquery']); - $dbh->do("DELETE FROM namedqueries" . - " WHERE userid = ? AND name = ?", - undef, ($userid, DEFAULT_QUERY_NAME)); + my $named_queries = Bugzilla::Search::Saved->match( + { userid => $userid, name => DEFAULT_QUERY_NAME }); + if (@$named_queries) { + $named_queries->[0]->remove_from_db(); + } } $buffer = ""; } diff --git a/sanitycheck.cgi b/sanitycheck.cgi index 256438bd8..2f87fb828 100755 --- a/sanitycheck.cgi +++ b/sanitycheck.cgi @@ -73,6 +73,7 @@ else { } } my $vars = {}; +my $clear_memcached = 0; print $cgi->header() unless Bugzilla->usage_mode == USAGE_MODE_CMDLINE; @@ -149,6 +150,7 @@ if ($cgi->param('createmissinggroupcontrolmapentries')) { } Status('group_control_map_entries_repaired', {counter => $counter}); + $clear_memcached = 1 if $counter; } ########################################################################### @@ -175,6 +177,7 @@ if ($cgi->param('repair_creation_date')) { $sth_UpdateDate->execute($date, $bugid); } Status('bug_creation_date_fixed', {bug_count => scalar(@$bug_ids)}); + $clear_memcached = 1 if @$bug_ids; } ########################################################################### @@ -191,6 +194,7 @@ if ($cgi->param('repair_everconfirmed')) { $dbh->do("UPDATE bugs SET everconfirmed = 1 WHERE bug_status IN ($confirmed_open_states)"); Status('everconfirmed_end'); + $clear_memcached = 1; } ########################################################################### @@ -206,11 +210,12 @@ if ($cgi->param('repair_bugs_fulltext')) { ON bugs_fulltext.bug_id = bugs.bug_id WHERE bugs_fulltext.bug_id IS NULL'); - foreach my $bugid (@$bug_ids) { - Bugzilla::Bug->new($bugid)->_sync_fulltext( new_bug => 1 ); - } + foreach my $bugid (@$bug_ids) { + Bugzilla::Bug->new($bugid)->_sync_fulltext( new_bug => 1 ); + } - Status('bugs_fulltext_fixed', {bug_count => scalar(@$bug_ids)}); + Status('bugs_fulltext_fixed', {bug_count => scalar(@$bug_ids)}); + $clear_memcached = 1 if @$bug_ids; } ########################################################################### @@ -246,7 +251,10 @@ if ($cgi->param('rescanallBugMail')) { Bugzilla::BugMail::Send($bugid, $vars); } - Status('send_bugmail_end') if scalar(@$list); + if (@$list) { + Status('send_bugmail_end'); + Bugzilla->memcached->clear_all(); + } unless (Bugzilla->usage_mode == USAGE_MODE_CMDLINE) { $template->process('global/footer.html.tmpl', $vars) @@ -280,6 +288,7 @@ if ($cgi->param('remove_invalid_bug_references')) { if (scalar(@$bug_ids)) { $dbh->do("DELETE FROM $table WHERE $field IN (" . join(',', @$bug_ids) . ")"); + $clear_memcached = 1; } } @@ -310,6 +319,7 @@ if ($cgi->param('remove_invalid_attach_references')) { $dbh->bz_commit_transaction(); Status('attachment_reference_deletion_end'); + $clear_memcached = 1 if @$attach_ids; } ########################################################################### @@ -336,12 +346,17 @@ if ($cgi->param('remove_old_whine_targets')) { $dbh->do("DELETE FROM whine_schedules WHERE mailto_type = $type AND mailto IN (" . join(',', @$old_ids) . ")"); + $clear_memcached = 1; } } $dbh->bz_commit_transaction(); Status('whines_obsolete_target_deletion_end'); } +# If any repairs were attempted or made, we need to clear memcached to ensure +# state is consistent. +Bugzilla->memcached->clear_all() if $clear_memcached; + ########################################################################### # Repair hook ########################################################################### @@ -717,6 +732,7 @@ if (scalar(@invalid_flags)) { # Silently delete these flags, with no notification to requesters/setters. $dbh->do('DELETE FROM flags WHERE id IN (' . join(',', @flag_ids) .')'); Status('flag_deletion_end'); + Bugzilla->memcached->clear_all(); } else { foreach my $flag (@$invalid_flags) { diff --git a/userprefs.cgi b/userprefs.cgi index 638004c1e..8a93bf5d3 100755 --- a/userprefs.cgi +++ b/userprefs.cgi @@ -491,12 +491,13 @@ sub SaveSavedSearches { } $user->flush_queries_cache; - + # Update profiles.mybugslink. my $showmybugslink = defined($cgi->param("showmybugslink")) ? 1 : 0; $dbh->do("UPDATE profiles SET mybugslink = ? WHERE userid = ?", - undef, ($showmybugslink, $user->id)); + undef, ($showmybugslink, $user->id)); $user->{'showmybugslink'} = $showmybugslink; + Bugzilla->memcached->clear({ table => 'profiles', id => $user->id }); } -- cgit v1.2.3-24-g4f1b