Merge lp:~leonardr/launchpad/toggle-representation-cache into lp:launchpad

Proposed by Leonard Richardson
Status: Merged
Approved by: Graham Binns
Approved revision: no longer in the source branch.
Merged at revision: 11011
Proposed branch: lp:~leonardr/launchpad/toggle-representation-cache
Merge into: lp:launchpad
Diff against target: 513 lines (+331/-9)
14 files modified
configs/development/launchpad-lazr.conf (+3/-0)
lib/canonical/config/schema-lazr.conf (+3/-0)
lib/canonical/database/sqlbase.py (+7/-0)
lib/canonical/launchpad/pagetests/webservice/cache.txt (+77/-0)
lib/canonical/launchpad/rest/configuration.py (+4/-0)
lib/canonical/launchpad/testing/pages.py (+12/-0)
lib/canonical/launchpad/zcml/webservice.zcml (+5/-0)
lib/lp/registry/stories/webservice/xx-project-registry.txt (+2/-0)
lib/lp/services/memcache/doc/restful-cache.txt (+143/-0)
lib/lp/services/memcache/restful.py (+63/-0)
lib/lp/services/memcache/tests/test_doc.py (+8/-8)
lib/lp/soyuz/stories/webservice/xx-builds.txt (+1/-0)
lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt (+2/-0)
versions.cfg (+1/-1)
To merge this branch: bzr merge lp:~leonardr/launchpad/toggle-representation-cache
Reviewer Review Type Date Requested Status
Aaron Bentley (community) Approve
Graham Binns (community) code Approve
Paul Hummer (community) code Approve
Jelmer Vernooij (community) code Approve
Review via email: mp+26725@code.launchpad.net

Description of the change

This branch implements the enable_server_side_representation_cache lazr.restful configuration option using a Launchpad configuration option. That option is defined in https://code.edge.launchpad.net/~leonardr/lazr.restful/disable-cache/+merge/26713 and is still in review.

To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

In lib/lp/services/memcache/doc/restful-cache.txt you're missing an asterisk on the first line.

It'd be nice to have docstrings for the methods in MemcachedStormRepresentationCache, even if they just say "See `BaseRepositoryCache`."

review: Approve (code)
Revision history for this message
Leonard Richardson (leonardr) wrote :

I need another review of the last couple revisions. While going through stub's review of my earlier branch, I remembered that we planned to set a default expiration time for items in the cache, so that when we tested on staging the cache wouldn't contain bad data indefinitely. I've implemented that code and a test, as well as responding to Jelmer's feedback and some more of Stuart's.

