From 9193214274889f2b7636146e72d8200e9bfaeb7b Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Tue, 4 Mar 2014 15:50:54 +0800 Subject: Bug 966180: backport bug 956233 to bmo (enable USE_MEMCACHE on most objects) --- Bugzilla/Attachment.pm | 15 ++++++++++++++- Bugzilla/Auth/Verify.pm | 8 ++++++-- Bugzilla/Auth/Verify/DB.pm | 1 + Bugzilla/Bug.pm | 2 ++ Bugzilla/Classification.pm | 14 ++++++++++++-- Bugzilla/Comment/TagWeights.pm | 3 +++ Bugzilla/Field.pm | 2 ++ Bugzilla/Flag.pm | 2 ++ Bugzilla/FlagType.pm | 12 ++++++++++-- Bugzilla/Install/Requirements.pm | 6 ++++++ Bugzilla/Memcached.pm | 34 ++++++++++++++++++++++++++++++---- Bugzilla/Milestone.pm | 1 + Bugzilla/Object.pm | 20 ++++++++++++++++---- Bugzilla/Search/Recent.pm | 3 +++ Bugzilla/Search/Saved.pm | 1 + Bugzilla/User.pm | 1 + 16 files changed, 110 insertions(+), 15 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/Attachment.pm b/Bugzilla/Attachment.pm index bb0f83aba..2b2159346 100644 --- a/Bugzilla/Attachment.pm +++ b/Bugzilla/Attachment.pm @@ -921,6 +921,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; @@ -945,11 +948,21 @@ 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->selectcol_arrayref( + '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)); $dbh->bz_commit_transaction(); + + # 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 a8cd0af2c..3578631f1 100644 --- a/Bugzilla/Auth/Verify.pm +++ b/Bugzilla/Auth/Verify.pm @@ -97,6 +97,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. @@ -109,23 +110,26 @@ sub create_or_update_user { 'Bugzilla::Auth::Verify::create_or_update_user'}) unless $user_id; - my $user = new Bugzilla::User($user_id); + my $user = new Bugzilla::User({ id => $user_id, cache => 1 }); # Now that we have a valid User, we need to see if any data has to be # updated. + my $user_updated = 0; if ($username && lc($user->login) ne lc($username)) { validate_email_syntax($username) || return { failure => AUTH_ERROR, error => 'auth_invalid_email', details => {addr => $username} }; $user->set_login($username); + $user_updated = 1; } if ($real_name && $user->name ne $real_name) { # $real_name is more than likely tainted, but we only use it # in a placeholder and we never use it after this. trick_taint($real_name); $user->set_name($real_name); + $user_updated = 1; } - $user->update(); + $user->update() if $user_updated; return { user => $user }; } diff --git a/Bugzilla/Auth/Verify/DB.pm b/Bugzilla/Auth/Verify/DB.pm index 2fcfd4017..783e7490a 100644 --- a/Bugzilla/Auth/Verify/DB.pm +++ b/Bugzilla/Auth/Verify/DB.pm @@ -103,6 +103,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 b6861b509..a99dfaf85 100644 --- a/Bugzilla/Bug.pm +++ b/Bugzilla/Bug.pm @@ -76,6 +76,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 88ec4eb3b..5d75a6dc6 100644 --- a/Bugzilla/Classification.pm +++ b/Bugzilla/Classification.pm @@ -56,6 +56,7 @@ use constant VALIDATORS => { ############################### #### Constructors ##### ############################### + sub remove_from_db { my $self = shift; my $dbh = Bugzilla->dbh; @@ -63,9 +64,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->selectcol_arrayref( + '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 5cd246b8e..97d03dc42 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -1078,6 +1078,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 }); } }; diff --git a/Bugzilla/Flag.pm b/Bugzilla/Flag.pm index 089ac6c9f..6ef7be81e 100644 --- a/Bugzilla/Flag.pm +++ b/Bugzilla/Flag.pm @@ -480,6 +480,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; } @@ -626,6 +627,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 910e2d425..3325c8c5c 100644 --- a/Bugzilla/FlagType.pm +++ b/Bugzilla/FlagType.pm @@ -200,8 +200,16 @@ 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->selectcol_arrayref( + 'SELECT id FROM flags WHERE type_id = ? AND requestee_id IS NOT NULL', + undef, $self->id); + + if (@$ids) { + $dbh->do('UPDATE flags SET requestee_id = NULL WHERE ' . $dbh->sql_in('id', $ids)); + foreach my $id (@$ids) { + Bugzilla->memcached->clear({ table => 'flags', id => $id }); + } + } } Bugzilla::Hook::process('flagtype_end_of_update', diff --git a/Bugzilla/Install/Requirements.pm b/Bugzilla/Install/Requirements.pm index cea5a4a34..384df221e 100644 --- a/Bugzilla/Install/Requirements.pm +++ b/Bugzilla/Install/Requirements.pm @@ -374,6 +374,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..4bfca2b65 100644 --- a/Bugzilla/Memcached.pm +++ b/Bugzilla/Memcached.pm @@ -14,6 +14,12 @@ use warnings; use Bugzilla::Error; use Bugzilla::Util qw(trick_taint); use Scalar::Util qw(blessed); +use URI::Escape; +use Encode; +use Sys::Syslog qw(:DEFAULT); + +# memcached keys have a maximum length of 250 bytes +use constant MAX_KEY_LENGTH => 250; sub _new { my $invocant = shift; @@ -26,10 +32,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); @@ -129,6 +136,11 @@ sub clear_all { if (!$memcached->incr("prefix", 1)) { $memcached->add("prefix", time()); } + + # BMO - log that we've wiped the cache + openlog('apache', 'cons,pid', 'local4'); + syslog('notice', encode_utf8('[memcached] cache cleared')); + closelog(); } # in order to clear all our keys, we add a prefix to all our keys. when we @@ -155,6 +167,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 +182,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 +211,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 92bc2192a..a5f3ed383 100644 --- a/Bugzilla/Milestone.pm +++ b/Bugzilla/Milestone.pm @@ -121,6 +121,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 }); } return $changes; } diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index 43d2c07ac..936465b3f 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -48,9 +48,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 @@ -188,7 +187,20 @@ sub new_from_list { # their own implementation of match which is not compatible # with this one. However, match() still needs to have the right $invocant # in order to do $class->DB_TABLE and so on. - return match($invocant, { $id_field => \@detainted_ids }); + my $list = match($invocant, { $id_field => \@detainted_ids }); + + # BMO: Populate the object cache with bug objects, which helps + # inline-history when viewing bugs with dependencies. + if ($class eq 'Bugzilla::Bug') { + foreach my $object (@$list) { + $class->_object_cache_set( + { id => $object->id, cache => 1 }, + $object + ); + } + } + + return $list; } sub new_from_hash { diff --git a/Bugzilla/Search/Recent.pm b/Bugzilla/Search/Recent.pm index 125850e85..b9f6428d7 100644 --- a/Bugzilla/Search/Recent.pm +++ b/Bugzilla/Search/Recent.pm @@ -53,6 +53,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 99194112a..536cf39a4 100644 --- a/Bugzilla/Search/Saved.pm +++ b/Bugzilla/Search/Saved.pm @@ -199,6 +199,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 1c6e68078..0429a25e3 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -305,6 +305,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 }); } } -- cgit v1.2.3-24-g4f1b