launchpad-reviewers team mailing list archive

[Robert Collins <robertc@xxxxxxxxxxxxxxxxx>]

Robert Collins has proposed merging lp:~lifeless/launchpad/memcache into lp:launchpad.

Requested reviews:
  Curtis Hovey (sinzui): code
Related bugs:
  Bug #627033 in Launchpad itself: "warn when cached TALes in loop does not provide extra key info"
  https://bugs.launchpad.net/launchpad/+bug/627033
  Bug #869063 in Launchpad itself: ""Hide comment" link appears for comments if the comment has been viewed by an admin, with memcache on"
  https://bugs.launchpad.net/launchpad/+bug/869063

For more details, see:
https://code.launchpad.net/~lifeless/launchpad/memcache/+merge/109551

This branch does four things.

Firstly, it removes a large number of 'band-aid' memcache applications. Our early memcache use was done by putting a slow operation behind a cache-check-or-set helper. This doesn't aid performance much, and gives users poor experiences when they are the first user to use a given page. In addition, as we had no just-in-time expiry mechanism, many of these bandaid-spots resulted in bug reports, e.g. when a user cannot see a bug they just linked to a milestone on the milestone page, or cannot see new series. I can't find the bug reports just now, but I've seen them go by :). We may find there are poorly hidden queries behind things like the front page counters. If so, we should fix them - they will already be causing timeouts (and so this patch may make them more prominent, but it will in no way introduce the bug).

Secondly, it removes a couple of pathological memcache uses, where memcache made the system slower by introducing O(N) scaling to a constant page. memcache accesses cost upwards of 3ms, sometimes as much as 20 for hits, and doing that for every object in a page with hundreds of objects rapidly consumes a lot of time - more than an optimised load+render (which we have since done) requires.

Thirdly, it removes the one remaining arguable legitimate use we have, caching the blog posts on the front page; we have a squid cache that already caches this content, and is getting lots of hits. It is possible that this will turn out to be a catastrophe, but it is more likely that poor processing of object counts on the front page is actually the cause of existing timeouts.

Doing these three things permits the fourth thing, which is to remove our inappropriately layered memcache helper (which acted as a poor http-style cache, see the bugs described in #1 above), and kills a significant amount of LoC - about 1000.
-- 
https://code.launchpad.net/~lifeless/launchpad/memcache/+merge/109551
Your team Launchpad code reviewers is subscribed to branch lp:launchpad.