Last Comment Bug 993939 - Bugzilla::User::Setting::groups() should use memcached
: Bugzilla::User::Setting::groups() should use memcached
: bmo-goal, perf
Product: Bugzilla
Classification: Server Software
Component: Bugzilla-General (show other bugs)
: unspecified
: All All
-- enhancement (vote)
: Bugzilla 5.0
Assigned To: Byron Jones ‹:glob›
Depends on:
  Show dependency treegraph
Reported: 2014-04-09 01:10 PDT by Byron Jones ‹:glob›
Modified: 2014-05-27 23:41 PDT (History)
2 users (show)
justdave: approval+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

993939_1.patch (7.99 KB, patch)
2014-05-01 00:29 PDT, Byron Jones ‹:glob›
dkl: review+
Details | Diff | Splinter Review

Description User image Byron Jones ‹:glob› 2014-04-09 01:10:52 PDT
$user->groups() is called for each commenter in a bug.  this results in frequent database calls for data that changes very infrequently, making it an ideal candidate for memcached.

implementation notes:
- in Bugzilla::User::groups() 
  - build a cache key "user_groups.$user_id"
  - wrap the db check and tree walking with memcached->get_config and set_config (follow how $membership_rows is cached)
- in Bugzilla::User::update()
  - call memcached->clear_config({ key => "user_groups.$user_id" }) if login_name changed
- in editusers.cgi
  - call memcached->clear_config({ key => "user_groups.$user_id" }) if $action is update
Comment 1 User image Byron Jones ‹:glob› 2014-05-01 00:29:26 PDT
Created attachment 8415777 [details] [diff] [review]

- stores and checks a user's group membership in memcached
- extends memcached->clear_config to support clearing individual keys
- clears the cached value in $user->derive_regexp_groups()
- clears the cached value in edituser when groups membership is changed
- changes memcached's prefix delimiter from : to . to avoid escaping, making keys easier to read when debugging
Comment 2 User image David Lawrence [:dkl] 2014-05-20 13:48:52 PDT
Comment on attachment 8415777 [details] [diff] [review]

Review of attachment 8415777 [details] [diff] [review]:

Works as expected. r=dkl
Comment 3 User image Byron Jones ‹:glob› 2014-05-27 22:54:19 PDT
To ssh://
   ccff248..6d730fa  master -> master
Comment 4 User image Byron Jones ‹:glob› 2014-05-27 23:20:07 PDT
this broke the build:
Can't call method "get" on an undefined value at Bugzilla/ line 187.
Comment 5 User image Byron Jones ‹:glob› 2014-05-27 23:41:20 PDT
To ssh://
   6d730fa..b5b5061  master -> master

Note You need to log in before you can comment on or make changes to this bug.