summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorByron Jones <glob@mozilla.com>2014-04-07 10:38:31 +0200
committerByron Jones <glob@mozilla.com>2014-04-07 10:38:31 +0200
commit3ef245b21a397e5253ba1bda607a1510245d5c09 (patch)
tree50175bb633117c7ae264a2e13872ead78ce91530
parentdc9486b05615630a5d6165d8a1bd256284c19268 (diff)
downloadbugzilla-3ef245b21a397e5253ba1bda607a1510245d5c09.tar.gz
bugzilla-3ef245b21a397e5253ba1bda607a1510245d5c09.tar.xz
Bug 987032: allow memcached to cache bugzilla configuration information
r=dkl, a=glob
-rw-r--r--Bugzilla/Classification.pm3
-rw-r--r--Bugzilla/Field.pm3
-rw-r--r--Bugzilla/Field/Choice.pm2
-rw-r--r--Bugzilla/Group.pm2
-rw-r--r--Bugzilla/Keyword.pm2
-rw-r--r--Bugzilla/Memcached.pm124
-rw-r--r--Bugzilla/Milestone.pm1
-rw-r--r--Bugzilla/Object.pm80
-rw-r--r--Bugzilla/Product.pm2
-rw-r--r--Bugzilla/Status.pm20
-rw-r--r--Bugzilla/User.pm20
-rwxr-xr-xcontrib/convert-workflow.pl1
-rwxr-xr-xeditclassifications.cgi1
13 files changed, 216 insertions, 45 deletions
diff --git a/Bugzilla/Classification.pm b/Bugzilla/Classification.pm
index 6e88bdc63..0ae548bb6 100644
--- a/Bugzilla/Classification.pm
+++ b/Bugzilla/Classification.pm
@@ -23,6 +23,8 @@ use parent qw(Bugzilla::Field::ChoiceInterface Bugzilla::Object Exporter);
#### Initialization ####
###############################
+use constant IS_CONFIG => 1;
+
use constant DB_TABLE => 'classifications';
use constant LIST_ORDER => 'sortkey, name';
@@ -67,6 +69,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 1c9927bf4..6289f110a 100644
--- a/Bugzilla/Field.pm
+++ b/Bugzilla/Field.pm
@@ -74,6 +74,8 @@ use Scalar::Util qw(blessed);
#### Initialization ####
###############################
+use constant IS_CONFIG => 1;
+
use constant DB_TABLE => 'fielddefs';
use constant LIST_ORDER => 'sortkey, name';
@@ -1078,6 +1080,7 @@ sub create {
unless $is_obsolete;
Bugzilla->memcached->clear({ table => 'fielddefs', id => $field->id });
+ Bugzilla->memcached->clear_config();
}
return $field;
diff --git a/Bugzilla/Field/Choice.pm b/Bugzilla/Field/Choice.pm
index b3bca0dd2..6f730072f 100644
--- a/Bugzilla/Field/Choice.pm
+++ b/Bugzilla/Field/Choice.pm
@@ -24,6 +24,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 25a792417..534313d9c 100644
--- a/Bugzilla/Group.pm
+++ b/Bugzilla/Group.pm
@@ -21,6 +21,8 @@ use Bugzilla::Config qw(:admin);
##### Module Initialization ###
###############################
+use constant IS_CONFIG => 1;
+
use constant DB_COLUMNS => qw(
groups.id
groups.name
diff --git a/Bugzilla/Keyword.pm b/Bugzilla/Keyword.pm
index 77c9f5f77..a8c098854 100644
--- a/Bugzilla/Keyword.pm
+++ b/Bugzilla/Keyword.pm
@@ -19,6 +19,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 0752bcce9..2bf7f1393 100644
--- a/Bugzilla/Memcached.pm
+++ b/Bugzilla/Memcached.pm
@@ -40,6 +40,10 @@ sub _new {
return bless($self, $class);
}
+sub enabled {
+ return $_[0]->{memcached} ? 1 : 0;
+}
+
sub set {
my ($self, $args) = @_;
return unless $self->{memcached};
@@ -95,6 +99,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};
@@ -130,39 +160,61 @@ 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");
+}
+
+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->{memcached_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());
+ }
+ delete Bugzilla->request_cache->{"memcached_prefix_$name"};
+}
+
+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;
@@ -175,6 +227,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);
@@ -191,10 +244,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);
@@ -202,6 +264,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)
@@ -266,6 +337,14 @@ L<Bugzilla-E<gt>memcached()|Bugzilla/memcached>.
=head1 METHODS
+=over
+
+=item C<enabled>
+
+Returns true if Memcached support is available and enabled.
+
+=back
+
=head2 Setting
Adds a value to Memcached.
@@ -285,6 +364,13 @@ to C<undef>.
This is a convenience method which allows cached data to be later retrieved by
specifying the C<table> and either the C<id> or C<name>.
+=item C<set_config({ key =E<gt> $key, data =E<gt> $data })>
+
+Adds the C<data> using the C<key> while identifying the data as part of
+Bugzilla's configuration (such as fields, products, components, groups, etc).
+Values set with C<set_config> are automatically cleared when changes are made
+to Bugzilla's configuration.
+
=back
=head2 Getting
@@ -306,6 +392,11 @@ Return C<value> with the specified C<table> and C<id>.
Return C<value> with the specified C<table> and C<name>.
+=item C<get_config({ key =E<gt> $key })>
+
+Return C<value> with the specified C<key> from the configuration cache. See
+C<set_config> for more information.
+
=back
=head2 Clearing
@@ -328,6 +419,11 @@ corresponding C<table> and C<name> entry.
Removes C<value> with the specified C<table> and C<name>, as well as the
corresponding C<table> and C<id> entry.
+=item C<clear_config>
+
+Removes all configuration related values from the cache. See C<set_config> for
+more information.
+
=item C<clear_all>
Removes all values from the cache.
diff --git a/Bugzilla/Milestone.pm b/Bugzilla/Milestone.pm
index b1783de88..daa362c34 100644
--- a/Bugzilla/Milestone.pm
+++ b/Bugzilla/Milestone.pm
@@ -114,6 +114,7 @@ sub update {
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_config();
}
$dbh->bz_commit_transaction();
diff --git a/Bugzilla/Object.pm b/Bugzilla/Object.pm
index ff1454539..e5704b3b1 100644
--- a/Bugzilla/Object.pm
+++ b/Bugzilla/Object.pm
@@ -38,6 +38,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.
@@ -56,7 +61,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})
{
@@ -352,22 +357,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);
}
@@ -495,8 +520,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;
@@ -517,8 +545,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;
@@ -585,6 +616,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;
}
@@ -680,7 +718,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 c3c3416b9..55c4de0b8 100644
--- a/Bugzilla/Product.pm
+++ b/Bugzilla/Product.pm
@@ -34,6 +34,8 @@ use constant DEFAULT_CLASSIFICATION_ID => 1;
#### Initialization ####
###############################
+use constant IS_CONFIG => 1;
+
use constant DB_TABLE => 'products';
use constant DB_COLUMNS => qw(
diff --git a/Bugzilla/Status.pm b/Bugzilla/Status.pm
index 964df9070..1f8862a36 100644
--- a/Bugzilla/Status.pm
+++ b/Bugzilla/Status.pm
@@ -108,11 +108,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 4f047ebdd..c37ab23f3 100644
--- a/Bugzilla/User.pm
+++ b/Bugzilla/User.pm
@@ -675,13 +675,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);
}
diff --git a/contrib/convert-workflow.pl b/contrib/convert-workflow.pl
index fdcab977f..8f76dac7f 100755
--- a/contrib/convert-workflow.pl
+++ b/contrib/convert-workflow.pl
@@ -151,6 +151,7 @@ if ($enable_unconfirmed) {
$dbh->do('UPDATE products SET allows_unconfirmed = 1');
}
$dbh->bz_commit_transaction();
+Bugzilla->memcached->clear_all();
print <<END;
Done. There are some things you may want to fix, now:
diff --git a/editclassifications.cgi b/editclassifications.cgi
index 149433b25..b743433b7 100755
--- a/editclassifications.cgi
+++ b/editclassifications.cgi
@@ -222,6 +222,7 @@ if ($action eq 'reclassify') {
foreach my $name (@names) {
Bugzilla->memcached->clear({ table => 'products', name => $name });
}
+ Bugzilla->memcached->clear_config();
LoadTemplate($action);
}