You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@libcloud.apache.org by er...@apache.org on 2017/01/06 18:04:00 UTC
libcloud git commit: [google compute] Improve performance of list
nodes by caching volume information.
Repository: libcloud
Updated Branches:
refs/heads/trunk c06497f17 -> 03575ecfd
[google compute] Improve performance of list nodes by caching volume information.
We leverage the aggregated disk call and store the result. For the list node operation, we've added an extra parameter to use the cached data, which results to true.
Tests and fixtures updated as well.
Closes #813
Closes LIBCLOUD-826
Signed-off-by: Eric Johnson <er...@google.com>
Project: http://git-wip-us.apache.org/repos/asf/libcloud/repo
Commit: http://git-wip-us.apache.org/repos/asf/libcloud/commit/03575ecf
Tree: http://git-wip-us.apache.org/repos/asf/libcloud/tree/03575ecf
Diff: http://git-wip-us.apache.org/repos/asf/libcloud/diff/03575ecf
Branch: refs/heads/trunk
Commit: 03575ecfd9ac13a64c9469be87c96b1677de8493
Parents: c06497f
Author: Tom Melendez <su...@google.com>
Authored: Wed Jun 15 02:08:33 2016 +0000
Committer: Eric Johnson <er...@google.com>
Committed: Fri Jan 6 17:44:40 2017 +0000
----------------------------------------------------------------------
CHANGES.rst | 4 +
libcloud/compute/drivers/gce.py | 221 +++++++++++++++++--
.../compute/fixtures/gce/aggregated_disks.json | 117 +++++++++-
libcloud/test/compute/test_gce.py | 2 +-
4 files changed, 315 insertions(+), 29 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/libcloud/blob/03575ecf/CHANGES.rst
----------------------------------------------------------------------
diff --git a/CHANGES.rst b/CHANGES.rst
index 5e59ea1..7fff3fd 100644
--- a/CHANGES.rst
+++ b/CHANGES.rst
@@ -15,6 +15,10 @@ Common
Compute
~~~~~~~
+- [google compute] Improve performance of list nodes by caching volume information.
+ (GITHUB-813, LIBCLOUD-826)
+ [Tom Melendez]
+
- [azure] New method for accessing rate cards.
[GITHUB-957]
(Soren L. Hansen)
http://git-wip-us.apache.org/repos/asf/libcloud/blob/03575ecf/libcloud/compute/drivers/gce.py
----------------------------------------------------------------------
diff --git a/libcloud/compute/drivers/gce.py b/libcloud/compute/drivers/gce.py
index 581f1e5..3bfe9ee 100644
--- a/libcloud/compute/drivers/gce.py
+++ b/libcloud/compute/drivers/gce.py
@@ -72,6 +72,7 @@ class GCEConnection(GoogleBaseConnection):
GCEConnection extends :class:`google.GoogleBaseConnection` for 2 reasons:
1. modify request_path for GCE URI.
2. Implement gce_params functionality described below.
+ 3. Add request_aggregated_items method for making aggregated API calls.
If the parameter gce_params is set to a dict prior to calling request(),
the URL parameters will be updated to include those key/values FOR A
@@ -130,6 +131,80 @@ class GCEConnection(GoogleBaseConnection):
return response
+ def request_aggregated_items(self, api_name):
+ """
+ Perform request(s) to obtain all results from 'api_name'.
+
+ This method will make requests to the aggregated 'api_name' until
+ all results are received. It will then, through a helper function,
+ combine all results and return a single 'items' dictionary.
+
+ :param api_name: Name of API to call. Consult API docs
+ for valid names.
+ :type api_name: ``str``
+
+ :return: dict in the format of the API response.
+ format: { 'items': {'key': {api_name: []}} }
+ ex: { 'items': {'zones/us-central1-a': {disks: []}} }
+ :rtype: ``dict``
+ """
+ request_path = "/aggregated/%s" % api_name
+ api_responses = []
+
+ params = {'maxResults': 500}
+ more_results = True
+ while more_results:
+ self.gce_params = params
+ response = self.request(request_path, method='GET').object
+ if 'items' in response:
+ api_responses.append(response)
+ more_results = 'pageToken' in params
+ return self._merge_response_items(api_name, api_responses)
+
+ def _merge_response_items(self, list_name, response_list):
+ """
+ Take a list of API responses ("item"-portion only) and combine them.
+
+ Helper function to combine multiple aggegrated responses into a single
+ dictionary that resembles an API response.
+
+ Note: keys that don't have a 'list_name" key (including warnings)
+ are omitted.
+
+ :param list_name: Name of list in dict. Practically, this is
+ the name of the API called (e.g. 'disks').
+ :type list_name: ``str``
+
+ :param response_list: list of API responses (e.g. resp['items']).
+ Each entry in the list is the result of a
+ single API call. Expected format is:
+ [ { items: {
+ key1: { api_name:[]},
+ key2: { api_name:[]}
+ }}, ... ]
+ :type response_list: ``dict``
+
+ :return: dict in the format of:
+ { items: {key: {api_name:[]}, key2: {api_name:[]}} }
+ ex: { items: {
+ 'us-east1-a': {'disks': []},
+ 'us-east1-b': {'disks': []}
+ }}
+ :rtype: ``dict``
+ """
+ merged_items = {}
+ for resp in response_list:
+ if 'items' in resp:
+ # example k would be a zone or region name
+ # example v would be { "disks" : [], "otherkey" : "..." }
+ for k, v in resp['items'].items():
+ if list_name in v:
+ merged_items.setdefault(k, {}).setdefault(
+ list_name, [])
+ # Combine the list with the existing list.
+ merged_items[k][list_name] += v[list_name]
+ return {'items': merged_items}
+
class GCEList(object):
"""
@@ -1719,6 +1794,10 @@ class GCENodeDriver(NodeDriver):
else:
self.region = None
+ # Volume details are looked up in this name-zone dict.
+ # It is populated if the volume name is not found or the dict is empty.
+ self._ex_volume_dict = {}
+
def ex_add_access_config(self, node, name, nic, nat_ip=None,
config_type=None):
"""
@@ -2278,7 +2357,7 @@ class GCENodeDriver(NodeDriver):
for n in response.get('items', [])]
return list_networks
- def list_nodes(self, ex_zone=None):
+ def list_nodes(self, ex_zone=None, ex_use_disk_cache=True):
"""
Return a list of nodes in the current zone or all zones.
@@ -2286,6 +2365,11 @@ class GCENodeDriver(NodeDriver):
:type ex_zone: ``str`` or :class:`GCEZone` or
:class:`NodeLocation` or ``None``
+ :keyword ex_use_disk_cache: Disk information for each node will
+ retrieved from a dictionary rather
+ than making a distinct API call for it.
+ :type ex_use_disk_cache: ``bool``
+
:return: List of Node objects
:rtype: ``list`` of :class:`Node`
"""
@@ -2295,16 +2379,20 @@ class GCENodeDriver(NodeDriver):
request = '/aggregated/instances'
else:
request = '/zones/%s/instances' % (zone.name)
-
response = self.connection.request(request, method='GET').object
if 'items' in response:
# The aggregated response returns a dict for each zone
if zone is None:
+ # Create volume cache now for fast lookups of disk info.
+ self._ex_populate_volume_dict()
for v in response['items'].values():
for i in v.get('instances', []):
try:
- list_nodes.append(self._to_node(i))
+ list_nodes.append(
+ self._to_node(i,
+ use_disk_cache=ex_use_disk_cache)
+ )
# If a GCE node has been deleted between
# - is was listed by `request('.../instances', 'GET')
# - it is converted by `self._to_node(i)`
@@ -2317,7 +2405,9 @@ class GCENodeDriver(NodeDriver):
else:
for i in response['items']:
try:
- list_nodes.append(self._to_node(i))
+ list_nodes.append(
+ self._to_node(i, use_disk_cache=ex_use_disk_cache)
+ )
# If a GCE node has been deleted between
# - is was listed by `request('.../instances', 'GET')
# - it is converted by `self._to_node(i)`
@@ -2327,6 +2417,8 @@ class GCENodeDriver(NodeDriver):
# other nodes.
except ResourceNotFoundError:
pass
+ # Clear the volume cache as lookups are complete.
+ self._ex_volume_dict = {}
return list_nodes
def ex_list_regions(self):
@@ -6929,26 +7021,36 @@ class GCENodeDriver(NodeDriver):
response = self.connection.request(request, method='GET').object
return self._to_snapshot(response)
- def ex_get_volume(self, name, zone=None):
+ def ex_get_volume(self, name, zone=None, use_cache=False):
"""
Return a Volume object based on a volume name and optional zone.
- :param name: The name of the volume
- :type name: ``str``
+ To improve performance, we request all disks and allow the user
+ to consult the cache dictionary rather than making an API call.
+
+ :param name: The name of the volume
+ :type name: ``str``
:keyword zone: The zone to search for the volume in (set to 'all' to
search all zones)
:type zone: ``str`` or :class:`GCEZone` or :class:`NodeLocation`
or ``None``
+ :keyword use_cache: Search for the volume in the existing cache of
+ volumes. If True, we omit the API call and search
+ self.volumes_dict. If False, a call to
+ disks/aggregatedList is made prior to searching
+ self._ex_volume_dict.
+ :type use_cache: ``bool``
+
:return: A StorageVolume object for the volume
:rtype: :class:`StorageVolume`
"""
- zone = self._set_zone(zone) or self._find_zone_or_region(
- name, 'disks', res_name='Volume')
- request = '/zones/%s/disks/%s' % (zone.name, name)
- response = self.connection.request(request, method='GET').object
- return self._to_storage_volume(response)
+ if not self._ex_volume_dict or use_cache is False:
+ # Make the API call and build volume dictionary
+ self._ex_populate_volume_dict()
+
+ return self._ex_lookup_volume(name, zone)
def ex_get_region(self, name):
"""
@@ -7201,6 +7303,85 @@ class GCENodeDriver(NodeDriver):
'scopes': self.scopes,
'credential_file': self.credential_file}
+ def _build_volume_dict(self, zone_dict):
+ """
+ Build a dictionary in [name][zone]=disk format.
+
+ :param zone_dict: dict in the format of:
+ { items: {key: {api_name:[], key2: api_name:[]}} }
+ :type zone_dict: ``dict``
+
+ :return: dict of volumes, organized by name, then zone Format:
+ { 'disk_name':
+ {'zone_name1': disk_info, 'zone_name2': disk_info} }
+ :rtype: ``dict``
+ """
+ name_zone_dict = {}
+ for k, v in zone_dict.items():
+ zone_name = k.replace('zones/', '')
+ disks = v.get('disks', [])
+ for disk in disks:
+ n = disk['name']
+ name_zone_dict.setdefault(n, {})
+ name_zone_dict[n].update({zone_name: disk})
+ return name_zone_dict
+
+ def _ex_lookup_volume(self, volume_name, zone=None):
+ """
+ Look up volume by name and zone in volume dict.
+
+ If zone isn't specified or equals 'all', we return the volume
+ for the first zone, as determined alphabetically.
+
+ :param volume_name: The name of the volume.
+ :type volume_name: ``str``
+
+ :keyword zone: The zone to search for the volume in (set to 'all' to
+ search all zones)
+ :type zone: ``str`` or ``None``
+
+ :return: A StorageVolume object for the volume.
+ :rtype: :class:`StorageVolume` or raise ``ResourceNotFoundError``.
+ """
+ if volume_name not in self._ex_volume_dict:
+ # Possibly added through another thread/process, so re-populate
+ # _volume_dict and try again. If still not found, raise exception.
+ self._ex_populate_dict()
+ if volume_name not in self._ex_volume_dict:
+ raise ResourceNotFoundError(
+ 'Volume name: \'%s\' not found. Zone: %s' % (
+ volume_name, zone), None, None)
+ # Disk names are not unique across zones, so if zone is None or
+ # 'all', we return the first one we find for that disk name. For
+ # consistency, we sort by keys and set the zone to the first key.
+ if zone is None or zone is 'all':
+ zone = sorted(self._ex_volume_dict[volume_name])[0]
+
+ volume = self._ex_volume_dict[volume_name].get(zone, None)
+ if not volume:
+ raise ResourceNotFoundError(
+ 'Volume \'%s\' not found for zone %s.' % (volume_name,
+ zone), None, None)
+ return self._to_storage_volume(volume)
+
+ def _ex_populate_volume_dict(self):
+ """
+ Fetch the volume information using disks/aggregatedList
+ and store it in _ex_volume_dict.
+
+ return: ``None``
+ """
+ # fill the volume dict by making an aggegatedList call to disks.
+ aggregated_items = self.connection.request_aggregated_items(
+ "disks")
+
+ # _ex_volume_dict is in the format of:
+ # { 'disk_name' : { 'zone1': disk, 'zone2': disk, ... }}
+ self._ex_volume_dict = self._build_volume_dict(
+ aggregated_items['items'])
+
+ return None
+
def _catch_error(self, ignore_errors=False):
"""
Catch an exception and raise it unless asked to ignore it.
@@ -8083,15 +8264,18 @@ class GCENodeDriver(NodeDriver):
country=location['name'].split('-')[0],
driver=self)
- def _to_node(self, node):
+ def _to_node(self, node, use_disk_cache=False):
"""
Return a Node object from the JSON-response dictionary.
- :param node: The dictionary describing the node.
- :type node: ``dict``
+ :param node: The dictionary describing the node.
+ :type node: ``dict``
- :return: Node object
- :rtype: :class:`Node`
+ :keyword use_disk_cache: If true, ex_get_volume call will use cache.
+ :type use_disk_cache: ``bool``
+
+ :return: Node object
+ :rtype: :class:`Node`
"""
public_ips = []
private_ips = []
@@ -8122,7 +8306,8 @@ class GCENodeDriver(NodeDriver):
for disk in extra['disks']:
if disk.get('boot') and disk.get('type') == 'PERSISTENT':
bd = self._get_components_from_path(disk['source'])
- extra['boot_disk'] = self.ex_get_volume(bd['name'], bd['zone'])
+ extra['boot_disk'] = self.ex_get_volume(
+ bd['name'], bd['zone'], use_cache=use_disk_cache)
if 'items' in node['tags']:
tags = node['tags']['items']
http://git-wip-us.apache.org/repos/asf/libcloud/blob/03575ecf/libcloud/test/compute/fixtures/gce/aggregated_disks.json
----------------------------------------------------------------------
diff --git a/libcloud/test/compute/fixtures/gce/aggregated_disks.json b/libcloud/test/compute/fixtures/gce/aggregated_disks.json
index 9133041..e884940 100644
--- a/libcloud/test/compute/fixtures/gce/aggregated_disks.json
+++ b/libcloud/test/compute/fixtures/gce/aggregated_disks.json
@@ -67,16 +67,68 @@
}
},
"zones/us-central1-a": {
- "warning": {
- "code": "NO_RESULTS_ON_PAGE",
- "data": [
- {
- "key": "scope",
- "value": "zones/us-central1-a"
- }
- ],
- "message": "There are no results for scope 'zones/us-central1-a' on this page."
- }
+ "disks": [
+ {
+ "creationTimestamp": "2013-12-13T10:45:20.308-08:00",
+ "description": "Image: https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-7-wheezy-v20131120",
+ "id": "0265567475385851075",
+ "kind": "compute#disk",
+ "name": "node-name",
+ "description": "I'm a happy little disk",
+ "type": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-a/diskTypes/pd-standard",
+ "selfLink": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-a/disks/node-name",
+ "sizeGb": "10",
+ "sourceImage": "https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-7-wheezy-v20131120",
+ "sourceImageId": "17312518942796567788",
+ "status": "READY",
+ "zone": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-a"
+ },
+ {
+ "creationTimestamp": "2013-12-13T10:45:42.139-08:00",
+ "description": "Image: https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-7-wheezy-v20131120",
+ "id": "08045379695757218000",
+ "kind": "compute#disk",
+ "name": "lcdisk",
+ "description": "I'm a happy little disk",
+ "type": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-a/diskTypes/pd-ssd",
+ "selfLink": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-a/disks/lcdisk",
+ "sizeGb": "10",
+ "sourceImage": "https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-7-wheezy-v20131120",
+ "sourceImageId": "17312518942796567789",
+ "status": "READY",
+ "zone": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-a"
+ },
+ {
+ "creationTimestamp": "2013-12-13T10:54:07.687-08:00",
+ "description": "Image: https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-7-wheezy-v20131120",
+ "id": "08045379695757218002",
+ "kind": "compute#disk",
+ "name": "lcnode-000",
+ "description": "I'm a happy little persistent disk",
+ "type": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-a/diskTypes/pd-standard",
+ "selfLink": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-a/disks/lcnode-000",
+ "sizeGb": "10",
+ "sourceImage": "https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-7-wheezy-v20131120",
+ "sourceImageId": "17312518942796567789",
+ "status": "READY",
+ "zone": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-a"
+ },
+ {
+ "creationTimestamp": "2013-12-13T10:54:07.687-08:00",
+ "description": "Image: https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-7-wheezy-v20131120",
+ "id": "08045379695757218000",
+ "kind": "compute#disk",
+ "name": "lcnode-001",
+ "description": "I'm a happy little persistent disk",
+ "type": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-a/diskTypes/pd-standard",
+ "selfLink": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-a/disks/lcnode-001",
+ "sizeGb": "10",
+ "sourceImage": "https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-7-wheezy-v20131120",
+ "sourceImageId": "17312518942796567791",
+ "status": "READY",
+ "zone": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-a"
+ }
+ ]
},
"zones/us-central1-b": {
"disks": [
@@ -90,6 +142,36 @@
"sizeGb": "10",
"status": "READY",
"zone": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-b"
+ },
+ {
+ "creationTimestamp": "2013-12-13T10:54:07.687-08:00",
+ "description": "Image: https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-7-wheezy-v20131120",
+ "id": "08045379695757218001",
+ "kind": "compute#disk",
+ "name": "libcloud-lb-demo-www-000",
+ "description": "I'm a happy little persistent disk",
+ "type": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-b/diskTypes/pd-standard",
+ "selfLink": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-b/disks/libcloud-lb-demo-www-000",
+ "sizeGb": "10",
+ "sourceImage": "https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-7-wheezy-v20131120",
+ "sourceImageId": "17312518942796567791",
+ "status": "READY",
+ "zone": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-b"
+ },
+ {
+ "creationTimestamp": "2013-12-13T10:54:07.687-08:00",
+ "description": "Image: https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-7-wheezy-v20131120",
+ "id": "08045379695757218001",
+ "kind": "compute#disk",
+ "name": "libcloud-lb-demo-www-001",
+ "description": "I'm a happy little persistent disk",
+ "type": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-b/diskTypes/pd-standard",
+ "selfLink": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-b/disks/libcloud-lb-demo-www-001",
+ "sizeGb": "10",
+ "sourceImage": "https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-7-wheezy-v20131120",
+ "sourceImageId": "17312518942796567791",
+ "status": "READY",
+ "zone": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central1-b"
}
]
},
@@ -156,6 +238,21 @@
"sourceImageId": "17312518942796567788",
"status": "READY",
"zone": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central2-a"
+ },
+ {
+ "creationTimestamp": "2013-12-13T10:45:42.139-08:00",
+ "description": "Image: https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-7-wheezy-v20131120",
+ "id": "08045379695757218000",
+ "kind": "compute#disk",
+ "name": "lcdisk",
+ "description": "I'm a happy little disk",
+ "type": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central2-a/diskTypes/pd-ssd",
+ "selfLink": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central2-a/disks/lcdisk",
+ "sizeGb": "10",
+ "sourceImage": "https://www.googleapis.com/compute/v1/projects/debian-cloud/global/images/debian-7-wheezy-v20131120",
+ "sourceImageId": "17312518942796567789",
+ "status": "READY",
+ "zone": "https://www.googleapis.com/compute/v1/projects/project_name/zones/us-central2-a"
}
]
}
http://git-wip-us.apache.org/repos/asf/libcloud/blob/03575ecf/libcloud/test/compute/test_gce.py
----------------------------------------------------------------------
diff --git a/libcloud/test/compute/test_gce.py b/libcloud/test/compute/test_gce.py
index bccd327..8dc361f 100644
--- a/libcloud/test/compute/test_gce.py
+++ b/libcloud/test/compute/test_gce.py
@@ -657,7 +657,7 @@ class GCENodeDriverTest(GoogleTestCase, TestCaseMixin):
volumes_all = self.driver.list_volumes('all')
volumes_uc1a = self.driver.list_volumes('us-central1-a')
self.assertEqual(len(volumes), 2)
- self.assertEqual(len(volumes_all), 10)
+ self.assertEqual(len(volumes_all), 17)
self.assertEqual(len(volumes_uc1a), 2)
self.assertEqual(volumes[0].name, 'lcdisk')
self.assertEqual(volumes_uc1a[0].name, 'lcdisk')