From 3ff56a88eebef3699df7e524dea89be7b593337f Mon Sep 17 00:00:00 2001 From: Byron Jones Date: Mon, 12 May 2014 13:49:53 +0800 Subject: Bug 992767: backport bug 987032 to bmo (allow memcached to cache bugzilla configuration information) --- Bugzilla/Classification.pm | 3 ++ Bugzilla/Field.pm | 5 ++ Bugzilla/Field/Choice.pm | 2 + Bugzilla/Group.pm | 4 ++ Bugzilla/Keyword.pm | 2 + Bugzilla/Memcached.pm | 132 ++++++++++++++++++++++++++++++++++++++------- Bugzilla/Milestone.pm | 14 +++-- Bugzilla/Object.pm | 80 +++++++++++++++++++-------- Bugzilla/Product.pm | 5 ++ Bugzilla/Status.pm | 20 +++++-- Bugzilla/User.pm | 20 +++++-- 11 files changed, 235 insertions(+), 52 deletions(-) (limited to 'Bugzilla') diff --git a/Bugzilla/Classification.pm b/Bugzilla/Classification.pm index 5d75a6dc6..a989a40f2 100644 --- a/Bugzilla/Classification.pm +++ b/Bugzilla/Classification.pm @@ -31,6 +31,8 @@ use base qw(Bugzilla::Field::ChoiceInterface Bugzilla::Object); #### Initialization #### ############################### +use constant IS_CONFIG => 1; + use constant DB_TABLE => 'classifications'; use constant LIST_ORDER => 'sortkey, name'; @@ -75,6 +77,7 @@ sub remove_from_db { foreach my $id (@$product_ids) { Bugzilla->memcached->clear({ table => 'products', id => $id }); } + Bugzilla->memcached->clear_config(); } $self->SUPER::remove_from_db(); diff --git a/Bugzilla/Field.pm b/Bugzilla/Field.pm index 97d03dc42..3e69d152d 100644 --- a/Bugzilla/Field.pm +++ b/Bugzilla/Field.pm @@ -87,6 +87,8 @@ use Scalar::Util qw(blessed); #### Initialization #### ############################### +use constant IS_CONFIG => 1; + use constant DB_TABLE => 'fielddefs'; use constant LIST_ORDER => 'sortkey, name'; @@ -1056,6 +1058,7 @@ sub create { $field->_update_visibility_values(); $dbh->bz_commit_transaction(); + Bugzilla->memcached->clear_config(); if ($field->custom) { my $name = $field->name; @@ -1080,6 +1083,7 @@ sub create { unless $is_obsolete; Bugzilla->memcached->clear({ table => 'fielddefs', id => $field->id }); + Bugzilla->memcached->clear_config(); } }; @@ -1103,6 +1107,7 @@ sub update { $dbh->do("UPDATE " . $self->name . " SET visibility_value_id = NULL"); } $self->_update_visibility_values(); + Bugzilla->memcached->clear_config(); return $changes; } diff --git a/Bugzilla/Field/Choice.pm b/Bugzilla/Field/Choice.pm index 773dbd4ce..8d7d61def 100644 --- a/Bugzilla/Field/Choice.pm +++ b/Bugzilla/Field/Choice.pm @@ -37,6 +37,8 @@ use Scalar::Util qw(blessed); # Initialization # ################## +use constant IS_CONFIG => 1; + use constant DB_COLUMNS => qw( id value diff --git a/Bugzilla/Group.pm b/Bugzilla/Group.pm index 109f06d7f..732de348f 100644 --- a/Bugzilla/Group.pm +++ b/Bugzilla/Group.pm @@ -37,6 +37,8 @@ use Bugzilla::Config qw(:admin); ##### Module Initialization ### ############################### +use constant IS_CONFIG => 1; + use constant DB_COLUMNS => qw( groups.id groups.name @@ -231,6 +233,7 @@ sub update { Bugzilla::Hook::process('group_end_of_update', { group => $self, changes => $changes }); $dbh->bz_commit_transaction(); + Bugzilla->memcached->clear_config(); return $changes; } @@ -420,6 +423,7 @@ sub create { Bugzilla::Hook::process('group_end_of_create', { group => $group }); $dbh->bz_commit_transaction(); + Bugzilla->memcached->clear_config(); return $group; } diff --git a/Bugzilla/Keyword.pm b/Bugzilla/Keyword.pm index e2ecc29e5..cda401c59 100644 --- a/Bugzilla/Keyword.pm +++ b/Bugzilla/Keyword.pm @@ -27,6 +27,8 @@ use Bugzilla::Util; #### Initialization #### ############################### +use constant IS_CONFIG => 1; + use constant DB_COLUMNS => qw( keyworddefs.id keyworddefs.name diff --git a/Bugzilla/Memcached.pm b/Bugzilla/Memcached.pm index 4bfca2b65..7a9734375 100644 --- a/Bugzilla/Memcached.pm +++ b/Bugzilla/Memcached.pm @@ -42,6 +42,10 @@ sub _new { return bless($self, $class); } +sub enabled { + return $_[0]->{memcached} ? 1 : 0; +} + sub set { my ($self, $args) = @_; return unless $self->{memcached}; @@ -97,6 +101,32 @@ sub get { } } +sub set_config { + my ($self, $args) = @_; + return unless $self->{memcached}; + + if (exists $args->{key}) { + return $self->_set($self->_config_prefix . ':' . $args->{key}, $args->{data}); + } + else { + ThrowCodeError('params_required', { function => "Bugzilla::Memcached::set_config", + params => [ 'key' ] }); + } +} + +sub get_config { + my ($self, $args) = @_; + return unless $self->{memcached}; + + if (exists $args->{key}) { + return $self->_get($self->_config_prefix . ':' . $args->{key}); + } + else { + ThrowCodeError('params_required', { function => "Bugzilla::Memcached::get_config", + params => [ 'key' ] }); + } +} + sub clear { my ($self, $args) = @_; return unless $self->{memcached}; @@ -132,44 +162,66 @@ sub clear { sub clear_all { my ($self) = @_; - return unless my $memcached = $self->{memcached}; - if (!$memcached->incr("prefix", 1)) { - $memcached->add("prefix", time()); - } + return unless $self->{memcached}; + $self->_inc_prefix("global"); +} - # BMO - log that we've wiped the cache - openlog('apache', 'cons,pid', 'local4'); - syslog('notice', encode_utf8('[memcached] cache cleared')); - closelog(); +sub clear_config { + my ($self) = @_; + return unless $self->{memcached}; + $self->_inc_prefix("config"); } # in order to clear all our keys, we add a prefix to all our keys. when we # need to "clear" all current keys, we increment the prefix. sub _prefix { - my ($self) = @_; + my ($self, $name) = @_; # we don't want to change prefixes in the middle of a request my $request_cache = Bugzilla->request_cache; - if (!$request_cache->{memcached_prefix}) { + my $request_cache_key = "memcached_prefix_$name"; + if (!$request_cache->{$request_cache_key}) { my $memcached = $self->{memcached}; - my $prefix = $memcached->get("prefix"); + my $prefix = $memcached->get($name); if (!$prefix) { $prefix = time(); - if (!$memcached->add("prefix", $prefix)) { + if (!$memcached->add($name, $prefix)) { # if this failed, either another process set the prefix, or # memcached is down. assume we lost the race, and get the new # value. if that fails, memcached is down so use a dummy # prefix for this request. - $prefix = $memcached->get("prefix") || 0; + $prefix = $memcached->get($name) || 0; } } - $request_cache->{memcached_prefix} = $prefix; + $request_cache->{$request_cache_key} = $prefix; + } + return $request_cache->{$request_cache_key}; +} + +sub _inc_prefix { + my ($self, $name) = @_; + my $memcached = $self->{memcached}; + if (!$memcached->incr($name, 1)) { + $memcached->add($name, time()); } - return $request_cache->{memcached_prefix}; + delete Bugzilla->request_cache->{"memcached_prefix_$name"}; + + # BMO - log that we've wiped the cache + openlog('apache', 'cons,pid', 'local4'); + syslog('notice', encode_utf8("[memcached] $name cache cleared")); + closelog(); +} + +sub _global_prefix { + return $_[0]->_prefix("global"); +} + +sub _config_prefix { + return $_[0]->_prefix("config"); } sub _encode_key { my ($self, $key) = @_; - $key = $self->_prefix . ':' . uri_escape_utf8($key); + $key = $self->_global_prefix . ':' . uri_escape_utf8($key); return length($self->{namespace} . $key) > MAX_KEY_LENGTH ? undef : $key; @@ -182,6 +234,7 @@ sub _set { ThrowCodeError('param_invalid', { function => "Bugzilla::Memcached::set", param => "value" }); } + $key = $self->_encode_key($key) or return; return $self->{memcached}->set($key, $value); @@ -198,10 +251,19 @@ sub _get { # detaint returned values # hashes and arrays are detainted just one level deep if (ref($value) eq 'HASH') { - map { defined($_) && trick_taint($_) } values %$value; + _detaint_hashref($value); } elsif (ref($value) eq 'ARRAY') { - trick_taint($_) foreach @$value; + foreach my $value (@$value) { + next unless defined $value; + # arrays of hashes are common + if (ref($value) eq 'HASH') { + _detaint_hashref($value); + } + elsif (!ref($value)) { + trick_taint($value); + } + } } elsif (!ref($value)) { trick_taint($value); @@ -209,6 +271,15 @@ sub _get { return $value; } +sub _detaint_hashref { + my ($hashref) = @_; + foreach my $value (values %$hashref) { + if (defined($value) && !ref($value)) { + trick_taint($value); + } + } +} + sub _delete { my ($self, $key) = @_; $key = $self->_encode_key($key) @@ -273,6 +344,14 @@ Lmemcached()|Bugzilla/memcached>. =head1 METHODS +=over + +=item C + +Returns true if Memcached support is available and enabled. + +=back + =head2 Setting Adds a value to Memcached. @@ -292,6 +371,13 @@ to C. This is a convenience method which allows cached data to be later retrieved by specifying the C and either the C or C. +=item C $key, data =E $data })> + +Adds the C using the C while identifying the data as part of +Bugzilla's configuration (such as fields, products, components, groups, etc). +Values set with C are automatically cleared when changes are made +to Bugzilla's configuration. + =back =head2 Getting @@ -313,6 +399,11 @@ Return C with the specified C
and C. Return C with the specified C
and C. +=item C $key })> + +Return C with the specified C from the configuration cache. See +C for more information. + =back =head2 Clearing @@ -335,6 +426,11 @@ corresponding C
and C entry. Removes C with the specified C
and C, as well as the corresponding C
and C entry. +=item C + +Removes all configuration related values from the cache. See C for +more information. + =item C Removes all values from the cache. diff --git a/Bugzilla/Milestone.pm b/Bugzilla/Milestone.pm index a5f3ed383..d8fcd79a5 100644 --- a/Bugzilla/Milestone.pm +++ b/Bugzilla/Milestone.pm @@ -108,10 +108,12 @@ sub run_create_validators { sub update { my $self = shift; + my $dbh = Bugzilla->dbh; + + $dbh->bz_start_transaction(); my $changes = $self->SUPER::update(@_); if (exists $changes->{value}) { - my $dbh = Bugzilla->dbh; # The milestone value is stored in the bugs table instead of its ID. $dbh->do('UPDATE bugs SET target_milestone = ? WHERE target_milestone = ? AND product_id = ?', @@ -121,8 +123,11 @@ 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 }); + Bugzilla->memcached->clear({ table => 'products', id => $self->product_id }); } + $dbh->bz_commit_transaction(); + Bugzilla->memcached->clear_config(); + return $changes; } @@ -130,6 +135,8 @@ sub remove_from_db { my $self = shift; my $dbh = Bugzilla->dbh; + $dbh->bz_start_transaction(); + # The default milestone cannot be deleted. if ($self->name eq $self->product->default_milestone) { ThrowUserError('milestone_is_default', { milestone => $self }); @@ -158,8 +165,9 @@ sub remove_from_db { Bugzilla->user->id, $timestamp); } } + $self->SUPER::remove_from_db(); - $dbh->do('DELETE FROM milestones WHERE id = ?', undef, $self->id); + $dbh->bz_commit_transaction(); } ################################ diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm index 936465b3f..e3e4ca229 100644 --- a/Bugzilla/Object.pm +++ b/Bugzilla/Object.pm @@ -51,6 +51,11 @@ use constant AUDIT_REMOVES => 1; # Memcached. See documentation in Bugzilla::Memcached for more information. use constant USE_MEMCACHED => 1; +# When IS_CONFIG is true, the class is used to track seldom changed +# configuration objects. This includes, but is not limited to, fields, field +# values, keywords, products, classifications, priorities, severities, etc. +use constant IS_CONFIG => 0; + # 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 # less information. @@ -69,7 +74,7 @@ sub new { return $object if $object; my ($data, $set_memcached); - if (Bugzilla->feature('memcached') + if (Bugzilla->memcached->enabled && $class->USE_MEMCACHED && ref($param) eq 'HASH' && $param->{cache}) { @@ -380,22 +385,42 @@ sub _do_list_select { my $cols = join(',', $class->_get_db_columns); my $order = $class->LIST_ORDER; - my $sql = "SELECT $cols FROM $table"; - if (defined $where) { - $sql .= " WHERE $where "; + # Unconditional requests for configuration data are cacheable. + my ($objects, $set_memcached, $memcached_key); + if (!defined $where + && Bugzilla->memcached->enabled + && $class->IS_CONFIG) + { + $memcached_key = "$class:get_all"; + $objects = Bugzilla->memcached->get_config({ key => $memcached_key }); + $set_memcached = $objects ? 0 : 1; } - $sql .= " ORDER BY $order"; - - $sql .= " $postamble" if $postamble; - - my $dbh = Bugzilla->dbh; - # Sometimes the values are tainted, but we don't want to untaint them - # for the caller. So we copy the array. It's safe to untaint because - # they're only used in placeholders here. - my @untainted = @{ $values || [] }; - trick_taint($_) foreach @untainted; - my $objects = $dbh->selectall_arrayref($sql, {Slice=>{}}, @untainted); - $class->_serialisation_keys($objects->[0]) if @$objects; + + if (!$objects) { + my $sql = "SELECT $cols FROM $table"; + if (defined $where) { + $sql .= " WHERE $where "; + } + $sql .= " ORDER BY $order"; + $sql .= " $postamble" if $postamble; + + my $dbh = Bugzilla->dbh; + # Sometimes the values are tainted, but we don't want to untaint them + # for the caller. So we copy the array. It's safe to untaint because + # they're only used in placeholders here. + my @untainted = @{ $values || [] }; + trick_taint($_) foreach @untainted; + $objects = $dbh->selectall_arrayref($sql, {Slice=>{}}, @untainted); + $class->_serialisation_keys($objects->[0]) if @$objects; + } + + if ($objects && $set_memcached) { + Bugzilla->memcached->set_config({ + key => $memcached_key, + data => $objects + }); + } + foreach my $object (@$objects) { $object = $class->new_from_hash($object); } @@ -523,8 +548,11 @@ sub update { $self->audit_log(\%changes) if $self->AUDIT_UPDATES; $dbh->bz_commit_transaction(); - Bugzilla->memcached->clear({ table => $table, id => $self->id }) - if $self->USE_MEMCACHED && @values; + if ($self->USE_MEMCACHED && @values) { + Bugzilla->memcached->clear({ table => $table, id => $self->id }); + Bugzilla->memcached->clear_config() + if $self->IS_CONFIG; + } $self->_object_cache_remove({ id => $self->id }); $self->_object_cache_remove({ name => $self->name }) if $self->name; @@ -545,8 +573,11 @@ sub remove_from_db { $self->audit_log(AUDIT_REMOVE) if $self->AUDIT_REMOVES; $dbh->do("DELETE FROM $table WHERE $id_field = ?", undef, $self->id); $dbh->bz_commit_transaction(); - Bugzilla->memcached->clear({ table => $table, id => $self->id }) - if $self->USE_MEMCACHED; + if ($self->USE_MEMCACHED) { + Bugzilla->memcached->clear({ table => $table, id => $self->id }); + Bugzilla->memcached->clear_config() + if $self->IS_CONFIG; + } $self->_object_cache_remove({ id => $self->id }); $self->_object_cache_remove({ name => $self->name }) if $self->name; undef $self; @@ -613,6 +644,13 @@ sub create { my $object = $class->insert_create_data($field_values); $dbh->bz_commit_transaction(); + if (Bugzilla->memcached->enabled + && $class->USE_MEMCACHED + && $class->IS_CONFIG) + { + Bugzilla->memcached->clear_config(); + } + return $object; } @@ -708,7 +746,7 @@ sub insert_create_data { sub get_all { my $class = shift; - return @{$class->_do_list_select()}; + return @{ $class->_do_list_select() }; } ############################### diff --git a/Bugzilla/Product.pm b/Bugzilla/Product.pm index 452ae90fc..3e74b9afd 100644 --- a/Bugzilla/Product.pm +++ b/Bugzilla/Product.pm @@ -41,6 +41,8 @@ use constant DEFAULT_CLASSIFICATION_ID => 1; #### Initialization #### ############################### +use constant IS_CONFIG => 1; + use constant DB_TABLE => 'products'; use constant DB_COLUMNS => qw( @@ -107,6 +109,7 @@ sub create { Bugzilla::Hook::process('product_end_of_create', { product => $product }); $dbh->bz_commit_transaction(); + Bugzilla->memcached->clear_config(); return $product; } @@ -262,6 +265,7 @@ sub update { # Changes have been committed. delete $self->{check_group_controls}; Bugzilla->user->clear_product_cache(); + Bugzilla->memcached->clear_config(); return $changes; } @@ -320,6 +324,7 @@ sub remove_from_db { $dbh->do("DELETE FROM products WHERE id = ?", undef, $self->id); $dbh->bz_commit_transaction(); + Bugzilla->memcached->clear_config(); # We have to delete these internal variables, else we get # the old lists of products and classifications again. diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm index ffef600de..4a82f68d0 100644 --- a/Bugzilla/Status.pm +++ b/Bugzilla/Status.pm @@ -126,11 +126,21 @@ sub _check_value { sub BUG_STATE_OPEN { my $dbh = Bugzilla->dbh; - my $cache = Bugzilla->request_cache; - $cache->{status_bug_state_open} ||= - $dbh->selectcol_arrayref('SELECT value FROM bug_status - WHERE is_open = 1'); - return @{ $cache->{status_bug_state_open} }; + my $request_cache = Bugzilla->request_cache; + my $cache_key = 'status_bug_state_open'; + return @{ $request_cache->{$cache_key} } + if exists $request_cache->{$cache_key}; + + my $rows = Bugzilla->memcached->get_config({ key => $cache_key }); + if (!$rows) { + $rows = $dbh->selectcol_arrayref( + 'SELECT value FROM bug_status WHERE is_open = 1' + ); + Bugzilla->memcached->set_config({ key => $cache_key, data => $rows }); + } + + $request_cache->{$cache_key} = $rows; + return @$rows; } # Tells you whether or not the argument is a valid "open" state. diff --git a/Bugzilla/User.pm b/Bugzilla/User.pm index 98aa78e0a..4c0b18250 100644 --- a/Bugzilla/User.pm +++ b/Bugzilla/User.pm @@ -683,13 +683,23 @@ sub groups { FROM user_group_map WHERE user_id = ? AND isbless = 0}, undef, $self->id); - my $rows = $dbh->selectall_arrayref( - "SELECT DISTINCT grantor_id, member_id - FROM group_group_map - WHERE grant_type = " . GROUP_MEMBERSHIP); + my $cache_key = 'group_grant_type_' . GROUP_MEMBERSHIP; + my $membership_rows = Bugzilla->memcached->get_config({ + key => $cache_key, + }); + if (!$membership_rows) { + $membership_rows = $dbh->selectall_arrayref( + "SELECT DISTINCT grantor_id, member_id + FROM group_group_map + WHERE grant_type = " . GROUP_MEMBERSHIP); + Bugzilla->memcached->set_config({ + key => $cache_key, + data => $membership_rows, + }); + } my %group_membership; - foreach my $row (@$rows) { + foreach my $row (@$membership_rows) { my ($grantor_id, $member_id) = @$row; push (@{ $group_membership{$member_id} }, $grantor_id); } -- cgit v1.2.3-24-g4f1b