Add findall server side filtering
Findall method in cinderclient/base.py isn't very efficient: it loads the entire list then filters on the Python side. When calling "cinder show 'volName'" on a tenant holding a lot of volumes (> 1000) the run time is too long. On my env show command with 10000 volumes takes ~2 min, after apply this patch few seconds. Closes-Bug: #1449444 Change-Id: I23070d94d5bb100b2dd8677f67d7c8b1e7d34e52
This commit is contained in:
parent
f098b02564
commit
0b8b9f2de7
|
@ -210,8 +210,8 @@ class ManagerWithFind(six.with_metaclass(abc.ABCMeta, Manager)):
|
||||||
"""
|
"""
|
||||||
Find a single item with attributes matching ``**kwargs``.
|
Find a single item with attributes matching ``**kwargs``.
|
||||||
|
|
||||||
This isn't very efficient: it loads the entire list then filters on
|
This isn't very efficient for search options which require the
|
||||||
the Python side.
|
Python side filtering(e.g. 'human_id')
|
||||||
"""
|
"""
|
||||||
matches = self.findall(**kwargs)
|
matches = self.findall(**kwargs)
|
||||||
num_matches = len(matches)
|
num_matches = len(matches)
|
||||||
|
@ -227,16 +227,29 @@ class ManagerWithFind(six.with_metaclass(abc.ABCMeta, Manager)):
|
||||||
"""
|
"""
|
||||||
Find all items with attributes matching ``**kwargs``.
|
Find all items with attributes matching ``**kwargs``.
|
||||||
|
|
||||||
This isn't very efficient: it loads the entire list then filters on
|
This isn't very efficient for search options which require the
|
||||||
the Python side.
|
Python side filtering(e.g. 'human_id')
|
||||||
"""
|
"""
|
||||||
found = []
|
|
||||||
searches = list(kwargs.items())
|
|
||||||
|
|
||||||
# Want to search for all tenants here so that when attempting to delete
|
# Want to search for all tenants here so that when attempting to delete
|
||||||
# that a user like admin doesn't get a failure when trying to delete
|
# that a user like admin doesn't get a failure when trying to delete
|
||||||
# another tenant's volume by name.
|
# another tenant's volume by name.
|
||||||
for obj in self.list(search_opts={'all_tenants': 1}):
|
search_opts = {'all_tenants': 1}
|
||||||
|
|
||||||
|
# Pass 'name' or 'display_name' search_opts to server filtering to
|
||||||
|
# increase search performance.
|
||||||
|
if 'name' in kwargs:
|
||||||
|
search_opts['name'] = kwargs['name']
|
||||||
|
elif 'display_name' in kwargs:
|
||||||
|
search_opts['display_name'] = kwargs['display_name']
|
||||||
|
|
||||||
|
found = []
|
||||||
|
searches = kwargs.items()
|
||||||
|
|
||||||
|
# Not all resources attributes support filters on server side
|
||||||
|
# (e.g. 'human_id' doesn't), so when doing findall some client
|
||||||
|
# side filtering is still needed.
|
||||||
|
for obj in self.list(search_opts=search_opts):
|
||||||
try:
|
try:
|
||||||
if all(getattr(obj, attr) == value
|
if all(getattr(obj, attr) == value
|
||||||
for (attr, value) in searches):
|
for (attr, value) in searches):
|
||||||
|
|
|
@ -234,7 +234,8 @@ class ShellTest(utils.TestCase):
|
||||||
|
|
||||||
def test_delete_by_name(self):
|
def test_delete_by_name(self):
|
||||||
self.run_command('delete sample-volume')
|
self.run_command('delete sample-volume')
|
||||||
self.assert_called_anytime('GET', '/volumes/detail?all_tenants=1')
|
self.assert_called_anytime('GET', '/volumes/detail?all_tenants=1&'
|
||||||
|
'display_name=sample-volume')
|
||||||
self.assert_called('DELETE', '/volumes/1234')
|
self.assert_called('DELETE', '/volumes/1234')
|
||||||
|
|
||||||
def test_delete_multiple(self):
|
def test_delete_multiple(self):
|
||||||
|
|
|
@ -344,7 +344,8 @@ class ShellTest(utils.TestCase):
|
||||||
|
|
||||||
def test_delete_by_name(self):
|
def test_delete_by_name(self):
|
||||||
self.run_command('delete sample-volume')
|
self.run_command('delete sample-volume')
|
||||||
self.assert_called_anytime('GET', '/volumes/detail?all_tenants=1')
|
self.assert_called_anytime('GET', '/volumes/detail?all_tenants=1&'
|
||||||
|
'name=sample-volume')
|
||||||
self.assert_called('DELETE', '/volumes/1234')
|
self.assert_called('DELETE', '/volumes/1234')
|
||||||
|
|
||||||
def test_delete_multiple(self):
|
def test_delete_multiple(self):
|
||||||
|
|
Loading…
Reference in New Issue