Bugzilla::User::Setting::groups() should use memcached

RESOLVED FIXED in Bugzilla 5.0

Status

()

Bugzilla
Bugzilla-General
--
enhancement
RESOLVED FIXED
3 years ago
3 years ago

People

(Reporter: glob, Assigned: glob)

Tracking

({bmo-goal, perf})

unspecified
Bugzilla 5.0
bmo-goal, perf
Bug Flags:
approval +

Details

Attachments

(1 attachment)

(Assignee)

Description

3 years ago
$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

Updated

3 years ago
Severity: normal → enhancement
(Assignee)

Updated

3 years ago
Keywords: bmo-goal
(Assignee)

Updated

3 years ago
Status: NEW → ASSIGNED
QA Contact: default-qa → glob
(Assignee)

Updated

3 years ago
Assignee: general → glob
QA Contact: glob
(Assignee)

Comment 1

3 years ago
Created attachment 8415777 [details] [diff] [review]
993939_1.patch

- 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
Attachment #8415777 - Flags: review?(dkl)
Comment on attachment 8415777 [details] [diff] [review]
993939_1.patch

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

Works as expected. r=dkl
Attachment #8415777 - Flags: review?(dkl) → review+
(Assignee)

Updated

3 years ago
Flags: approval?
Target Milestone: --- → Bugzilla 5.0
Flags: approval? → approval+
(Assignee)

Comment 3

3 years ago
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   ccff248..6d730fa  master -> master
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
(Assignee)

Comment 4

3 years ago
this broke the build:
https://travis-ci.org/bugzilla/bugzilla/jobs/26188673
Can't call method "get" on an undefined value at Bugzilla/Memcached.pm line 187.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
(Assignee)

Comment 5

3 years ago
To ssh://gitolite3@git.mozilla.org/bugzilla/bugzilla.git
   6d730fa..b5b5061  master -> master
Status: REOPENED → RESOLVED
Last Resolved: 3 years ago3 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.