Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[LIBCLOUD-826] [GCE]: Improve performance of list nodes by caching volume information #813

wants to merge 1 commit into from


Copy link

@supertom supertom commented Jun 15, 2016

[GCE] Improve performance of list nodes by caching volume information


When listing nodes, the GCE driver currently calls the disk API for each disk attached to the node. This PR changes that behavior by using the aggregatedLIst call for disks (once per list_node request) and using that information to provide disk details.

See sample performance info in LIBCLOUD-826


For disk information, aggregated calls are now always used and the disk information is stored in a dictionary, called 'volume_dict'. If the user would like the most current information, they may set the use_cache keyword to false and the call (and subsequent population of volume_dict) will be made prior to returning disk information.

Code was added/changed in two classes. In GCENodeDriver, added two methods and an additional parameter to build, lookup and toggle the refresh of the volume cache. In GCEConnection, added convenience and helper methods to the class, which not only support this performance improvement but also support the longer term vision of leveraging aggregatedList calls elsewhere.

  • (new method) def request_aggregated_items(self, api_name) - make all necessary calls, handling maxresults and saving the 'items' portion of the response.
  • (new method) def _merge_response_items(self, list_name, response_list) - helper method to merge responses into a single dictionary
  • (new member) volume_dict - dictionary organized by name, zone. Name is always available to us, but is not unique across zones. Zones, though, are optionally supplied. By organizing by name, we remove the need to search through the entire list of disks each time and can do a single hash lookup to have access to all disks by that name. If we have the zone, another hash lookup, if not, we take the first key alphabetically.
  • (new method) _build_name_zone_dict(self, zone_dict) - internal method to populate volume dict
  • (new method) _ex_populate_volume_dict(self) - Void method to call API and build volume dictionary
  • (new method) _ex_lookup_volume(self, name, zone=None) - implements the actual lookup. If zone is not provided, take the disk with that name from the (alphabetical) first zone (this is only an issue if there are more than two disks with the same name).
  • (new parameter) list_nodes(self, ex_zone=None, use_disk_cache=True) - use_disk_cache parameter for list_nodes to pass through, defaults to True. If set to true, no more than one call per 500 disks would be made to populate all disk info for nodes
  • (new parameter, revision) ex_get_volume(self, name, zone=None, use_cache=False) - revised to check if volume_dict has been populated or should be used, followed by returning the call to _ex_lookup_volume


  • done, ready for review

Checklist (tick everything that applies)

  • Code linting (required, can be done after the PR checks)
  • Documentation
  • Tests
  • ICLA (required for bigger changes)

/cc @erjohnso /cc @tonybaloney

@supertom supertom changed the title [GCE] LIBCLOUD-826: Improve performance of list nodes by caching volume information [LIBCLOUD-826] [GCE]: Improve performance of list nodes by caching volume information Jun 16, 2016
:rtype: :class:`StorageVolume` or raise ``ResourceNotFoundError``.
if volume_name not in self.volume_dict:
raise ResourceNotFoundError(

This comment has been minimized.


erjohnso Jun 23, 2016

Should this instead make an API call in case the cache is missing a recently created disk?

@@ -38,6 +38,7 @@
from libcloud.utils.iso8601 import parse_date


This comment has been minimized.


erjohnso Jun 23, 2016

iirc, this is already the default, so perhaps this is redundant.

This comment has been minimized.


supertom Jun 27, 2016
Author Contributor

Yes, definitely the default is 500 for all the APIs. I have conflicting views about being explicit, but I'll take it out for simplicity.

if volume_name not in self._volume_dict:
# Possibly added through another thread/process, so re-populate
# _volume_dict and try again. If still not found, raise exception.

This comment has been minimized.


sayap Sep 8, 2016

Should this be _ex_populate_volume_dict?

@@ -131,6 +132,80 @@ def request(self, *args, **kwargs):

return response

def request_aggregated_items(self, api_name):

This comment has been minimized.


sayap Sep 21, 2016

I think this always returns only up to 500 items, as the if self.gce_params: condition few lines above will always evaluate to false.

This comment has been minimized.


supertom Jan 6, 2017
Author Contributor

Thanks @sayap, fixed.

@supertom supertom force-pushed the supertom:LIBCLOUD-826 branch from 35c5381 to ce7b9f5 Dec 16, 2016
@supertom supertom force-pushed the supertom:LIBCLOUD-826 branch from ce7b9f5 to d98624d Jan 6, 2017
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.
@supertom supertom force-pushed the supertom:LIBCLOUD-826 branch from d98624d to d5efc32 Jan 6, 2017
@asfgit asfgit closed this in 03575ec Jan 6, 2017
asfgit pushed a commit that referenced this pull request Jan 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.