Last Comment Bug 993926 - Bugzilla::User::Setting::get_all_settings() should use memcached
: Bugzilla::User::Setting::get_all_settings() should use memcached
: bmo-goal, perf
Product: Bugzilla
Classification: Server Software
Component: Bugzilla-General (show other bugs)
: unspecified
: All All
P1 enhancement (vote)
: Bugzilla 5.0
Assigned To: Byron Jones ‹:glob›
: default-qa
Depends on:
  Show dependency treegraph
Reported: 2014-04-09 00:26 PDT by Byron Jones ‹:glob›
Modified: 2014-08-11 23:05 PDT (History)
1 user (show)
glob: approval+
See Also:
QA Whiteboard:
Iteration: ---
Points: ---

993926_1.patch (4.72 KB, patch)
2014-06-23 09:14 PDT, Byron Jones ‹:glob›
mail: review+
Details | Diff | Splinter Review

Description User image Byron Jones ‹:glob› 2014-04-09 00:26:12 PDT
due to the gravatar extension on BMO, $user->settings() is called for each commenter.  this results in frequent database calls for data that changes very infrequently, making it an ideal candidate for memcached.

changes required:
- during an update in editsettings.cgi call Bugzilla->memcached->clear_config()
- update Bugzilla::User::Settings:
  - update get_all_settings($user_id)
    - build a cache key as "user_settings.$user_id"
    - check the cache with Bugzilla->memcached->get_config($key) and return if found
    - if not found, query the database and store with memcached->set_config()
  - add a method clear_settings_cache($user_id)
    - calls memcached->clear({ key => $key }) using the same key as get_all_settings()
- call clear_settings_cache() from userprefs.cgi::SaveSettings()

i'm not totally comfortable with clear_settings_cache(), however the way settings have been implemented makes abstraction difficult (the settings hashref inside the user object is updated directly by userprefs.cgi, and using a tied hashref would be an overkill).
Comment 1 User image Byron Jones ‹:glob› 2014-06-23 09:14:09 PDT
Created attachment 8444484 [details] [diff] [review]
Comment 2 User image Byron Jones ‹:glob› 2014-08-11 23:05:51 PDT
To ssh://
   8b98912..c19dc4f  master -> master

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