Perf-o-matic should memcache dashboard images
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Mar 2012 01:01:51 +0000 (01:01 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 6 Mar 2012 01:01:51 +0000 (01:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=80349

Reviewed by Eric Seidel.

Added DashboardImage.create and DashboardImage.get_image to encapsulate memcache.
Also replaced transaction in DashboardImage.set_cache by a single put since it duplicates
what put does by default.

Also removed redundant cache_* functions and merged them into handler code.

* Websites/webkit-perf.appspot.com/controller.py:
(ManifestUpdateHandler.post):
(CachedManifestHandler.get):
(DashboardUpdateHandler.post):
(CachedDashboardHandler.get):
(RunsUpdateHandler):
(RunsUpdateHandler.post):
(RunsChartHandler):
(RunsChartHandler.post):
(DashboardImageHandler.get):
* Websites/webkit-perf.appspot.com/models.py:
(PersistentCache.set_cache):
(DashboardImage):
(DashboardImage.create):
(DashboardImage.get_image):
* Websites/webkit-perf.appspot.com/models_unittest.py:
(PersistentCacheTests.setUp):
(PersistentCacheTests.test_set_cache):
(PersistentCacheTests.test_get_cache):
(DashboardImageTests.setUp):
(DashboardImageTests):
(DashboardImageTests.test_create):
(DashboardImageTests.test_get):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@109821 268f45cc-cd09-0410-ab3c-d52691b4dbfc

ChangeLog
Websites/webkit-perf.appspot.com/controller.py
Websites/webkit-perf.appspot.com/models.py
Websites/webkit-perf.appspot.com/models_unittest.py

index 3edb632..61970e4 100644 (file)
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,40 @@
+2012-03-05  Ryosuke Niwa  <rniwa@webkit.org>
+
+        Perf-o-matic should memcache dashboard images
+        https://bugs.webkit.org/show_bug.cgi?id=80349
+
+        Reviewed by Eric Seidel.
+
+        Added DashboardImage.create and DashboardImage.get_image to encapsulate memcache.
+        Also replaced transaction in DashboardImage.set_cache by a single put since it duplicates
+        what put does by default.
+
+        Also removed redundant cache_* functions and merged them into handler code.
+
+        * Websites/webkit-perf.appspot.com/controller.py:
+        (ManifestUpdateHandler.post):
+        (CachedManifestHandler.get):
+        (DashboardUpdateHandler.post):
+        (CachedDashboardHandler.get):
+        (RunsUpdateHandler):
+        (RunsUpdateHandler.post):
+        (RunsChartHandler):
+        (RunsChartHandler.post):
+        (DashboardImageHandler.get):
+        * Websites/webkit-perf.appspot.com/models.py:
+        (PersistentCache.set_cache):
+        (DashboardImage):
+        (DashboardImage.create):
+        (DashboardImage.get_image):
+        * Websites/webkit-perf.appspot.com/models_unittest.py:
+        (PersistentCacheTests.setUp):
+        (PersistentCacheTests.test_set_cache):
+        (PersistentCacheTests.test_get_cache):
+        (DashboardImageTests.setUp):
+        (DashboardImageTests):
+        (DashboardImageTests.test_create):
+        (DashboardImageTests.test_get):
+
 2012-03-05  Sheriff Bot  <webkit.review.bot@gmail.com>
 
         Unreviewed, rolling out r109760.
index 1e03579..3436050 100644 (file)
@@ -43,10 +43,6 @@ from models import PersistentCache
 from models import model_from_numeric_id
 
 
-def cache_manifest(cache):
-    PersistentCache.set_cache('manifest', cache)
-
-
 def schedule_manifest_update():
     taskqueue.add(url='/api/test/update')
 
@@ -54,7 +50,7 @@ def schedule_manifest_update():
 class ManifestUpdateHandler(webapp2.RequestHandler):
     def post(self):
         self.response.headers['Content-Type'] = 'text/plain; charset=utf-8'
-        cache_manifest(ManifestJSONGenerator().to_json())
+        PersistentCache.set_cache('manifest', ManifestJSONGenerator().to_json())
         self.response.out.write('OK')
 
 
@@ -68,10 +64,6 @@ class CachedManifestHandler(webapp2.RequestHandler):
             schedule_manifest_update()
 
 
-def cache_dashboard(cache):
-    PersistentCache.set_cache('dashboard', cache)
-
-
 def schedule_dashboard_update():
     taskqueue.add(url='/api/test/dashboard/update')
 
@@ -79,7 +71,7 @@ def schedule_dashboard_update():
 class DashboardUpdateHandler(webapp2.RequestHandler):
     def post(self):
         self.response.headers['Content-Type'] = 'text/plain; charset=utf-8'
-        cache_dashboard(DashboardJSONGenerator().to_json())
+        PersistentCache.set_cache('dashboard', DashboardJSONGenerator().to_json())
         self.response.out.write('OK')
 
 
@@ -93,10 +85,6 @@ class CachedDashboardHandler(webapp2.RequestHandler):
             schedule_dashboard_update()
 
 
-def cache_runs(test_id, branch_id, platform_id, cache):
-    PersistentCache.set_cache(Test.cache_key(test_id, branch_id, platform_id), cache)
-
-
 def schedule_runs_update(test_id, branch_id, platform_id):
     taskqueue.add(url='/api/test/runs/update', params={'id': test_id, 'branchid': branch_id, 'platformid': platform_id})
     for display_days in [7, 30, 90, 365]:
@@ -117,9 +105,6 @@ def _get_test_branch_platform_ids(handler):
 
 
 class RunsUpdateHandler(webapp2.RequestHandler):
-    def get(self):
-        self.post()
-
     def post(self):
         self.response.headers['Content-Type'] = 'text/plain; charset=utf-8'
         test_id, branch_id, platform_id = _get_test_branch_platform_ids(self)
@@ -131,7 +116,7 @@ class RunsUpdateHandler(webapp2.RequestHandler):
         assert platform
         assert test
 
-        cache_runs(test_id, branch_id, platform_id, Runs(branch, platform, test.name).to_json())
+        PersistentCache.set_cache(Test.cache_key(test_id, branch_id, platform_id), Runs(branch, platform, test.name).to_json())
         self.response.out.write('OK')
 
 
@@ -148,9 +133,6 @@ class CachedRunsHandler(webapp2.RequestHandler):
 
 
 class RunsChartHandler(webapp2.RequestHandler):
-    def get(self):
-        self.post()
-
     def post(self):
         self.response.headers['Content-Type'] = 'text/plain; charset=utf-8'
         test_id, branch_id, platform_id = _get_test_branch_platform_ids(self)
@@ -166,8 +148,7 @@ class RunsChartHandler(webapp2.RequestHandler):
         params = Runs(branch, platform, test.name).chart_params(display_days)
         dashboard_chart_file = urllib.urlopen('http://chart.googleapis.com/chart', urllib.urlencode(params))
 
-        DashboardImage(key_name=DashboardImage.key_name(branch.id, platform.id, test.id, display_days),
-            image=dashboard_chart_file.read()).put()
+        DashboardImage.create(branch.id, platform.id, test.id, display_days, dashboard_chart_file.read())
 
 
 class DashboardImageHandler(webapp2.RequestHandler):
@@ -182,9 +163,7 @@ class DashboardImageHandler(webapp2.RequestHandler):
             self.response.out.write('Failed')
 
         self.response.headers['Content-Type'] = 'image/png'
-        image = DashboardImage.get_by_key_name(DashboardImage.key_name(branch_id, platform_id, test_id, display_days))
-        if image:
-            self.response.out.write(image.image)
+        self.response.out.write(DashboardImage.get_image(branch_id, platform_id, test_id, display_days))
 
 
 def schedule_report_process(log):
index f692419..c4433ac 100644 (file)
@@ -303,16 +303,7 @@ class PersistentCache(db.Model):
     @staticmethod
     def set_cache(name, value):
         memcache.set(name, value)
-
-        def execute():
-            cache = PersistentCache.get_by_key_name(name)
-            if cache:
-                cache.value = value
-                cache.put()
-            else:
-                PersistentCache(key_name=name, value=value).put()
-
-        db.run_in_transaction(execute)
+        PersistentCache(key_name=name, value=value).put()
 
     @staticmethod
     def get_cache(name):
@@ -330,6 +321,24 @@ class DashboardImage(db.Model):
     image = db.BlobProperty(required=True)
     createdAt = db.DateTimeProperty(required=True, auto_now=True)
 
+    @staticmethod
+    def create(branch_id, platform_id, test_id, display_days, image):
+        key_name = DashboardImage.key_name(branch_id, platform_id, test_id, display_days)
+        instance = DashboardImage(key_name=key_name, image=image)
+        instance.put()
+        memcache.set('dashboard-image:' + key_name, image)
+        return instance
+
+    @staticmethod
+    def get_image(branch_id, platform_id, test_id, display_days):
+        key_name = DashboardImage.key_name(branch_id, platform_id, test_id, display_days)
+        image = memcache.get('dashboard-image:' + key_name)
+        if not image:
+            instance = DashboardImage.get_by_key_name(key_name)
+            image = instance.image
+            memcache.set('dashboard-image:' + key_name, image)
+        return image
+
     @classmethod
     def needs_update(cls, branch_id, platform_id, test_id, display_days, now=datetime.now()):
         if display_days < 10:
index 1f4c1bf..57e0554 100644 (file)
@@ -489,16 +489,14 @@ class ReportLogTests(DataStoreTestsBase):
 
 class PersistentCacheTests(DataStoreTestsBase):
     def setUp(self):
-        self.testbed = testbed.Testbed()
-        self.testbed.activate()
-        self.testbed.init_datastore_v3_stub()
+        super(PersistentCacheTests, self).setUp()
         self.testbed.init_memcache_stub()
 
     def _assert_persistent_cache(self, name, value):
         self.assertEqual(PersistentCache.get_by_key_name(name).value, value)
         self.assertEqual(memcache.get(name), value)
 
-    def test_set(self):
+    def test_set_cache(self):
         self.assertThereIsNoInstanceOf(PersistentCache)
 
         PersistentCache.set_cache('some-cache', 'some data')
@@ -508,7 +506,7 @@ class PersistentCacheTests(DataStoreTestsBase):
 
         self._assert_persistent_cache('some-cache', 'some other data')
 
-    def test_get(self):
+    def test_get_cache(self):
         self.assertEqual(memcache.get('some-cache'), None)
         self.assertEqual(PersistentCache.get_cache('some-cache'), None)
 
@@ -523,6 +521,31 @@ class PersistentCacheTests(DataStoreTestsBase):
 
 
 class DashboardImageTests(DataStoreTestsBase):
+    def setUp(self):
+        super(DashboardImageTests, self).setUp()
+        self.testbed.init_memcache_stub()
+
+    def test_create(self):
+        self.assertEqual(memcache.get('dashboard-image:1:2:3:7'), None)
+        self.assertThereIsNoInstanceOf(DashboardImage)
+        image = DashboardImage.create(1, 2, 3, 7, 'blah')
+        self.assertOnlyInstance(image)
+        self.assertEqual(memcache.get('dashboard-image:1:2:3:7'), 'blah')
+
+    def test_get(self):
+        image = DashboardImage.create(1, 2, 3, 7, 'blah')
+        self.assertEqual(memcache.get('dashboard-image:1:2:3:7'), 'blah')
+        memcache.set('dashboard-image:1:2:3:7', 'new value')
+
+        # Check twice to make sure the first call doesn't clear memcache
+        self.assertEqual(DashboardImage.get_image(1, 2, 3, 7), 'new value')
+        self.assertEqual(DashboardImage.get_image(1, 2, 3, 7), 'new value')
+
+        memcache.delete('dashboard-image:1:2:3:7')
+        self.assertEqual(memcache.get('dashboard-image:1:2:3:7'), None)
+        self.assertEqual(DashboardImage.get_image(1, 2, 3, 7), 'blah')
+        self.assertEqual(memcache.get('dashboard-image:1:2:3:7'), 'blah')
+
     def test_needs_update(self):
         self.assertTrue(DashboardImage.needs_update(1, 2, 3, 7))
         self.assertTrue(DashboardImage.needs_update(1, 2, 3, 30))