1277432 - Bugzilla::Memcached needs to support caching and clearing whole tables

Status: NEW

(Bugzilla :: Database, enhancement)

Description

[Dylan Hardison [:dylan] (he/him)]

There are a few tables (yes, whole tables) that are reasonably small enough to store in memcached. Of these, fielddefs is a good target.

glob: The prefix / incr / etc stuff that is used to quickly invalidate cache, is that technique discussed somewhere? I'd like to make sure I understand its significance before adding anything else in there.

Comment 1

[:glob ✱]

(In reply to Dylan William Hardison [:dylan] from comment #0)
> glob: The prefix / incr / etc stuff that is used to quickly invalidate
> cache, is that technique discussed somewhere?

tons of places, including the official faq: https://github.com/memcached/memcached/wiki/ProgrammingTricks#namespacing

Comment 2

[:glob ✱]

note a lot of these sorts of tables, including fielddefs, already have IS_CONFIG => 1, so core 'load whole table' and cache invalidation mechanisms already exist.

https://github.com/mozilla/webtools-bmo-bugzilla/blob/master/Bugzilla/Object.pm#L414

of course making smarter use of this could be of value (priming the cache on startup, supporting filtered requests, etc).

Comment 3

[Dylan Hardison [:dylan] (he/him)]

well, right now fielddefs is loaded from the db twice per request (for some requests, anyway).
My plan is to put caching in match() for Bugzilla::Field, for instance.

Comment 4

[:glob ✱]

(In reply to Dylan William Hardison [:dylan] from comment #3)
> well, right now fielddefs is loaded from the db twice per request (for some
> requests, anyway).
> My plan is to put caching in match() for Bugzilla::Field, for instance.

sounds like a solid plan :)

Comment 5

[Anamika_07]

Attachment: first patch

Comment 6

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8796847 [details] [diff] [review]
first patch

Please remove all the commented out code and submit another patch. :-)

Comment 7

[Anamika_07]

Attachment: Modified First Patch

Comment 8

[Dylan Hardison [:dylan] (he/him)]

note: ignoring those extra files. it was because of how we did a .git directory transplant.

Comment 9

[Anamika_07]

Attachment: Third patch

Comment 10

[Dylan Hardison [:dylan] (he/him)]

Comment on attachment 8796849 [details] [diff] [review]
Third patch

I was too involved in discovering this fix, so I'm going to ask dkl to review it.

Comment 11

[David Lawrence [:dkl]]

Comment on attachment 8796849 [details] [diff] [review]
Third patch

Review of attachment 8796849 [details] [diff] [review]:
-----------------------------------------------------------------

This could be a future issue as our memcached instance limits keys to 250 characters in length. Certain where strings
can be longer than this and the key would be truncated to 250 silently. This could cause unintended key collisions is 
some cases and we need to guarantee this doesn't happen. I would suggest we first hash the where string and use the 
hash string in the key. Then use this to get/set. Or someother method if there is one better to guarantee uniqueness 
but still always be under the size limit. 

We could use something similar to how Bugzilla::Token::issue_hash_token uses md5_hex from Digest::MD5 to hash the where
string. Maybe give that a try first.

dkl

::: Bugzilla/Object.pm
@@ +381,5 @@
>          && $class->IS_CONFIG)
>      {
> +        if ($where) {
> +            $memcached_key = "$class:where:$where";
> +        } else { 

nit:
}
else {

Comment 12

[Anamika_07]

Attachment: Revised_patch

Comment 13

[Anamika_07]

Attachment: Revised_patch

Comment 14

[Dylan Hardison [:dylan] (he/him)]

This looks promising.

Comment 15

[David Lawrence [:dkl]]

Comment on attachment 8807411 [details] [diff] [review]
Revised_patch

I cannot get to this at the moment. Dylan might be faster then I.

dkl