(Stuart's review is here: https://code.edge.launchpad.net/~leonardr/launchpad/test-representation-cache/+merge/26513)

I couldn't think of any way to test the expiration except from actually sleeping for 2 seconds, since I don't think we have a mocked-up memcached for use in testing. Suggestions are welcome.

Revision history for this message
Paul Hummer (rockstar) :
review: Approve (code)
Revision history for this message
Leonard Richardson (leonardr) wrote :

I need one more tiny review. I got a lots of test failures because memcached thinks space is a control character. So after creating the key, I replace any spaces with periods.

Revision history for this message
Graham Binns (gmb) :
review: Approve (code)
Revision history for this message
Leonard Richardson (leonardr) wrote :

I was still getting test failures, so I made a number of changes (which required a new version of lazr.restful: see https://code.edge.launchpad.net/~leonardr/lazr.restful/cache-unauthorized/+merge/27523). Here's a paste with the contents of the two commits I've made that weren't merges against trunk:

http://pastebin.ubuntu.com/449786/

Revision history for this message
Aaron Bentley (abentley) wrote :

As discussed in IRC
- Please catch storm.exceptions.ClassInfoError rather than Exception
- If possible, narrow the scope of the related try/except block.
- Please be more specific in applying defaults, so that cached values that evaluate to False are handled correctly.

review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'configs/development/launchpad-lazr.conf'
2--- configs/development/launchpad-lazr.conf 2010-05-27 07:05:20 +0000
3+++ configs/development/launchpad-lazr.conf 2010-06-15 13:39:43 +0000
4@@ -275,6 +275,9 @@
5 [vhost.api]
6 hostname: api.launchpad.dev
7 rooturl: https://api.launchpad.dev/
8+# Turn this on once we've solved cache invalidation problems and are
9+# ready to test.
10+# enable_server_side_representation_cache: True
11
12 [vhost.blueprints]
13 hostname: blueprints.launchpad.dev
14
15=== modified file 'lib/canonical/config/schema-lazr.conf'
16--- lib/canonical/config/schema-lazr.conf 2010-06-08 15:13:20 +0000
17+++ lib/canonical/config/schema-lazr.conf 2010-06-15 13:39:43 +0000
18@@ -1950,6 +1950,9 @@
19 [vhost.api]
20 # This key should be removed once the production configs have been updated.
21 beta_test_team: disabled
22+enable_server_side_representation_cache: False
23+# By default, cache representations for 4 hours.
24+representation_cache_expiration_time: 14400
25
26 [vhost.blueprints]
27
28
29=== modified file 'lib/canonical/database/sqlbase.py'
30--- lib/canonical/database/sqlbase.py 2010-03-24 17:37:26 +0000
31+++ lib/canonical/database/sqlbase.py 2010-06-15 13:39:43 +0000
32@@ -28,6 +28,8 @@
33 from zope.interface import implements
34 from zope.security.proxy import removeSecurityProxy
35
36+from lazr.restful.interfaces import IRepresentationCache
37+
38 from canonical.config import config, dbconfig
39 from canonical.database.interfaces import ISQLBase
40
41@@ -246,6 +248,11 @@
42 """Inverse of __eq__."""
43 return not (self == other)
44
45+ def __storm_flushed__(self):
46+ """Invalidate the web service cache."""
47+ cache = getUtility(IRepresentationCache)
48+ cache.delete(self)
49+
50 alreadyInstalledMsg = ("A ZopelessTransactionManager with these settings is "
51 "already installed. This is probably caused by calling initZopeless twice.")
52
53
54=== added file 'lib/canonical/launchpad/pagetests/webservice/cache.txt'
55--- lib/canonical/launchpad/pagetests/webservice/cache.txt 1970-01-01 00:00:00 +0000
56+++ lib/canonical/launchpad/pagetests/webservice/cache.txt 2010-06-15 13:39:43 +0000
57@@ -0,0 +1,77 @@
58+************************
59+The representation cache
60+************************
61+
62+Launchpad stores JSON representations of objects in a memcached
63+cache. The full cache functionality is tested in lazr.restful and in
64+lib/lp/services/memcache/doc/restful-cache.txt. This is just a simple
65+integration test.
66+
67+By default, the cache is disabled, even in the testrunner
68+environment. (This will change once we improve the Launchpad cache's
69+cache invalidation.) Let's enable the cache just for this test.
70+
71+ >>> from canonical.config import config
72+ >>> config.vhost.api.enable_server_side_representation_cache
73+ False
74+ >>> config.push('enable cache', """\
75+ ... [vhost.api]
76+ ... enable_server_side_representation_cache: True
77+ ... """)
78+
79+Now we need to get a reference to the cache object, so we can look
80+inside.
81+
82+ >>> from zope.component import getUtility
83+ >>> from lazr.restful.interfaces import IRepresentationCache
84+ >>> cache = getUtility(IRepresentationCache)
85+
86+Since the cache is keyed by the underlying database object, we also
87+need one of those objects.
88+
89+ >>> from lp.registry.interfaces.person import IPersonSet
90+ >>> login(ANONYMOUS)
91+ >>> person = getUtility(IPersonSet).getByName('salgado')
92+ >>> key = cache.key_for(person, 'application/json', 'devel')
93+ >>> logout()
94+
95+The cache starts out empty.
96+
97+ >>> print cache.get_by_key(key)
98+ None
99+
100+Retrieving a representation of an object populates the cache.
101+
102+ >>> ignore = webservice.get("/~salgado", api_version="devel").jsonBody()
103+
104+ >>> cache.get_by_key(key)
105+ '{...}'
106+
107+Once the cache is populated with a representation, the cached
108+representation is used in preference to generating a new
109+representation of that object. We can verify this by putting a fake
110+value into the cache and retrieving a representation of the
111+corresponding object.
112+
113+ >>> import simplejson
114+ >>> cache.set_by_key(key, simplejson.dumps("Fake representation"))
115+
116+ >>> print webservice.get("/~salgado", api_version="devel").jsonBody()
117+ Fake representation
118+
119+If there's a problem with the cache or the invalidation code, we can
120+disable the cache by setting a configuration variable.
121+
122+Cleanup: re-disable the cache.
123+
124+ >>> ignore = config.pop('enable cache')
125+
126+Note that documents are never served from a disabled cache, even if the
127+cache is populated.
128+
129+ >>> print webservice.get("/~salgado", api_version="devel").jsonBody()
130+ {...}
131+
132+Cleanup: clear the cache.
133+
134+ >>> cache.delete(person)
135
136=== modified file 'lib/canonical/launchpad/rest/configuration.py'
137--- lib/canonical/launchpad/rest/configuration.py 2010-05-17 20:03:02 +0000
138+++ lib/canonical/launchpad/rest/configuration.py 2010-06-15 13:39:43 +0000
139@@ -63,6 +63,10 @@
140 return request
141
142 @property
143+ def enable_server_side_representation_cache(self):
144+ return config.vhost.api.enable_server_side_representation_cache
145+
146+ @property
147 def default_batch_size(self):
148 return config.launchpad.default_batch_size
149
150
151=== modified file 'lib/canonical/launchpad/testing/pages.py'
152--- lib/canonical/launchpad/testing/pages.py 2010-03-30 09:40:48 +0000
153+++ lib/canonical/launchpad/testing/pages.py 2010-06-15 13:39:43 +0000
154@@ -40,6 +40,7 @@
155 from canonical.launchpad.webapp.interfaces import OAuthPermission
156 from canonical.launchpad.webapp.url import urlsplit
157 from canonical.testing import PageTestLayer
158+from lazr.restful.interfaces import IRepresentationCache
159 from lazr.restful.testing.webservice import WebServiceCaller
160 from lp.testing import (
161 ANONYMOUS, launchpadlib_for, login, login_person, logout)
162@@ -672,6 +673,16 @@
163 return LaunchpadWebServiceCaller(consumer_key, access_token.key)
164
165
166+def ws_uncache(obj):
167+ """Manually remove an object from the web service representation cache.
168+
169+ Directly modifying a data model object during a test may leave
170+ invalid data in the representation cache.
171+ """
172+ cache = getUtility(IRepresentationCache)
173+ cache.delete(obj)
174+
175+
176 def setupDTCBrowser():
177 """Testbrowser configured for Distribution Translations Coordinators.
178
179@@ -776,6 +787,7 @@
180 test.globs['print_tag_with_id'] = print_tag_with_id
181 test.globs['PageTestLayer'] = PageTestLayer
182 test.globs['stop'] = stop
183+ test.globs['ws_uncache'] = ws_uncache
184
185
186 class PageStoryTestCase(unittest.TestCase):
187
188=== modified file 'lib/canonical/launchpad/zcml/webservice.zcml'
189--- lib/canonical/launchpad/zcml/webservice.zcml 2010-03-26 19:11:50 +0000
190+++ lib/canonical/launchpad/zcml/webservice.zcml 2010-06-15 13:39:43 +0000
191@@ -16,6 +16,11 @@
192 provides="lazr.restful.interfaces.IWebServiceConfiguration">
193 </utility>
194
195+ <utility
196+ factory="lp.services.memcache.restful.MemcachedStormRepresentationCache"
197+ provides="lazr.restful.interfaces.IRepresentationCache">
198+ </utility>
199+
200 <securedutility
201 class="canonical.launchpad.systemhomes.WebServiceApplication"
202 provides="canonical.launchpad.interfaces.IWebServiceApplication">
203
204=== modified file 'lib/lp/registry/stories/webservice/xx-project-registry.txt'
205--- lib/lp/registry/stories/webservice/xx-project-registry.txt 2010-06-11 18:05:59 +0000
206+++ lib/lp/registry/stories/webservice/xx-project-registry.txt 2010-06-15 13:39:43 +0000
207@@ -1250,6 +1250,8 @@
208
209 >>> logout()
210
211+ >>> from lazr.restful.interfaces import IRepresentationCache
212+ >>> ws_uncache(mmm)
213 >>> mmm = webservice.get("/mega-money-maker").jsonBody()
214 >>> print mmm['display_name']
215 Mega Money Maker
216
217=== added file 'lib/lp/services/memcache/doc/restful-cache.txt'
218--- lib/lp/services/memcache/doc/restful-cache.txt 1970-01-01 00:00:00 +0000
219+++ lib/lp/services/memcache/doc/restful-cache.txt 2010-06-15 13:39:43 +0000
220@@ -0,0 +1,143 @@
221+****************************************
222+The Storm/memcached representation cache
223+****************************************
224+
225+The web service library lazr.restful will store the representations it
226+generates in a cache, if a suitable cache implementation is
227+provided. We implement a cache that stores representations of Storm
228+objects in memcached.
229+
230+ >>> login('foo.bar@canonical.com')
231+
232+ >>> from lp.services.memcache.restful import (
233+ ... MemcachedStormRepresentationCache)
234+ >>> cache = MemcachedStormRepresentationCache()
235+
236+An object's cache key is derived from its Storm metadata: its database
237+table name and its primary key.
238+
239+ >>> from zope.component import getUtility
240+ >>> from lp.registry.interfaces.person import IPersonSet
241+ >>> person = getUtility(IPersonSet).getByName('salgado')
242+
243+ >>> cache_key = cache.key_for(
244+ ... person, 'media/type', 'web-service-version')
245+ >>> print person.id, cache_key
246+ 29 Person(29,),testrunner,media/type,web-service-version
247+
248+ >>> from operator import attrgetter
249+ >>> languages = sorted(person.languages, key=attrgetter('englishname'))
250+ >>> for language in languages:
251+ ... cache_key = cache.key_for(
252+ ... language, 'media/type', 'web-service-version')
253+ ... print language.id, cache_key
254+ 119 Language(119,),testrunner,media/type,web-service-version
255+ 521 Language(521,),testrunner,media/type,web-service-version
256+
257+The cache starts out empty.
258+
259+ >>> json_type = 'application/json'
260+
261+ >>> print cache.get(person, json_type, "v1", default="missing")
262+ missing
263+
264+Add a representation to the cache, and you can retrieve it later.
265+
266+ >>> cache.set(person, json_type, "beta",
267+ ... "This is a representation for version beta.")
268+
269+ >>> print cache.get(person, json_type, "beta")
270+ This is a representation for version beta.
271+
272+If an object has no Storm metadata, it is currently not cached at all.
273+
274+ >>> from lp.hardwaredb.interfaces.hwdb import IHWDBApplication
275+ >>> hwdb_app = getUtility(IHWDBApplication)
276+ >>> cache.set(hwdb_app, 'media/type', 'web-service-version', 'data')
277+ >>> print cache.get(hwdb_app, 'media/type', 'web-service-version')
278+ None
279+
280+A single object can cache different representations for different
281+web service versions.
282+
283+ >>> cache.set(person, json_type, '1.0',
284+ ... 'This is a different representation for version 1.0.')
285+
286+ >>> print cache.get(person, json_type, "1.0")
287+ This is a different representation for version 1.0.
288+
289+The web service version doesn't have to actually be defined in the
290+configuration. (But you shouldn't use this--see below!)
291+
292+ >>> cache.set(person, json_type, 'no-such-version',
293+ ... 'This is a representation for a nonexistent version.')
294+
295+ >>> print cache.get(person, json_type, "no-such-version")
296+ This is a representation for a nonexistent version.
297+
298+A single object can also cache different representations for different
299+media types, not just application/json. (But you shouldn't use
300+this--see below!)
301+
302+ >>> cache.set(person, 'media/type', '1.0',
303+ ... 'This is a representation for a strange media type.')
304+
305+ >>> print cache.get(person, "media/type", "1.0")
306+ This is a representation for a strange media type.
307+
308+When a Launchpad object is modified, its JSON representations for
309+recognized web service versions are automatically removed from the
310+cache.
311+
312+ >>> person.addressline1 = "New address"
313+ >>> from canonical.launchpad.ftests import syncUpdate
314+ >>> syncUpdate(person)
315+
316+ >>> print cache.get(person, json_type, "beta", default="missing")
317+ missing
318+
319+ >>> print cache.get(person, json_type, "1.0", default="missing")
320+ missing
321+
322+But non-JSON representations, and representations for unrecognized web
323+service versions, are _not_ removed from the cache. (This is why you
324+shouldn't put such representations into the cache.)
325+
326+ >>> print cache.get(person, json_type, "no-such-version")
327+ This is a representation for a nonexistent version.
328+
329+ >>> print cache.get(person, "media/type", "1.0")
330+ This is a representation for a strange media type.
331+
332+Expiration time
333+===============
334+
335+Objects will automatically be purged from the cache after a
336+configurable time. The default is 4 hours (14400 seconds)
337+
338+ >>> from canonical.config import config
339+ >>> print config.vhost.api.representation_cache_expiration_time
340+ 14400
341+
342+Let's change that.
343+
344+ >>> from canonical.config import config
345+ >>> config.push('short expiration time', """\
346+ ... [vhost.api]
347+ ... representation_cache_expiration_time: 1
348+ ... """)
349+
350+ >>> cache.set(person, "some/type", "version", "Some representation")
351+ >>> print cache.get(person, "some/type", "version")
352+ Some representation
353+
354+Now objects are purged from the cache after one second.
355+
356+ >>> import time
357+ >>> time.sleep(2)
358+ >>> print cache.get(person, "some/type", "version")
359+ None
360+
361+Cleanup.
362+
363+ >>> ignore = config.pop("short expiration time")
364
365=== added file 'lib/lp/services/memcache/restful.py'
366--- lib/lp/services/memcache/restful.py 1970-01-01 00:00:00 +0000
367+++ lib/lp/services/memcache/restful.py 2010-06-15 13:39:43 +0000
368@@ -0,0 +1,63 @@
369+# Copyright 2010 Canonical Ltd. This software is licensed under the
370+# GNU Affero General Public License version 3 (see the file LICENSE).
371+
372+"""Storm/memcached implementation of lazr.restful's representation cache."""
373+
374+import storm
375+
376+from zope.component import getUtility
377+from zope.security.proxy import removeSecurityProxy
378+from zope.traversing.browser import absoluteURL
379+
380+from canonical.config import config
381+from lp.services.memcache.interfaces import IMemcacheClient
382+from lazr.restful.simple import BaseRepresentationCache
383+from lazr.restful.utils import get_current_web_service_request
384+
385+__metaclass__ = type
386+__all__ = [
387+ 'MemcachedStormRepresentationCache',
388+]
389+
390+
391+class MemcachedStormRepresentationCache(BaseRepresentationCache):
392+ """Caches lazr.restful representations of Storm objects in memcached."""
393+
394+ def __init__(self):
395+ """Initialize with the memcached client."""
396+ self.client = getUtility(IMemcacheClient)
397+
398+ def key_for(self, obj, media_type, version):
399+ """See `BaseRepresentationCache`."""
400+ obj = removeSecurityProxy(obj)
401+ try:
402+ storm_info = storm.info.get_obj_info(obj)
403+ except storm.exceptions.ClassInfoError, e:
404+ # There's no Storm data for this object. Don't cache it,
405+ # since we don't know how to invalidate the cache.
406+ return self.DO_NOT_CACHE
407+ table_name = storm_info.cls_info.table
408+ primary_key = tuple(var.get() for var in storm_info.primary_vars)
409+ identifier = table_name + repr(primary_key)
410+
411+ key = (identifier
412+ + ',' + config._instance_name
413+ + ',' + media_type + ',' + str(version)).replace(' ', '.')
414+ return key
415+
416+ def get_by_key(self, key, default=None):
417+ """See `BaseRepresentationCache`."""
418+ value = self.client.get(key)
419+ if value is None:
420+ value = default
421+ return value
422+
423+ def set_by_key(self, key, value):
424+ """See `BaseRepresentationCache`."""
425+ self.client.set(
426+ key, value,
427+ time=config.vhost.api.representation_cache_expiration_time)
428+
429+ def delete_by_key(self, key):
430+ """See `BaseRepresentationCache`."""
431+ self.client.delete(key)
432
433=== modified file 'lib/lp/services/memcache/tests/test_doc.py'
434--- lib/lp/services/memcache/tests/test_doc.py 2010-03-03 11:00:42 +0000
435+++ lib/lp/services/memcache/tests/test_doc.py 2010-06-15 13:39:43 +0000
436@@ -7,9 +7,7 @@
437
438 import os.path
439 from textwrap import dedent
440-import unittest
441
442-from zope.component import getUtility
443 import zope.pagetemplate.engine
444 from zope.pagetemplate.pagetemplate import PageTemplate
445 from zope.publisher.browser import TestRequest
446@@ -17,9 +15,7 @@
447 from canonical.launchpad.testing.systemdocs import (
448 LayeredDocFileSuite, setUp, tearDown)
449 from canonical.testing.layers import LaunchpadFunctionalLayer, MemcachedLayer
450-from lp.services.memcache.interfaces import IMemcacheClient
451 from lp.services.testing import build_test_suite
452-from lp.testing import TestCase
453
454
455 here = os.path.dirname(os.path.realpath(__file__))
456@@ -59,11 +55,15 @@
457 test.globs['MemcachedLayer'] = MemcachedLayer
458
459
460+def suite_for_doctest(filename):
461+ return LayeredDocFileSuite(
462+ '../doc/%s' % filename,
463+ setUp=memcacheSetUp, tearDown=tearDown,
464+ layer=LaunchpadFunctionalLayer)
465+
466 special = {
467- 'tales-cache.txt': LayeredDocFileSuite(
468- '../doc/tales-cache.txt',
469- setUp=memcacheSetUp, tearDown=tearDown,
470- layer=LaunchpadFunctionalLayer),
471+ 'tales-cache.txt': suite_for_doctest('tales-cache.txt'),
472+ 'restful-cache.txt': suite_for_doctest('restful-cache.txt'),
473 }
474
475
476
477=== modified file 'lib/lp/soyuz/stories/webservice/xx-builds.txt'
478--- lib/lp/soyuz/stories/webservice/xx-builds.txt 2010-05-17 11:30:31 +0000
479+++ lib/lp/soyuz/stories/webservice/xx-builds.txt 2010-06-15 13:39:43 +0000
480@@ -93,6 +93,7 @@
481
482 >>> build = getUtility(IBinaryPackageBuildSet).getByBuildID(26)
483 >>> build.upload_log = build.log
484+ >>> ws_uncache(build)
485
486 >>> logout()
487
488
489=== modified file 'lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt'
490--- lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt 2010-02-26 14:49:00 +0000
491+++ lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt 2010-06-15 13:39:43 +0000
492@@ -152,6 +152,8 @@
493 ... pub.sourcepackagerelease.dscsigningkey = None
494 >>> logout()
495
496+ >>> ws_uncache(pub)
497+
498 Query the source again:
499
500 >>> pubs = webservice.named_get(
501
502=== modified file 'versions.cfg'
503--- versions.cfg 2010-06-09 19:45:18 +0000
504+++ versions.cfg 2010-06-15 13:39:43 +0000
505@@ -28,7 +28,7 @@
506 lazr.delegates = 1.1.0
507 lazr.enum = 1.1.2
508 lazr.lifecycle = 1.1
509-lazr.restful = 0.9.26
510+lazr.restful = 0.9.29
511 lazr.restfulclient = 0.9.14
512 lazr.smtptest = 1.1
513 lazr.testing = 0.1.1