Filter snapshots data on the DB side
It filters snapshots data on the DB side to improve performance. Some tests are removed from test_snapshots.py because their function implemented on the DB side. Partial-Implements: blueprint data-filtering-on-the-db-side Change-Id: I1c3e47a4f3780de54d7a5920f463e26cc8b7bcc2
This commit is contained in:
Yuriy Nesenenko
parent
5f6cbecc5e
commit
f5e77c982d
@ -285,14 +285,14 @@ def snapshot_get(context, snapshot_id):
|
||||
return IMPL.snapshot_get(context, snapshot_id)
|
||||
|
||||
|
||||
def snapshot_get_all(context):
|
||||
def snapshot_get_all(context, filters=None):
|
||||
"""Get all snapshots."""
|
||||
return IMPL.snapshot_get_all(context)
|
||||
return IMPL.snapshot_get_all(context, filters)
|
||||
|
||||
|
||||
def snapshot_get_all_by_project(context, project_id):
|
||||
def snapshot_get_all_by_project(context, project_id, filters=None):
|
||||
"""Get all snapshots belonging to a project."""
|
||||
return IMPL.snapshot_get_all_by_project(context, project_id)
|
||||
return IMPL.snapshot_get_all_by_project(context, project_id, filters)
|
||||
|
||||
|
||||
def snapshot_get_by_host(context, host, filters=None):
|
||||
|
||||
@ -1974,10 +1974,22 @@ def snapshot_get(context, snapshot_id):
|
||||
|
||||
|
||||
@require_admin_context
|
||||
def snapshot_get_all(context):
|
||||
return model_query(context, models.Snapshot).\
|
||||
options(joinedload('snapshot_metadata')).\
|
||||
all()
|
||||
def snapshot_get_all(context, filters=None):
|
||||
# Ensure that the filter value exists on the model
|
||||
if filters:
|
||||
for key in filters.keys():
|
||||
try:
|
||||
getattr(models.Snapshot, key)
|
||||
except AttributeError:
|
||||
LOG.debug("'%s' filter key is not valid.", key)
|
||||
return []
|
||||
|
||||
query = model_query(context, models.Snapshot)
|
||||
|
||||
if filters:
|
||||
query = query.filter_by(**filters)
|
||||
|
||||
return query.options(joinedload('snapshot_metadata')).all()
|
||||
|
||||
|
||||
@require_context
|
||||
@ -2012,12 +2024,15 @@ def snapshot_get_all_for_cgsnapshot(context, cgsnapshot_id):
|
||||
|
||||
|
||||
@require_context
|
||||
def snapshot_get_all_by_project(context, project_id):
|
||||
def snapshot_get_all_by_project(context, project_id, filters=None):
|
||||
authorize_project_context(context, project_id)
|
||||
return model_query(context, models.Snapshot).\
|
||||
filter_by(project_id=project_id).\
|
||||
options(joinedload('snapshot_metadata')).\
|
||||
all()
|
||||
query = model_query(context, models.Snapshot)
|
||||
|
||||
if filters:
|
||||
query = query.filter_by(**filters)
|
||||
|
||||
return query.filter_by(project_id=project_id).\
|
||||
options(joinedload('snapshot_metadata')).all()
|
||||
|
||||
|
||||
@require_context
|
||||
|
||||
@ -211,8 +211,8 @@ class SnapshotList(base.ObjectListBase, base.CinderObject):
|
||||
}
|
||||
|
||||
@base.remotable_classmethod
|
||||
def get_all(cls, context):
|
||||
snapshots = db.snapshot_get_all(context)
|
||||
def get_all(cls, context, search_opts):
|
||||
snapshots = db.snapshot_get_all(context, search_opts)
|
||||
return base.obj_make_list(context, cls(), objects.Snapshot,
|
||||
snapshots,
|
||||
expected_attrs=['metadata'])
|
||||
@ -224,8 +224,9 @@ class SnapshotList(base.ObjectListBase, base.CinderObject):
|
||||
snapshots, expected_attrs=['metadata'])
|
||||
|
||||
@base.remotable_classmethod
|
||||
def get_all_by_project(cls, context, project_id):
|
||||
snapshots = db.snapshot_get_all_by_project(context, project_id)
|
||||
def get_all_by_project(cls, context, project_id, search_opts):
|
||||
snapshots = db.snapshot_get_all_by_project(context, project_id,
|
||||
search_opts)
|
||||
return base.obj_make_list(context, cls(context), objects.Snapshot,
|
||||
snapshots, expected_attrs=['metadata'])
|
||||
|
||||
|
||||
@ -118,13 +118,13 @@ def stub_snapshot(id, **kwargs):
|
||||
return snapshot
|
||||
|
||||
|
||||
def stub_snapshot_get_all(self):
|
||||
def stub_snapshot_get_all(self, search_opts=None):
|
||||
return [stub_snapshot(100, project_id='fake'),
|
||||
stub_snapshot(101, project_id='superfake'),
|
||||
stub_snapshot(102, project_id='superduperfake')]
|
||||
|
||||
|
||||
def stub_snapshot_get_all_by_project(self, context):
|
||||
def stub_snapshot_get_all_by_project(self, context, search_opts=None):
|
||||
return [stub_snapshot(1)]
|
||||
|
||||
|
||||
|
||||
@ -311,95 +311,6 @@ class SnapshotApiTest(test.TestCase):
|
||||
resp_snapshot = resp_snapshots.pop()
|
||||
self.assertEqual(resp_snapshot['id'], UUID)
|
||||
|
||||
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
|
||||
def test_snapshot_list_by_status(self, snapshot_metadata_get):
|
||||
def stub_snapshot_get_all_by_project(context, project_id):
|
||||
return [
|
||||
stubs.stub_snapshot(1, display_name='backup1',
|
||||
status='available'),
|
||||
stubs.stub_snapshot(2, display_name='backup2',
|
||||
status='available'),
|
||||
stubs.stub_snapshot(3, display_name='backup3',
|
||||
status='creating'),
|
||||
]
|
||||
self.stubs.Set(db, 'snapshot_get_all_by_project',
|
||||
stub_snapshot_get_all_by_project)
|
||||
|
||||
# no status filter
|
||||
req = fakes.HTTPRequest.blank('/v1/snapshots')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 3)
|
||||
# single match
|
||||
req = fakes.HTTPRequest.blank('/v1/snapshots?status=creating')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 1)
|
||||
self.assertEqual(resp['snapshots'][0]['status'], 'creating')
|
||||
# multiple match
|
||||
req = fakes.HTTPRequest.blank('/v1/snapshots?status=available')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 2)
|
||||
for snapshot in resp['snapshots']:
|
||||
self.assertEqual(snapshot['status'], 'available')
|
||||
# no match
|
||||
req = fakes.HTTPRequest.blank('/v1/snapshots?status=error')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 0)
|
||||
|
||||
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
|
||||
def test_snapshot_list_by_volume(self, snapshot_metadata_get):
|
||||
def stub_snapshot_get_all_by_project(context, project_id):
|
||||
return [
|
||||
stubs.stub_snapshot(1, volume_id='vol1', status='creating'),
|
||||
stubs.stub_snapshot(2, volume_id='vol1', status='available'),
|
||||
stubs.stub_snapshot(3, volume_id='vol2', status='available'),
|
||||
]
|
||||
self.stubs.Set(db, 'snapshot_get_all_by_project',
|
||||
stub_snapshot_get_all_by_project)
|
||||
|
||||
# single match
|
||||
req = fakes.HTTPRequest.blank('/v1/snapshots?volume_id=vol2')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 1)
|
||||
self.assertEqual(resp['snapshots'][0]['volume_id'], 'vol2')
|
||||
# multiple match
|
||||
req = fakes.HTTPRequest.blank('/v1/snapshots?volume_id=vol1')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 2)
|
||||
for snapshot in resp['snapshots']:
|
||||
self.assertEqual(snapshot['volume_id'], 'vol1')
|
||||
# multiple filters
|
||||
req = fakes.HTTPRequest.blank('/v1/snapshots?volume_id=vol1'
|
||||
'&status=available')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 1)
|
||||
self.assertEqual(resp['snapshots'][0]['volume_id'], 'vol1')
|
||||
self.assertEqual(resp['snapshots'][0]['status'], 'available')
|
||||
|
||||
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
|
||||
def test_snapshot_list_by_name(self, snapshot_metadata_get):
|
||||
def stub_snapshot_get_all_by_project(context, project_id):
|
||||
return [
|
||||
stubs.stub_snapshot(1, display_name='backup1'),
|
||||
stubs.stub_snapshot(2, display_name='backup2'),
|
||||
stubs.stub_snapshot(3, display_name='backup3'),
|
||||
]
|
||||
self.stubs.Set(db, 'snapshot_get_all_by_project',
|
||||
stub_snapshot_get_all_by_project)
|
||||
|
||||
# no display_name filter
|
||||
req = fakes.HTTPRequest.blank('/v1/snapshots')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 3)
|
||||
# filter by one name
|
||||
req = fakes.HTTPRequest.blank('/v1/snapshots?display_name=backup2')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 1)
|
||||
self.assertEqual(resp['snapshots'][0]['display_name'], 'backup2')
|
||||
# filter no match
|
||||
req = fakes.HTTPRequest.blank('/v1/snapshots?display_name=backup4')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 0)
|
||||
|
||||
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
|
||||
def test_admin_list_snapshots_limited_to_project(self,
|
||||
snapshot_metadata_get):
|
||||
@ -414,7 +325,8 @@ class SnapshotApiTest(test.TestCase):
|
||||
def test_list_snapshots_with_limit_and_offset(self,
|
||||
snapshot_metadata_get):
|
||||
def list_snapshots_with_limit_and_offset(is_admin):
|
||||
def stub_snapshot_get_all_by_project(context, project_id):
|
||||
def stub_snapshot_get_all_by_project(context, project_id,
|
||||
search_opts):
|
||||
return [
|
||||
stubs.stub_snapshot(1, display_name='backup1'),
|
||||
stubs.stub_snapshot(2, display_name='backup2'),
|
||||
|
||||
@ -153,13 +153,13 @@ def stub_snapshot(id, **kwargs):
|
||||
return snapshot
|
||||
|
||||
|
||||
def stub_snapshot_get_all(self):
|
||||
def stub_snapshot_get_all(self, search_opts=None):
|
||||
return [stub_snapshot(100, project_id='fake'),
|
||||
stub_snapshot(101, project_id='superfake'),
|
||||
stub_snapshot(102, project_id='superduperfake')]
|
||||
|
||||
|
||||
def stub_snapshot_get_all_by_project(self, context):
|
||||
def stub_snapshot_get_all_by_project(self, context, search_opts=None):
|
||||
return [stub_snapshot(1)]
|
||||
|
||||
|
||||
|
||||
@ -323,95 +323,6 @@ class SnapshotApiTest(test.TestCase):
|
||||
resp_snapshot = resp_snapshots.pop()
|
||||
self.assertEqual(resp_snapshot['id'], UUID)
|
||||
|
||||
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
|
||||
def test_snapshot_list_by_status(self, snapshot_metadata_get):
|
||||
def stub_snapshot_get_all_by_project(context, project_id):
|
||||
return [
|
||||
stubs.stub_snapshot(1, display_name='backup1',
|
||||
status='available'),
|
||||
stubs.stub_snapshot(2, display_name='backup2',
|
||||
status='available'),
|
||||
stubs.stub_snapshot(3, display_name='backup3',
|
||||
status='creating'),
|
||||
]
|
||||
self.stubs.Set(db, 'snapshot_get_all_by_project',
|
||||
stub_snapshot_get_all_by_project)
|
||||
|
||||
# no status filter
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 3)
|
||||
# single match
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots?status=creating')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 1)
|
||||
self.assertEqual(resp['snapshots'][0]['status'], 'creating')
|
||||
# multiple match
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots?status=available')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 2)
|
||||
for snapshot in resp['snapshots']:
|
||||
self.assertEqual(snapshot['status'], 'available')
|
||||
# no match
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots?status=error')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 0)
|
||||
|
||||
@mock.patch('cinder.db.snapshot_metadata_get', return_value={})
|
||||
def test_snapshot_list_by_volume(self, snapshot_metadata_get):
|
||||
def stub_snapshot_get_all_by_project(context, project_id):
|
||||
return [
|
||||
stubs.stub_snapshot(1, volume_id='vol1', status='creating'),
|
||||
stubs.stub_snapshot(2, volume_id='vol1', status='available'),
|
||||
stubs.stub_snapshot(3, volume_id='vol2', status='available'),
|
||||
]
|
||||
self.stubs.Set(db, 'snapshot_get_all_by_project',
|
||||
stub_snapshot_get_all_by_project)
|
||||
|
||||
# single match
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots?volume_id=vol2')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 1)
|
||||
self.assertEqual(resp['snapshots'][0]['volume_id'], 'vol2')
|
||||
# multiple match
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots?volume_id=vol1')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 2)
|
||||
for snapshot in resp['snapshots']:
|
||||
self.assertEqual(snapshot['volume_id'], 'vol1')
|
||||
# multiple filters
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots?volume_id=vol1'
|
||||
'&status=available')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 1)
|
||||
self.assertEqual(resp['snapshots'][0]['volume_id'], 'vol1')
|
||||
self.assertEqual(resp['snapshots'][0]['status'], 'available')
|
||||
|
||||
@mock.patch('cinder.db.snapshot_metadata_get', return_value={})
|
||||
def test_snapshot_list_by_name(self, snapshot_metadata_get):
|
||||
def stub_snapshot_get_all_by_project(context, project_id):
|
||||
return [
|
||||
stubs.stub_snapshot(1, display_name='backup1'),
|
||||
stubs.stub_snapshot(2, display_name='backup2'),
|
||||
stubs.stub_snapshot(3, display_name='backup3'),
|
||||
]
|
||||
self.stubs.Set(db, 'snapshot_get_all_by_project',
|
||||
stub_snapshot_get_all_by_project)
|
||||
|
||||
# no name filter
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 3)
|
||||
# filter by one name
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots?name=backup2')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 1)
|
||||
self.assertEqual(resp['snapshots'][0]['name'], 'backup2')
|
||||
# filter no match
|
||||
req = fakes.HTTPRequest.blank('/v2/snapshots?name=backup4')
|
||||
resp = self.controller.index(req)
|
||||
self.assertEqual(len(resp['snapshots']), 0)
|
||||
|
||||
@mock.patch('cinder.db.snapshot_metadata_get', return_value=dict())
|
||||
def test_admin_list_snapshots_limited_to_project(self,
|
||||
snapshot_metadata_get):
|
||||
@ -426,7 +337,8 @@ class SnapshotApiTest(test.TestCase):
|
||||
def test_list_snapshots_with_limit_and_offset(self,
|
||||
snapshot_metadata_get):
|
||||
def list_snapshots_with_limit_and_offset(is_admin):
|
||||
def stub_snapshot_get_all_by_project(context, project_id):
|
||||
def stub_snapshot_get_all_by_project(context, project_id,
|
||||
search_opts):
|
||||
return [
|
||||
stubs.stub_snapshot(1, display_name='backup1'),
|
||||
stubs.stub_snapshot(2, display_name='backup2'),
|
||||
|
||||
@ -146,9 +146,12 @@ class TestSnapshotList(test_objects.BaseObjectsTestCase):
|
||||
fake_volume_obj = fake_volume.fake_volume_obj(self.context)
|
||||
volume_get_by_id.return_value = fake_volume_obj
|
||||
|
||||
snapshots = objects.SnapshotList.get_all(self.context)
|
||||
search_opts = mock.sentinel.search_opts
|
||||
snapshots = objects.SnapshotList.get_all(
|
||||
self.context, search_opts)
|
||||
self.assertEqual(1, len(snapshots))
|
||||
TestSnapshot._compare(self, fake_snapshot, snapshots[0])
|
||||
snapshot_get_all.assert_called_once_with(self.context, search_opts)
|
||||
|
||||
@mock.patch('cinder.db.snapshot_metadata_get', return_value={})
|
||||
@mock.patch('cinder.objects.Volume.get_by_id')
|
||||
@ -173,10 +176,14 @@ class TestSnapshotList(test_objects.BaseObjectsTestCase):
|
||||
fake_volume_obj = fake_volume.fake_volume_obj(self.context)
|
||||
volume_get_by_id.return_value = fake_volume_obj
|
||||
|
||||
search_opts = mock.sentinel.search_opts
|
||||
snapshots = objects.SnapshotList.get_all_by_project(
|
||||
self.context, self.project_id)
|
||||
self.context, self.project_id, search_opts)
|
||||
self.assertEqual(1, len(snapshots))
|
||||
TestSnapshot._compare(self, fake_snapshot, snapshots[0])
|
||||
get_all_by_project.assert_called_once_with(self.context,
|
||||
self.project_id,
|
||||
search_opts)
|
||||
|
||||
@mock.patch('cinder.db.snapshot_metadata_get', return_value={})
|
||||
@mock.patch('cinder.objects.volume.Volume.get_by_id')
|
||||
|
||||
@ -954,11 +954,72 @@ class DBAPISnapshotTestCase(BaseTest):
|
||||
actual = db.snapshot_data_get_for_project(self.ctxt, 'project1')
|
||||
self.assertEqual(actual, (1, 42))
|
||||
|
||||
def test_snapshot_get_all(self):
|
||||
def test_snapshot_get_all_by_filter(self):
|
||||
db.volume_create(self.ctxt, {'id': 1})
|
||||
snapshot = db.snapshot_create(self.ctxt, {'id': 1, 'volume_id': 1})
|
||||
self._assertEqualListsOfObjects([snapshot],
|
||||
db.snapshot_get_all(self.ctxt),
|
||||
db.volume_create(self.ctxt, {'id': 2})
|
||||
snapshot1 = db.snapshot_create(self.ctxt, {'id': 1, 'volume_id': 1,
|
||||
'display_name': 'one',
|
||||
'status': 'available'})
|
||||
snapshot2 = db.snapshot_create(self.ctxt, {'id': 2, 'volume_id': 1,
|
||||
'display_name': 'two',
|
||||
'status': 'creating'})
|
||||
snapshot3 = db.snapshot_create(self.ctxt, {'id': 3, 'volume_id': 2,
|
||||
'display_name': 'three',
|
||||
'status': 'available'})
|
||||
# no filter
|
||||
filters = {}
|
||||
snapshots = db.snapshot_get_all(self.ctxt, filters=filters)
|
||||
self.assertEqual(3, len(snapshots))
|
||||
# single match
|
||||
filters = {'display_name': 'two'}
|
||||
self._assertEqualListsOfObjects([snapshot2],
|
||||
db.snapshot_get_all(
|
||||
self.ctxt,
|
||||
filters),
|
||||
ignored_keys=['metadata', 'volume'])
|
||||
filters = {'volume_id': 2}
|
||||
self._assertEqualListsOfObjects([snapshot3],
|
||||
db.snapshot_get_all(
|
||||
self.ctxt,
|
||||
filters),
|
||||
ignored_keys=['metadata', 'volume'])
|
||||
# filter no match
|
||||
filters = {'volume_id': 5}
|
||||
self._assertEqualListsOfObjects([],
|
||||
db.snapshot_get_all(
|
||||
self.ctxt,
|
||||
filters),
|
||||
ignored_keys=['metadata', 'volume'])
|
||||
filters = {'status': 'error'}
|
||||
self._assertEqualListsOfObjects([],
|
||||
db.snapshot_get_all(
|
||||
self.ctxt,
|
||||
filters),
|
||||
ignored_keys=['metadata', 'volume'])
|
||||
# multiple match
|
||||
filters = {'volume_id': 1}
|
||||
self._assertEqualListsOfObjects([snapshot1, snapshot2],
|
||||
db.snapshot_get_all(
|
||||
self.ctxt,
|
||||
filters),
|
||||
ignored_keys=['metadata', 'volume'])
|
||||
filters = {'status': 'available'}
|
||||
self._assertEqualListsOfObjects([snapshot1, snapshot3],
|
||||
db.snapshot_get_all(
|
||||
self.ctxt,
|
||||
filters),
|
||||
ignored_keys=['metadata', 'volume'])
|
||||
filters = {'volume_id': 1, 'status': 'available'}
|
||||
self._assertEqualListsOfObjects([snapshot1],
|
||||
db.snapshot_get_all(
|
||||
self.ctxt,
|
||||
filters),
|
||||
ignored_keys=['metadata', 'volume'])
|
||||
filters = {'fake_key': 'fake'}
|
||||
self._assertEqualListsOfObjects([],
|
||||
db.snapshot_get_all(
|
||||
self.ctxt,
|
||||
filters),
|
||||
ignored_keys=['metadata', 'volume'])
|
||||
|
||||
def test_snapshot_get_by_host(self):
|
||||
|
||||
@ -507,23 +507,12 @@ class API(base.Base):
|
||||
if (context.is_admin and 'all_tenants' in search_opts):
|
||||
# Need to remove all_tenants to pass the filtering below.
|
||||
del search_opts['all_tenants']
|
||||
snapshots = objects.SnapshotList.get_all(context)
|
||||
snapshots = objects.SnapshotList.get_all(context,
|
||||
search_opts)
|
||||
else:
|
||||
snapshots = objects.SnapshotList.get_all_by_project(
|
||||
context, context.project_id)
|
||||
context, context.project_id, search_opts)
|
||||
|
||||
if search_opts:
|
||||
LOG.debug("Searching by: %s", search_opts)
|
||||
|
||||
results = []
|
||||
not_found = object()
|
||||
for snapshot in snapshots:
|
||||
for opt, value in search_opts.items():
|
||||
if snapshot.get(opt, not_found) != value:
|
||||
break
|
||||
else:
|
||||
results.append(snapshot)
|
||||
snapshots.objects = results
|
||||
LOG.info(_LI("Get all snaphsots completed successfully."))
|
||||
return snapshots
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user