From 169482c8871fb5c76a2698f8162323a5de7d0c3f Mon Sep 17 00:00:00 2001 From: Yuriy Nesenenko Date: Tue, 30 Jun 2015 19:06:47 +0300 Subject: [PATCH] Filter cgsnapshots data on the DB side It filters cgsnapshots data on the DB side to improve performance. Change-Id: I399ddec9d21345bf0496a293059963363c7c8cbe Partial-Implements: blueprint data-filtering-on-the-db-side --- cinder/consistencygroup/api.py | 16 ++-------- cinder/db/api.py | 12 ++++---- cinder/db/sqlalchemy/api.py | 49 ++++++++++++++++++++++++------ cinder/tests/unit/test_db_api.py | 52 +++++++++++++++++++++++++++++++- 4 files changed, 98 insertions(+), 31 deletions(-) diff --git a/cinder/consistencygroup/api.py b/cinder/consistencygroup/api.py index 6d7c5e036c0..9b560015755 100644 --- a/cinder/consistencygroup/api.py +++ b/cinder/consistencygroup/api.py @@ -686,21 +686,9 @@ 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'] - cgsnapshots = self.db.cgsnapshot_get_all(context) + cgsnapshots = self.db.cgsnapshot_get_all(context, search_opts) else: cgsnapshots = self.db.cgsnapshot_get_all_by_project( - context.elevated(), context.project_id) + context.elevated(), context.project_id, search_opts) - if search_opts: - LOG.debug("Searching by: %s", search_opts) - - results = [] - not_found = object() - for cgsnapshot in cgsnapshots: - for opt, value in search_opts.items(): - if cgsnapshot.get(opt, not_found) != value: - break - else: - results.append(cgsnapshot) - cgsnapshots = results return cgsnapshots diff --git a/cinder/db/api.py b/cinder/db/api.py index 0933888c329..3c5779d8143 100644 --- a/cinder/db/api.py +++ b/cinder/db/api.py @@ -936,9 +936,9 @@ def cgsnapshot_get(context, cgsnapshot_id): return IMPL.cgsnapshot_get(context, cgsnapshot_id) -def cgsnapshot_get_all(context): +def cgsnapshot_get_all(context, filters=None): """Get all cgsnapshots.""" - return IMPL.cgsnapshot_get_all(context) + return IMPL.cgsnapshot_get_all(context, filters) def cgsnapshot_create(context, values): @@ -946,14 +946,14 @@ def cgsnapshot_create(context, values): return IMPL.cgsnapshot_create(context, values) -def cgsnapshot_get_all_by_group(context, group_id): +def cgsnapshot_get_all_by_group(context, group_id, filters=None): """Get all cgsnapshots belonging to a consistency group.""" - return IMPL.cgsnapshot_get_all_by_group(context, group_id) + return IMPL.cgsnapshot_get_all_by_group(context, group_id, filters) -def cgsnapshot_get_all_by_project(context, project_id): +def cgsnapshot_get_all_by_project(context, project_id, filters=None): """Get all cgsnapshots belonging to a project.""" - return IMPL.cgsnapshot_get_all_by_project(context, project_id) + return IMPL.cgsnapshot_get_all_by_project(context, project_id, filters) def cgsnapshot_update(context, cgsnapshot_id, values): diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index dffb2f083bd..02208a54447 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -3601,23 +3601,52 @@ def cgsnapshot_get(context, cgsnapshot_id): return _cgsnapshot_get(context, cgsnapshot_id) -@require_admin_context -def cgsnapshot_get_all(context): - return model_query(context, models.Cgsnapshot).all() +def is_valid_model_filters(model, filters): + """Return True if filter values exist on the model + + :param model: a Cinder model + :param filters: dictionary of filters + """ + for key in filters.keys(): + try: + getattr(model, key) + except AttributeError: + LOG.debug("'%s' filter key is not valid.", key) + return False + return True + + +def _cgsnapshot_get_all(context, project_id=None, group_id=None, filters=None): + query = model_query(context, models.Cgsnapshot) + + if filters: + if not is_valid_model_filters(models.Cgsnapshot, filters): + return [] + query = query.filter_by(**filters) + + if project_id: + query.filter_by(project_id=project_id) + + if group_id: + query.filter_by(consistencygroup_id=group_id) + + return query.all() @require_admin_context -def cgsnapshot_get_all_by_group(context, group_id): - return model_query(context, models.Cgsnapshot).\ - filter_by(consistencygroup_id=group_id).all() +def cgsnapshot_get_all(context, filters=None): + return _cgsnapshot_get_all(context, filters=filters) + + +@require_admin_context +def cgsnapshot_get_all_by_group(context, group_id, filters=None): + return _cgsnapshot_get_all(context, group_id=group_id, filters=filters) @require_context -def cgsnapshot_get_all_by_project(context, project_id): +def cgsnapshot_get_all_by_project(context, project_id, filters=None): authorize_project_context(context, project_id) - - return model_query(context, models.Cgsnapshot).\ - filter_by(project_id=project_id).all() + return _cgsnapshot_get_all(context, project_id=project_id, filters=filters) @require_context diff --git a/cinder/tests/unit/test_db_api.py b/cinder/tests/unit/test_db_api.py index b8abe0cc139..b80a850cce3 100644 --- a/cinder/tests/unit/test_db_api.py +++ b/cinder/tests/unit/test_db_api.py @@ -28,7 +28,6 @@ from cinder import exception from cinder import quota from cinder import test - CONF = cfg.CONF THREE = 3 @@ -1187,6 +1186,57 @@ class DBAPISnapshotTestCase(BaseTest): self.assertEqual(should_be, db.snapshot_metadata_get(self.ctxt, 1)) +class DBAPICgsnapshotTestCase(BaseTest): + """Tests for cinder.db.api.cgsnapshot_*.""" + + def test_cgsnapshot_get_all_by_filter(self): + cgsnapshot1 = db.cgsnapshot_create(self.ctxt, {'id': 1, + 'consistencygroup_id': 'g1'}) + cgsnapshot2 = db.cgsnapshot_create(self.ctxt, {'id': 2, + 'consistencygroup_id': 'g1'}) + cgsnapshot3 = db.cgsnapshot_create(self.ctxt, {'id': 3, + 'consistencygroup_id': 'g2'}) + tests = [ + ({'consistencygroup_id': 'g1'}, [cgsnapshot1, cgsnapshot2]), + ({'id': 3}, [cgsnapshot3]), + ({'fake_key': 'fake'}, []) + ] + + # no filter + filters = None + cgsnapshots = db.cgsnapshot_get_all(self.ctxt, filters=filters) + self.assertEqual(3, len(cgsnapshots)) + + for filters, expected in tests: + self._assertEqualListsOfObjects(expected, + db.cgsnapshot_get_all( + self.ctxt, + filters)) + + def test_cgsnapshot_get_all_by_project(self): + cgsnapshot1 = db.cgsnapshot_create(self.ctxt, + {'id': 1, + 'consistencygroup_id': 'g1', + 'project_id': 1}) + cgsnapshot2 = db.cgsnapshot_create(self.ctxt, + {'id': 2, + 'consistencygroup_id': 'g1', + 'project_id': 1}) + project_id = 1 + tests = [ + ({'id': 1}, [cgsnapshot1]), + ({'consistencygroup_id': 'g1'}, [cgsnapshot1, cgsnapshot2]), + ({'fake_key': 'fake'}, []) + ] + + for filters, expected in tests: + self._assertEqualListsOfObjects(expected, + db.cgsnapshot_get_all_by_project( + self.ctxt, + project_id, + filters)) + + class DBAPIVolumeTypeTestCase(BaseTest): """Tests for the db.api.volume_type_* methods."""