[config] Cache result of gitiles.Location.parse_resolve in memcache.
It is almost certainly static, yet we make gitiles RPCs all the time to get it.
This should reduce average QPS to gitiles by at least 1.5x (hard to tell for
sure without doing some math to see what portion of the cache will be stale on
average during each cron tick).
R=nodir@chromium.org
Change-Id: Iaac70d328e18cbd0ca29c1ab50a5cee7e3e4c48d
Reviewed-on: https://chromium-review.googlesource.com/c/1428160
Commit-Queue: Vadim Shtayura <vadimsh@chromium.org>
Reviewed-by: Nodir Turakulov <nodir@chromium.org>
diff --git a/appengine/components/components/gitiles.py b/appengine/components/components/gitiles.py
index 8cf7768..c36fbad 100644
--- a/appengine/components/components/gitiles.py
+++ b/appengine/components/components/gitiles.py
@@ -64,6 +64,36 @@
def join(self, *parts):
return self._replace(path=posixpath.join(self.path, *parts))
+ def to_dict(self):
+ """Serializes this Location to a jsonish dict."""
+ _validate_args(
+ self.hostname,
+ self.project,
+ self.treeish,
+ self.path,
+ path_required=True)
+ return {
+ 'hostname': self.hostname,
+ 'project': self.project,
+ 'treeish': self.treeish,
+ 'path': self.path,
+ }
+
+ @classmethod
+ def from_dict(cls, d):
+ """Restores Location from a dict produced by to_dict."""
+ hostname = d.get('hostname')
+ project = d.get('project')
+ treeish = d.get('treeish')
+ path = d.get('path')
+ _validate_args(
+ hostname,
+ project,
+ treeish,
+ path,
+ path_required=True)
+ return cls(hostname, project, treeish, path)
+
@classmethod
def parse(cls, url, treeishes=None):
"""Parses a Gitiles-formatted url.
diff --git a/appengine/components/components/gitiles_test.py b/appengine/components/components/gitiles_test.py
index 4eef1e2..ec6d4f2 100755
--- a/appengine/components/components/gitiles_test.py
+++ b/appengine/components/components/gitiles_test.py
@@ -281,6 +281,11 @@
req_path,
headers={'Accept': 'text/plain'})
+ def test_to_from_dict(self):
+ loc = gitiles.Location.parse(
+ 'http://localhost/project/+/treeish/path/to/something')
+ self.assertEqual(loc, gitiles.Location.from_dict(loc.to_dict()))
+
def test_parse_location(self):
url = 'http://localhost/project/+/treeish/path/to/something'
loc = gitiles.Location.parse(url)
diff --git a/appengine/config_service/gitiles_import.py b/appengine/config_service/gitiles_import.py
index ad893bb..fa0332b 100644
--- a/appengine/config_service/gitiles_import.py
+++ b/appengine/config_service/gitiles_import.py
@@ -21,8 +21,9 @@
import StringIO
import tarfile
-from google.appengine.api import urlfetch_errors
+from google.appengine.api import memcache
from google.appengine.api import taskqueue
+from google.appengine.api import urlfetch_errors
from google.appengine.ext import ndb
from google.protobuf import text_format
@@ -91,6 +92,22 @@
## Low level import functions
+def _resolved_location(url):
+ """Gitiles URL string -> gitiles.Location.parse_resolve(url).
+
+ Does caching internally for X sec (X in [30m, 1h30m]) to avoid hitting Gitiles
+ all the time for data that is almost certainly static.
+ """
+ cache_key = 'gitiles_location:v1:' + url
+ as_dict = memcache.get(cache_key)
+ if as_dict is not None:
+ return gitiles.Location.from_dict(as_dict)
+ logging.debug('Cache miss when resolving gitiles location %s', url)
+ loc = gitiles.Location.parse_resolve(url)
+ memcache.set(cache_key, loc.to_dict(), time=random.randint(1800, 5400))
+ return loc
+
+
def _import_revision(config_set, base_location, commit, force_update):
"""Imports a referenced Gitiles revision into a config set.
@@ -301,7 +318,7 @@
raise Error('services are not stored on Gitiles')
if not conf.services_config_location:
raise Error('services config location is not set')
- location_root = gitiles.Location.parse_resolve(conf.services_config_location)
+ location_root = _resolved_location(conf.services_config_location)
service_location = location_root._replace(
path=os.path.join(location_root.path, service_id))
_import_config_set('services/%s' % service_id, service_location)
@@ -320,7 +337,7 @@
raise Error('project %s is not a Gitiles project' % project_id)
try:
- loc = gitiles.Location.parse_resolve(project.config_location.url)
+ loc = _resolved_location(project.config_location.url)
except gitiles.TreeishResolutionError:
@ndb.transactional
@@ -355,7 +372,7 @@
if project.config_location.storage_type != GITILES_LOCATION_TYPE:
raise Error('project %s is not a Gitiles project' % project_id)
- # We don't call parse_resolve here because we are replacing treeish and
+ # We don't call _resolved_location here because we are replacing treeish and
# path below anyway.
loc = gitiles.Location.parse(project.config_location.url)
@@ -444,7 +461,7 @@
config_sets = []
if (conf and conf.services_config_storage_type == GITILES_STORAGE_TYPE and
conf.services_config_location):
- loc = gitiles.Location.parse_resolve(conf.services_config_location)
+ loc = _resolved_location(conf.services_config_location)
config_sets += _service_config_sets(loc)
config_sets += _project_and_ref_config_sets()
