From 1bcf5f5431d3c9620596f5329d7654872235c7ee Mon Sep 17 00:00:00 2001 From: Jesse Andrews Date: Wed, 7 Mar 2012 13:05:28 -0800 Subject: [PATCH] improve speed of metadata * don't load every possible answer, only do what is needed * cache instance data for a given address for a 15 seconds using either memcache or fake memcache (in-memory). This means only a single queue/db lookup for multiple calls to metadata service * add cache expirey to fake memcache (don't grow forever) and move it to nova.common.memorycache Addresses Bug #851159 Change-Id: Icf794156e055b18915b8b5be9ba2ab97d2338bbe --- nova/api/ec2/__init__.py | 2 +- nova/api/metadata/handler.py | 212 +++++++++++++----- nova/auth/ldapdriver.py | 2 +- nova/auth/manager.py | 2 +- .../memcache.py => common/memorycache.py} | 15 +- nova/testing/fake/__init__.py | 1 - nova/tests/test_metadata.py | 8 +- 7 files changed, 169 insertions(+), 73 deletions(-) rename nova/{testing/fake/memcache.py => common/memorycache.py} (84%) diff --git a/nova/api/ec2/__init__.py b/nova/api/ec2/__init__.py index 89ac275421b6..cfa7e91bed06 100644 --- a/nova/api/ec2/__init__.py +++ b/nova/api/ec2/__init__.py @@ -156,7 +156,7 @@ class Lockout(wsgi.Middleware): if FLAGS.memcached_servers: import memcache else: - from nova.testing.fake import memcache + from nova.common import memorycache as memcache self.mc = memcache.Client(FLAGS.memcached_servers, debug=0) super(Lockout, self).__init__(application) diff --git a/nova/api/metadata/handler.py b/nova/api/metadata/handler.py index 15bf9283edc3..acadab28f423 100644 --- a/nova/api/metadata/handler.py +++ b/nova/api/metadata/handler.py @@ -19,6 +19,7 @@ """Metadata request handler.""" import base64 +import collections import webob.dec import webob.exc @@ -32,6 +33,8 @@ from nova import exception from nova import flags from nova import log as logging from nova import network +from nova.rpc import common as rpc_common +from nova import utils from nova import volume from nova import wsgi @@ -41,6 +44,11 @@ FLAGS = flags.FLAGS flags.DECLARE('use_forwarded_for', 'nova.api.auth') flags.DECLARE('dhcp_domain', 'nova.network.manager') +if FLAGS.memcached_servers: + import memcache +else: + from nova.common import memorycache as memcache + _DEFAULT_MAPPINGS = {'ami': 'sda1', 'ephemeral0': 'sda2', 'root': block_device.DEFAULT_ROOT_DEV_NAME, @@ -76,7 +84,35 @@ class MetadataRequestHandler(wsgi.Application): network_api=self.network_api, volume_api=volume.API()) - def _format_instance_mapping(self, ctxt, instance_ref): + self.metadata_mapper = { + 'user-data': self.user_data, + 'meta-data': { + 'instance-id': self.instance_id, + 'instance-type': self.instance_type, + 'ami-id': self.ami_id, + 'kernel-id': self.kernel_id, + 'ramdisk-id': self.ramdisk_id, + 'block-device-mapping': self.block_device_mapping, + 'hostname': self.hostname, + 'public-hostname': self.hostname, + 'local-hostname': self.hostname, + 'local-ipv4': self.local_ipv4, + 'public-ipv4': self.public_ipv4, + 'security-groups': self.security_groups, + 'public-keys': self.public_keys, + 'ami-launch-index': self.ami_launch_index, + 'reservation-id': self.reservation_id, + 'placement': self.placement, + 'instance-action': self.instance_action, + 'ami-manifest-path': self.ami_manifest_path, + } + } + + self._cache = memcache.Client(FLAGS.memcached_servers, debug=0) + + def _format_instance_mapping(self, instance_ref): + ctxt = context.get_admin_context() + root_device_name = instance_ref['root_device_name'] if root_device_name is None: return _DEFAULT_MAPPINGS @@ -122,72 +158,123 @@ class MetadataRequestHandler(wsgi.Application): return mappings - def get_metadata(self, address): + def get_instance(self, address): + """get instance_ref for a given fixed_ip, raising + exception.NotFound if unable to find instance + + this will attempt to use memcache or fake memcache (an in-memory + cache) to remove the DB query + RPC query for batched calls (eg + cloud-init making dozens of queries on boot) + """ + + cache_key = 'metadata-%s' % address + instance_dict = self._cache.get(cache_key) + if instance_dict: + return instance_dict + if not address: raise exception.FixedIpNotFoundForAddress(address=address) ctxt = context.get_admin_context() + try: fixed_ip = self.network_api.get_fixed_ip_by_address(ctxt, address) - instance_ref = db.instance_get(ctxt, fixed_ip['instance_id']) - except exception.NotFound: - return None + except rpc_common.RemoteError: + raise exception.FixedIpNotFoundForAddress(address=address) + instance_ref = db.instance_get(ctxt, fixed_ip['instance_id']) + instance_dict = utils.to_primitive(instance_ref) - hostname = "%s.%s" % (instance_ref['hostname'], FLAGS.dhcp_domain) - host = instance_ref['host'] - services = db.service_get_all_by_host(ctxt.elevated(), host) - availability_zone = ec2utils.get_availability_zone_by_host(services, - host) + self._cache.set(cache_key, instance_dict, 15) + return instance_dict + + def user_data(self, address): + instance_ref = self.get_instance(address) + return base64.b64decode(instance_ref['user_data']) + + def instance_id(self, address): + instance_ref = self.get_instance(address) + return ec2utils.id_to_ec2_id(instance_ref['id']) + + def instance_type(self, address): + instance_ref = self.get_instance(address) + return instance_ref['instance_type']['name'] + + def ami_id(self, address): + instance_ref = self.get_instance(address) + return ec2utils.image_ec2_id(instance_ref['image_ref']) + + def kernel_id(self, address): + instance_ref = self.get_instance(address) + kernel_id = instance_ref.get('kernel_id') + if kernel_id: + return ec2utils.image_ec2_id(kernel_id, + ec2utils.image_type('kernel')) + + def ramdisk_id(self, address): + instance_ref = self.get_instance(address) + ramdisk_id = instance_ref.get('ramdisk_id') + if ramdisk_id: + return ec2utils.image_ec2_id(ramdisk_id, + ec2utils.image_type('ramdisk')) + + def ami_launch_index(self, address): + instance_ref = self.get_instance(address) + return instance_ref['launch_index'] + + def block_device_mapping(self, address): + instance_ref = self.get_instance(address) + return self._format_instance_mapping(instance_ref) + + def hostname(self, address): + instance_ref = self.get_instance(address) + return "%s.%s" % (instance_ref['hostname'], FLAGS.dhcp_domain) + + def local_ipv4(self, address): + return address + + def public_ipv4(self, address): + instance_ref = self.get_instance(address) + ctxt = context.get_admin_context() ip_info = ec2utils.get_ip_info_for_instance(ctxt, instance_ref) floating_ips = ip_info['floating_ips'] floating_ip = floating_ips and floating_ips[0] or '' + return floating_ip - ec2_id = ec2utils.id_to_ec2_id(instance_ref['id']) - image_ec2_id = ec2utils.image_ec2_id(instance_ref['image_ref']) - security_groups = db.security_group_get_by_instance(ctxt, - instance_ref['id']) - security_groups = [x['name'] for x in security_groups] - mappings = self._format_instance_mapping(ctxt, instance_ref) - data = { - 'user-data': base64.b64decode(instance_ref['user_data']), - 'meta-data': { - 'ami-id': image_ec2_id, - 'ami-launch-index': instance_ref['launch_index'], - 'ami-manifest-path': 'FIXME', - 'block-device-mapping': mappings, - 'hostname': hostname, - 'instance-action': 'none', - 'instance-id': ec2_id, - 'instance-type': instance_ref['instance_type']['name'], - 'local-hostname': hostname, - 'local-ipv4': address, - 'placement': {'availability-zone': availability_zone}, - 'public-hostname': hostname, - 'public-ipv4': floating_ip, - 'reservation-id': instance_ref['reservation_id'], - 'security-groups': security_groups}} + def reservation_id(self, address): + instance_ref = self.get_instance(address) + return instance_ref['reservation_id'] + def placement(self, address): + instance_ref = self.get_instance(address) + host = instance_ref['host'] + ctxt = context.get_admin_context() + # note(ja): original code had ctx.elevated? + services = db.service_get_all_by_host(ctxt, host) + zone = ec2utils.get_availability_zone_by_host(services, host) + return {'availability-zone': zone} + + def security_groups(self, address): + instance_ref = self.get_instance(address) + ctxt = context.get_admin_context() + groups = db.security_group_get_by_instance(ctxt, + instance_ref['id']) + return [g['name'] for g in groups] + + def public_keys(self, address): + instance_ref = self.get_instance(address) # public-keys should be in meta-data only if user specified one if instance_ref['key_name']: - data['meta-data']['public-keys'] = { - '0': {'_name': instance_ref['key_name'], - 'openssh-key': instance_ref['key_data']}} + return {'0': {'_name': instance_ref['key_name'], + 'openssh-key': instance_ref['key_data']}} - for image_type in ['kernel', 'ramdisk']: - if instance_ref.get('%s_id' % image_type): - ec2_id = ec2utils.image_ec2_id( - instance_ref['%s_id' % image_type], - ec2utils.image_type(image_type)) - data['meta-data']['%s-id' % image_type] = ec2_id + def ami_manifest_path(self, address): + return 'Not Implemented' - if False: # TODO(vish): store ancestor ids - data['ancestor-ami-ids'] = [] - if False: # TODO(vish): store product codes - data['product-codes'] = [] - return data + def instance_action(self, address): + return 'none' - def print_data(self, data): + def format_data(self, data): if isinstance(data, dict): output = '' for key in data: @@ -207,15 +294,21 @@ class MetadataRequestHandler(wsgi.Application): else: return str(data) - def lookup(self, path, data): + def lookup(self, path, address): items = path.split('/') + data = self.metadata_mapper for item in items: if item: if not isinstance(data, dict): + # FIXME(ja): should we check that we are at the end + # of the path as well before we just return? return data if not item in data: return None data = data[item] + if isinstance(data, collections.Callable): + # lazy evaluation + data = data(address) return data @webob.dec.wsgify(RequestClass=wsgi.Request) @@ -223,19 +316,20 @@ class MetadataRequestHandler(wsgi.Application): remote_address = req.remote_addr if FLAGS.use_forwarded_for: remote_address = req.headers.get('X-Forwarded-For', remote_address) + try: - meta_data = self.get_metadata(remote_address) - except Exception: + data = self.lookup(req.path_info, + remote_address) + except (exception.NotFound, exception.FixedIpNotFoundForAddress): + LOG.error(_('Failed to get metadata for ip: %s'), remote_address) + return webob.exc.HTTPNotFound() + except: LOG.exception(_('Failed to get metadata for ip: %s'), remote_address) msg = _('An unknown error has occurred. ' 'Please try your request again.') - exc = webob.exc.HTTPInternalServerError(explanation=unicode(msg)) - return exc - if meta_data is None: - LOG.error(_('Failed to get metadata for ip: %s'), remote_address) - raise webob.exc.HTTPNotFound() - data = self.lookup(req.path_info, meta_data) + return webob.exc.HTTPInternalServerError(explanation=unicode(msg)) + if data is None: raise webob.exc.HTTPNotFound() - return self.print_data(data) + return self.format_data(data) diff --git a/nova/auth/ldapdriver.py b/nova/auth/ldapdriver.py index 4c9431b8e5be..91135f1c7cdb 100644 --- a/nova/auth/ldapdriver.py +++ b/nova/auth/ldapdriver.py @@ -96,7 +96,7 @@ LOG = logging.getLogger(__name__) if FLAGS.memcached_servers: import memcache else: - from nova.testing.fake import memcache + from nova.common import memorycache as memcache # TODO(vish): make an abstract base class with the same public methods diff --git a/nova/auth/manager.py b/nova/auth/manager.py index ca2a3add5022..f03e453b84a5 100644 --- a/nova/auth/manager.py +++ b/nova/auth/manager.py @@ -97,7 +97,7 @@ LOG = logging.getLogger(__name__) if FLAGS.memcached_servers: import memcache else: - from nova.testing.fake import memcache + from nova.common import memorycache as memcache class AuthBase(object): diff --git a/nova/testing/fake/memcache.py b/nova/common/memorycache.py similarity index 84% rename from nova/testing/fake/memcache.py rename to nova/common/memorycache.py index e4f238aa9f3a..f6a61a1c3814 100644 --- a/nova/testing/fake/memcache.py +++ b/nova/common/memorycache.py @@ -29,11 +29,16 @@ class Client(object): self.cache = {} def get(self, key): - """Retrieves the value for a key or None.""" - (timeout, value) = self.cache.get(key, (0, None)) - if timeout == 0 or utils.utcnow_ts() < timeout: - return value - return None + """Retrieves the value for a key or None. + + this expunges expired keys during each get""" + + for k in self.cache.keys(): + (timeout, _value) = self.cache[k] + if timeout and utils.utcnow_ts() >= timeout: + del self.cache[k] + + return self.cache.get(key, (0, None))[1] def set(self, key, value, time=0, min_compress_len=0): """Sets the value for a key.""" diff --git a/nova/testing/fake/__init__.py b/nova/testing/fake/__init__.py index d9eebbd23a15..5cdad4717e53 100644 --- a/nova/testing/fake/__init__.py +++ b/nova/testing/fake/__init__.py @@ -1,2 +1 @@ -import memcache import rabbit diff --git a/nova/tests/test_metadata.py b/nova/tests/test_metadata.py index 7bd3ebcdd372..a8fe448bbcf2 100644 --- a/nova/tests/test_metadata.py +++ b/nova/tests/test_metadata.py @@ -119,7 +119,7 @@ class MetadataTestCase(test.TestCase): request = webob.Request.blank('/user-data') request.remote_addr = None response = request.get_response(self.app) - self.assertEqual(response.status_int, 500) + self.assertEqual(response.status_int, 404) def test_user_data_invalid_url(self): request = webob.Request.blank('/user-data-invalid') @@ -177,9 +177,7 @@ class MetadataTestCase(test.TestCase): 'swap': '/dev/sdc', 'ebs0': '/dev/sdh'} - self.assertEqual(self.app._format_instance_mapping(ctxt, - instance_ref0), + self.assertEqual(self.app._format_instance_mapping(instance_ref0), handler._DEFAULT_MAPPINGS) - self.assertEqual(self.app._format_instance_mapping(ctxt, - instance_ref1), + self.assertEqual(self.app._format_instance_mapping(instance_ref1), expected)