launchpad-reviewers team mailing list archive
-
launchpad-reviewers team
-
Mailing list archive
-
Message #06208
[Merge] lp:~wgrant/launchpad/no-representationcache into lp:launchpad
William Grant has proposed merging lp:~wgrant/launchpad/no-representationcache into lp:launchpad.
Requested reviews:
Launchpad code reviewers (launchpad-reviewers)
Related bugs:
Bug #780835 in lazr.restful: "representation cache a pessimization"
https://bugs.launchpad.net/lazr.restful/+bug/780835
For more details, see:
https://code.launchpad.net/~wgrant/launchpad/no-representationcache/+merge/90815
This branch removes LP support for the lazr.restful representation cache, which has never been used. Its invalidation system is flawed, among other issues (outlined in bug #780835). Profiling also indicates that the current invalidate-on-flush implementation slows down some DB operations by more than 30%.
--
https://code.launchpad.net/~wgrant/launchpad/no-representationcache/+merge/90815
Your team Launchpad code reviewers is requested to review the proposed merge of lp:~wgrant/launchpad/no-representationcache into lp:launchpad.
=== modified file 'configs/development/launchpad-lazr.conf'
--- configs/development/launchpad-lazr.conf 2012-01-01 02:25:04 +0000
+++ configs/development/launchpad-lazr.conf 2012-01-31 01:32:30 +0000
@@ -302,9 +302,6 @@
[vhost.api]
hostname: api.launchpad.dev
rooturl: https://api.launchpad.dev/
-# Turn this on once we've solved cache invalidation problems and are
-# ready to test.
-# enable_server_side_representation_cache: True
[vhost.blueprints]
hostname: blueprints.launchpad.dev
=== modified file 'lib/lp/registry/stories/webservice/xx-project-registry.txt'
--- lib/lp/registry/stories/webservice/xx-project-registry.txt 2012-01-23 20:12:30 +0000
+++ lib/lp/registry/stories/webservice/xx-project-registry.txt 2012-01-31 01:32:30 +0000
@@ -1264,8 +1264,6 @@
>>> print mmm.commercial_subscription.product.name
mega-money-maker
- >>> ws_uncache(mmm)
-
>>> logout()
>>> mmm = webservice.get("/mega-money-maker").jsonBody()
>>> print mmm['display_name']
=== modified file 'lib/lp/services/config/schema-lazr.conf'
--- lib/lp/services/config/schema-lazr.conf 2012-01-18 01:46:02 +0000
+++ lib/lp/services/config/schema-lazr.conf 2012-01-31 01:32:30 +0000
@@ -1879,9 +1879,6 @@
openid_delegate_profile: False
[vhost.api]
-enable_server_side_representation_cache: False
-# By default, cache representations for 4 hours.
-representation_cache_expiration_time: 14400
[vhost.blueprints]
=== modified file 'lib/lp/services/database/sqlbase.py'
--- lib/lp/services/database/sqlbase.py 2011-12-30 06:14:56 +0000
+++ lib/lp/services/database/sqlbase.py 2012-01-31 01:32:30 +0000
@@ -30,7 +30,6 @@
from datetime import datetime
-from lazr.restful.interfaces import IRepresentationCache
import psycopg2
from psycopg2.extensions import (
ISOLATION_LEVEL_AUTOCOMMIT,
@@ -248,11 +247,6 @@
"""Inverse of __eq__."""
return not (self == other)
- def __storm_flushed__(self):
- """Invalidate the web service cache."""
- cache = getUtility(IRepresentationCache)
- cache.delete(self)
-
def __storm_invalidated__(self):
"""Flush cached properties."""
# XXX: RobertCollins 2010-08-16 bug=622648: Note this is not directly
=== modified file 'lib/lp/services/database/stormbase.py'
--- lib/lp/services/database/stormbase.py 2011-01-24 19:46:53 +0000
+++ lib/lp/services/database/stormbase.py 2012-01-31 01:32:30 +0000
@@ -6,7 +6,6 @@
'StormBase',
]
-from lazr.restful.interfaces import IRepresentationCache
from storm.base import Storm
from zope.component import getUtility
@@ -21,11 +20,6 @@
# XXX: jcsackett 2011-01-20 bug=622648: Much as with the SQLBase,
# this is not tested.
- def __storm_flushed__(self):
- """Invalidate the web service cache."""
- cache = getUtility(IRepresentationCache)
- cache.delete(self)
-
def __storm_invalidated__(self):
"""Flush cached properties."""
clear_property_cache(self)
=== removed file 'lib/lp/services/memcache/doc/restful-cache.txt'
--- lib/lp/services/memcache/doc/restful-cache.txt 2011-12-29 05:29:36 +0000
+++ lib/lp/services/memcache/doc/restful-cache.txt 1970-01-01 00:00:00 +0000
@@ -1,143 +0,0 @@
-****************************************
-The Storm/memcached representation cache
-****************************************
-
-The web service library lazr.restful will store the representations it
-generates in a cache, if a suitable cache implementation is
-provided. We implement a cache that stores representations of Storm
-objects in memcached.
-
- >>> login('foo.bar@xxxxxxxxxxxxx')
-
- >>> from lp.services.memcache.restful import (
- ... MemcachedStormRepresentationCache)
- >>> cache = MemcachedStormRepresentationCache()
-
-An object's cache key is derived from its Storm metadata: its database
-table name and its primary key.
-
- >>> from zope.component import getUtility
- >>> from lp.registry.interfaces.person import IPersonSet
- >>> person = getUtility(IPersonSet).getByName('salgado')
-
- >>> cache_key = cache.key_for(
- ... person, 'media/type', 'web-service-version')
- >>> print person.id, cache_key
- 29 Person(29,),testrunner...,media/type,web-service-version
-
- >>> from operator import attrgetter
- >>> languages = sorted(person.languages, key=attrgetter('englishname'))
- >>> for language in languages:
- ... cache_key = cache.key_for(
- ... language, 'media/type', 'web-service-version')
- ... print language.id, cache_key
- 119 Language(119,),testrunner...,media/type,web-service-version
- 521 Language(521,),testrunner...,media/type,web-service-version
-
-The cache starts out empty.
-
- >>> json_type = 'application/json'
-
- >>> print cache.get(person, json_type, "v1", default="missing")
- missing
-
-Add a representation to the cache, and you can retrieve it later.
-
- >>> cache.set(person, json_type, "beta",
- ... "This is a representation for version beta.")
-
- >>> print cache.get(person, json_type, "beta")
- This is a representation for version beta.
-
-If an object has no Storm metadata, it is currently not cached at all.
-
- >>> from lp.hardwaredb.interfaces.hwdb import IHWDBApplication
- >>> hwdb_app = getUtility(IHWDBApplication)
- >>> cache.set(hwdb_app, 'media/type', 'web-service-version', 'data')
- >>> print cache.get(hwdb_app, 'media/type', 'web-service-version')
- None
-
-A single object can cache different representations for different
-web service versions.
-
- >>> cache.set(person, json_type, '1.0',
- ... 'This is a different representation for version 1.0.')
-
- >>> print cache.get(person, json_type, "1.0")
- This is a different representation for version 1.0.
-
-The web service version doesn't have to actually be defined in the
-configuration. (But you shouldn't use this--see below!)
-
- >>> cache.set(person, json_type, 'no-such-version',
- ... 'This is a representation for a nonexistent version.')
-
- >>> print cache.get(person, json_type, "no-such-version")
- This is a representation for a nonexistent version.
-
-A single object can also cache different representations for different
-media types, not just application/json. (But you shouldn't use
-this--see below!)
-
- >>> cache.set(person, 'media/type', '1.0',
- ... 'This is a representation for a strange media type.')
-
- >>> print cache.get(person, "media/type", "1.0")
- This is a representation for a strange media type.
-
-When a Launchpad object is modified, its JSON representations for
-recognized web service versions are automatically removed from the
-cache.
-
- >>> person.homepage_content = "Whatever"
- >>> from storm.store import Store
- >>> Store.of(person).flush()
-
- >>> print cache.get(person, json_type, "beta", default="missing")
- missing
-
- >>> print cache.get(person, json_type, "1.0", default="missing")
- missing
-
-But non-JSON representations, and representations for unrecognized web
-service versions, are _not_ removed from the cache. (This is why you
-shouldn't put such representations into the cache.)
-
- >>> print cache.get(person, json_type, "no-such-version")
- This is a representation for a nonexistent version.
-
- >>> print cache.get(person, "media/type", "1.0")
- This is a representation for a strange media type.
-
-Expiration time
-===============
-
-Objects will automatically be purged from the cache after a
-configurable time. The default is 4 hours (14400 seconds)
-
- >>> from lp.services.config import config
- >>> print config.vhost.api.representation_cache_expiration_time
- 14400
-
-Let's change that.
-
- >>> from lp.services.config import config
- >>> config.push('short expiration time', """\
- ... [vhost.api]
- ... representation_cache_expiration_time: 1
- ... """)
-
- >>> cache.set(person, "some/type", "version", "Some representation")
- >>> print cache.get(person, "some/type", "version")
- Some representation
-
-Now objects are purged from the cache after one second.
-
- >>> import time
- >>> time.sleep(2)
- >>> print cache.get(person, "some/type", "version")
- None
-
-Cleanup.
-
- >>> ignore = config.pop("short expiration time")
=== removed file 'lib/lp/services/memcache/restful.py'
--- lib/lp/services/memcache/restful.py 2011-12-29 05:29:36 +0000
+++ lib/lp/services/memcache/restful.py 1970-01-01 00:00:00 +0000
@@ -1,61 +0,0 @@
-# Copyright 2010 Canonical Ltd. This software is licensed under the
-# GNU Affero General Public License version 3 (see the file LICENSE).
-
-"""Storm/memcached implementation of lazr.restful's representation cache."""
-
-from lazr.restful.simple import BaseRepresentationCache
-import storm
-from zope.component import getUtility
-from zope.security.proxy import removeSecurityProxy
-
-from lp.services.config import config
-from lp.services.memcache.interfaces import IMemcacheClient
-
-
-__metaclass__ = type
-__all__ = [
- 'MemcachedStormRepresentationCache',
-]
-
-
-class MemcachedStormRepresentationCache(BaseRepresentationCache):
- """Caches lazr.restful representations of Storm objects in memcached."""
-
- def __init__(self):
- """Initialize with the memcached client."""
- self.client = getUtility(IMemcacheClient)
-
- def key_for(self, obj, media_type, version):
- """See `BaseRepresentationCache`."""
- obj = removeSecurityProxy(obj)
- try:
- storm_info = storm.info.get_obj_info(obj)
- except storm.exceptions.ClassInfoError:
- # There's no Storm data for this object. Don't cache it,
- # since we don't know how to invalidate the cache.
- return self.DO_NOT_CACHE
- table_name = storm_info.cls_info.table.name
- primary_key = tuple(var.get() for var in storm_info.primary_vars)
- identifier = table_name + repr(primary_key)
-
- key = (identifier
- + ',' + config._instance_name
- + ',' + media_type + ',' + str(version)).replace(' ', '.')
- return key
-
- def get_by_key(self, key, default=None):
- """See `BaseRepresentationCache`."""
- value = self.client.get(key)
- if value is None:
- value = default
- return value
-
- def set_by_key(self, key, value):
- """See `BaseRepresentationCache`."""
- self.client.set(
- key, value,
- time=config.vhost.api.representation_cache_expiration_time)
-
- def delete_by_key(self, key):
- """See `BaseRepresentationCache`."""
- self.client.delete(key)
=== modified file 'lib/lp/services/memcache/tests/test_doc.py'
--- lib/lp/services/memcache/tests/test_doc.py 2012-01-01 02:58:52 +0000
+++ lib/lp/services/memcache/tests/test_doc.py 2012-01-31 01:32:30 +0000
@@ -71,7 +71,6 @@
special = {
'tales-cache.txt': suite_for_doctest('tales-cache.txt'),
- 'restful-cache.txt': suite_for_doctest('restful-cache.txt'),
}
=== modified file 'lib/lp/services/webservice/configuration.py'
--- lib/lp/services/webservice/configuration.py 2012-01-06 23:05:37 +0000
+++ lib/lp/services/webservice/configuration.py 2012-01-31 01:32:30 +0000
@@ -29,6 +29,7 @@
view_permission = "launchpad.LimitedView"
require_explicit_versions = True
compensate_for_mod_compress_etag_modification = True
+ enable_server_side_representation_cache = False
service_description = """The Launchpad web service allows automated
clients to access most of the functionality available on the
@@ -70,10 +71,6 @@
return request
@property
- def enable_server_side_representation_cache(self):
- return config.vhost.api.enable_server_side_representation_cache
-
- @property
def default_batch_size(self):
return config.launchpad.default_batch_size
=== modified file 'lib/lp/services/webservice/configure.zcml'
--- lib/lp/services/webservice/configure.zcml 2011-12-24 17:49:30 +0000
+++ lib/lp/services/webservice/configure.zcml 2012-01-31 01:32:30 +0000
@@ -17,11 +17,6 @@
provides="lazr.restful.interfaces.IWebServiceConfiguration">
</utility>
- <utility
- factory="lp.services.memcache.restful.MemcachedStormRepresentationCache"
- provides="lazr.restful.interfaces.IRepresentationCache">
- </utility>
-
<securedutility
class="lp.systemhomes.WebServiceApplication"
provides="lp.services.webservice.interfaces.IWebServiceApplication">
=== removed file 'lib/lp/services/webservice/stories/cache.txt'
--- lib/lp/services/webservice/stories/cache.txt 2011-12-29 05:29:36 +0000
+++ lib/lp/services/webservice/stories/cache.txt 1970-01-01 00:00:00 +0000
@@ -1,77 +0,0 @@
-************************
-The representation cache
-************************
-
-Launchpad stores JSON representations of objects in a memcached
-cache. The full cache functionality is tested in lazr.restful and in
-lib/lp/services/memcache/doc/restful-cache.txt. This is just a simple
-integration test.
-
-By default, the cache is disabled, even in the testrunner
-environment. (This will change once we improve the Launchpad cache's
-cache invalidation.) Let's enable the cache just for this test.
-
- >>> from lp.services.config import config
- >>> config.vhost.api.enable_server_side_representation_cache
- False
- >>> config.push('enable cache', """\
- ... [vhost.api]
- ... enable_server_side_representation_cache: True
- ... """)
-
-Now we need to get a reference to the cache object, so we can look
-inside.
-
- >>> from zope.component import getUtility
- >>> from lazr.restful.interfaces import IRepresentationCache
- >>> cache = getUtility(IRepresentationCache)
-
-Since the cache is keyed by the underlying database object, we also
-need one of those objects.
-
- >>> from lp.registry.interfaces.person import IPersonSet
- >>> login(ANONYMOUS)
- >>> person = getUtility(IPersonSet).getByName('salgado')
- >>> key = cache.key_for(person, 'application/json', 'devel')
- >>> logout()
-
-The cache starts out empty.
-
- >>> print cache.get_by_key(key)
- None
-
-Retrieving a representation of an object populates the cache.
-
- >>> ignore = webservice.get("/~salgado", api_version="devel").jsonBody()
-
- >>> cache.get_by_key(key)
- '{...}'
-
-Once the cache is populated with a representation, the cached
-representation is used in preference to generating a new
-representation of that object. We can verify this by putting a fake
-value into the cache and retrieving a representation of the
-corresponding object.
-
- >>> import simplejson
- >>> cache.set_by_key(key, simplejson.dumps("Fake representation"))
-
- >>> print webservice.get("/~salgado", api_version="devel").jsonBody()
- Fake representation
-
-If there's a problem with the cache or the invalidation code, we can
-disable the cache by setting a configuration variable.
-
-Cleanup: re-disable the cache.
-
- >>> ignore = config.pop('enable cache')
-
-Note that documents are never served from a disabled cache, even if the
-cache is populated.
-
- >>> print webservice.get("/~salgado", api_version="devel").jsonBody()
- {...}
-
-Cleanup: clear the cache.
-
- >>> cache.delete(person)
=== modified file 'lib/lp/soyuz/stories/webservice/xx-builds.txt'
--- lib/lp/soyuz/stories/webservice/xx-builds.txt 2011-12-24 17:49:30 +0000
+++ lib/lp/soyuz/stories/webservice/xx-builds.txt 2012-01-31 01:32:30 +0000
@@ -105,7 +105,6 @@
>>> build = getUtility(IBinaryPackageBuildSet).getByID(26)
>>> build.upload_log = build.log
- >>> ws_uncache(build)
>>> logout()
=== modified file 'lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt'
--- lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt 2012-01-20 13:23:01 +0000
+++ lib/lp/soyuz/stories/webservice/xx-source-package-publishing.txt 2012-01-31 01:32:30 +0000
@@ -160,8 +160,6 @@
... pub.sourcepackagerelease.dscsigningkey = None
>>> logout()
- >>> ws_uncache(pub)
-
Query the source again:
>>> pubs = webservice.named_get(
=== modified file 'lib/lp/testing/pages.py'
--- lib/lp/testing/pages.py 2012-01-18 12:04:16 +0000
+++ lib/lp/testing/pages.py 2012-01-31 01:32:30 +0000
@@ -30,7 +30,6 @@
OAuthSignatureMethod_PLAINTEXT,
OAuthToken,
)
-from lazr.restful.interfaces import IRepresentationCache
from lazr.restful.testing.webservice import WebServiceCaller
import transaction
from zope.app.testing.functional import (
@@ -712,16 +711,6 @@
return LaunchpadWebServiceCaller(consumer_key, access_token.key)
-def ws_uncache(obj):
- """Manually remove an object from the web service representation cache.
-
- Directly modifying a data model object during a test may leave
- invalid data in the representation cache.
- """
- cache = getUtility(IRepresentationCache)
- cache.delete(obj)
-
-
def setupDTCBrowser():
"""Testbrowser configured for Distribution Translations Coordinators.
@@ -824,7 +813,6 @@
test.globs['print_tag_with_id'] = print_tag_with_id
test.globs['PageTestLayer'] = PageTestLayer
test.globs['stop'] = stop
- test.globs['ws_uncache'] = ws_uncache
# This function name doesn't follow our standard naming conventions,
