Closed Bug 966180 Opened 7 years ago Closed 7 years ago

backport bug 956233 to bmo (enable USE_MEMCACHE on most objects)

Categories

(bugzilla.mozilla.org :: General, defect, P1)

Production
x86
macOS
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: glob, Assigned: glob)

References

Details

(Keywords: bmo-goal)

Attachments

(2 files)

No description provided.
Priority: -- → P1
Preliminary Q1 milestone is to have this completed by Feb 28 (next Friday). Does that still seem realistic or should I adjust? I don't know how much work is yet to be done or what the blockers might be. :)
(In reply to Jake Maul [:jakem] from comment #1)
> Preliminary Q1 milestone is to have this completed by Feb 28 (next Friday).
> Does that still seem realistic or should I adjust?

having a patch ready is achievable for the current timeframe.

the bulk of the work has already happened in the upstream bug.  the work here centres around another pass at the code to ensure the upstream patch is sound, and auditing our extension code to remove objects from memcached if required.
Attached patch 966180_1.patch β€” β€” Splinter Review
most of this is a straight back-port.

i'd added clearing memcached entries for relevant contrib/reorg scripts, and tagged most of the objects used in the Push extension to not be cached in memcached (as there's no benefit).
Attachment #8378824 - Flags: review?(dkl)
Comment on attachment 8378824 [details] [diff] [review]
966180_1.patch

See bug 975896.
Attachment #8378824 - Flags: review?(dkl) → review-
i also somehow missed editcomments in my patch.  i'll check all extensions before the next revision.
Depends on: 975896
Attached patch 966180_2.patch β€” β€” Splinter Review
- incorporates the fix from bug 975896
- adds caching clearing to extensions
Attachment #8380707 - Flags: review?(dkl)
Status: NEW → ASSIGNED
Comment on attachment 8380707 [details] [diff] [review]
966180_2.patch

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

r=dkl for all of the changes so far.

Do we also need to clear in the following places?

extensions/UserProfile/lib/Util.pm:
tag_for_recount_from_bug (profiles)
update_statistics_by_user (profiles)

Also are we worried about any extension code that updates the bugs table directly (LastResolved, ShadowBugs, Voting, etc.) even though we are
not caching bugs yet? Should we at least add Bugzilla->memcache->clear({ table => 'bugs', id => $bug_id }) to be safe? That way it would be
there if/when we do enable bug caching. There are other places where you are clearing even though we do not currently cache such as flagtypes.

dkl

::: Bugzilla/Auth/Verify/DB.pm
@@ +107,1 @@
>  }

Strangely I do not see anywhere in the code where change_password() is used anymore. Normally we use $user->set_password($cryptpassword)->update() everywhere now. But code looks correct anyway :)
Attachment #8380707 - Flags: review?(dkl) → review+
(In reply to David Lawrence [:dkl] from comment #7)
> Do we also need to clear in the following places?
> extensions/UserProfile/lib/Util.pm:
> tag_for_recount_from_bug (profiles)
> update_statistics_by_user (profiles)

it shouldn't matter, because they only deal with last_statistics_ts, which appears to always be read directly from the db rather than from the object.

that said, i've added the memcached stuff for consistency.

> Also are we worried about any extension code that updates the bugs table directly?

let's worry about that when we start caching bug objects.
there's a lot of places that need to be touched, and i don't want to get too far ahead of ourselves.

> There are other places where you are clearing even though we do not currently
> cache such as flagtypes.

while there isn't any code which currently creates a flagtype object in a way that will populate the cache, it isn't explicitly disabled for this object so there may be cacheable obejcts created in future code.


Committing to: bzr+ssh://bjones%40mozilla.com@bzr.mozilla.org/bmo/4.2/
modified buglist.cgi
modified editclassifications.cgi
modified editusers.cgi
modified query.cgi
modified sanitycheck.cgi
modified userprefs.cgi
modified Bugzilla/Attachment.pm
modified Bugzilla/Bug.pm
modified Bugzilla/Classification.pm
modified Bugzilla/Field.pm
modified Bugzilla/Flag.pm
modified Bugzilla/FlagType.pm
modified Bugzilla/Memcached.pm
modified Bugzilla/Milestone.pm
modified Bugzilla/Object.pm
modified Bugzilla/User.pm
modified Bugzilla/Auth/Verify.pm
modified Bugzilla/Auth/Verify/DB.pm
modified Bugzilla/Comment/TagWeights.pm
modified Bugzilla/Install/Requirements.pm
modified Bugzilla/Search/Recent.pm
modified Bugzilla/Search/Saved.pm
modified contrib/merge-users.pl
missing contrib/reorg-tools/bmo-plan.txt
deleted contrib/reorg-tools/bmo-plan.txt
modified contrib/reorg-tools/convert_date_time_date.pl
modified contrib/reorg-tools/fix_all_open_status_queries.pl
modified contrib/reorg-tools/fixgroupqueries.pl
modified contrib/reorg-tools/fixqueries.pl
modified contrib/reorg-tools/migrate_crash_signatures.pl
modified contrib/reorg-tools/migrate_orange_bugs.pl
modified contrib/reorg-tools/move_flag_types.pl
modified contrib/reorg-tools/movebugs.pl
modified contrib/reorg-tools/movecomponent.pl
modified contrib/reorg-tools/reset_default_user.pl
modified extensions/ContributorEngagement/Extension.pm
modified extensions/EditComments/Extension.pm
modified extensions/FlagDefaultRequestee/Extension.pm
modified extensions/Push/lib/BacklogMessage.pm
modified extensions/Push/lib/Backoff.pm
modified extensions/Push/lib/LogEntry.pm
modified extensions/Push/lib/Message.pm
modified extensions/Review/Extension.pm
modified extensions/Review/lib/Util.pm
modified extensions/TagNewUsers/Extension.pm
modified extensions/UserProfile/Extension.pm
modified extensions/UserProfile/bin/update.pl
modified extensions/UserProfile/lib/Util.pm
Committed revision 9262.
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.