From 1852eaf48e965a00f1fb7458cfb2ec0a2125a2ab Mon Sep 17 00:00:00 2001 From: gordon chung Date: Fri, 3 Jul 2015 12:18:57 -0400 Subject: [PATCH] add mandatory limit value to resource list unrestricted listing of resources can require significant memory. this patch implements mandatory limit on resource-list Change-Id: I5820ce759b20757febc1b92c100f584234b36b68 Implements: blueprint mandatory-limit --- ceilometer/api/controllers/v2/resources.py | 8 ++-- ceilometer/storage/impl_db2.py | 11 ++++- ceilometer/storage/impl_hbase.py | 8 +++- ceilometer/storage/impl_mongodb.py | 30 ++++++++++--- ceilometer/storage/impl_sqlalchemy.py | 7 ++- .../api/v2/test_list_resources_scenarios.py | 45 +++++++++++++++++++ 6 files changed, 93 insertions(+), 16 deletions(-) diff --git a/ceilometer/api/controllers/v2/resources.py b/ceilometer/api/controllers/v2/resources.py index b90255155b..b517f4dc2b 100644 --- a/ceilometer/api/controllers/v2/resources.py +++ b/ceilometer/api/controllers/v2/resources.py @@ -129,8 +129,8 @@ class ResourcesController(rest.RestController): return Resource.from_db_and_links(resources[0], self._resource_links(resource_id)) - @wsme_pecan.wsexpose([Resource], [base.Query], int) - def get_all(self, q=None, meter_links=1): + @wsme_pecan.wsexpose([Resource], [base.Query], int, int) + def get_all(self, q=None, limit=None, meter_links=1): """Retrieve definitions of all of the resources. :param q: Filter rules for the resources to be returned. @@ -140,11 +140,13 @@ class ResourcesController(rest.RestController): rbac.enforce('get_resources', pecan.request) q = q or [] + limit = utils.enforce_limit(limit) kwargs = utils.query_to_kwargs( q, pecan.request.storage_conn.get_resources) resources = [ Resource.from_db_and_links(r, self._resource_links(r.resource_id, meter_links)) - for r in pecan.request.storage_conn.get_resources(**kwargs)] + for r in pecan.request.storage_conn.get_resources(limit=limit, + **kwargs)] return resources diff --git a/ceilometer/storage/impl_db2.py b/ceilometer/storage/impl_db2.py index 187bc6caa3..bc54d34ae1 100644 --- a/ceilometer/storage/impl_db2.py +++ b/ceilometer/storage/impl_db2.py @@ -258,7 +258,7 @@ class Connection(pymongo_base.Connection): def get_resources(self, user=None, project=None, source=None, start_timestamp=None, start_timestamp_op=None, end_timestamp=None, end_timestamp_op=None, - metaquery=None, resource=None): + metaquery=None, resource=None, limit=None): """Return an iterable of models.Resource instances :param user: Optional ID for user that owns the resource. @@ -270,7 +270,10 @@ class Connection(pymongo_base.Connection): :param end_timestamp_op: Optional end time operator, like lt, le. :param metaquery: Optional dict with metadata to match on. :param resource: Optional resource filter. + :param limit: Maximum number of results to return. """ + if limit == 0: + return metaquery = pymongo_utils.improve_keys(metaquery, metaquery=True) or {} q = {} @@ -302,7 +305,11 @@ class Connection(pymongo_base.Connection): sort_instructions = self._build_sort_instructions(sort_keys=sort_keys, sort_dir='desc') resource = lambda x: x['resource_id'] - meters = self.db.meter.find(q, sort=sort_instructions) + if limit is not None: + meters = self.db.meter.find(q, sort=sort_instructions, + limit=limit) + else: + meters = self.db.meter.find(q, sort=sort_instructions) for resource_id, r_meters in itertools.groupby(meters, key=resource): # Because we have to know first/last timestamp, and we need a full # list of references to the resource's meters, we need a tuple diff --git a/ceilometer/storage/impl_hbase.py b/ceilometer/storage/impl_hbase.py index 0edcab0a0b..64a9491d5b 100644 --- a/ceilometer/storage/impl_hbase.py +++ b/ceilometer/storage/impl_hbase.py @@ -186,7 +186,7 @@ class Connection(hbase_base.Connection, base.Connection): def get_resources(self, user=None, project=None, source=None, start_timestamp=None, start_timestamp_op=None, end_timestamp=None, end_timestamp_op=None, - metaquery=None, resource=None): + metaquery=None, resource=None, limit=None): """Return an iterable of models.Resource instances :param user: Optional ID for user that owns the resource. @@ -198,7 +198,10 @@ class Connection(hbase_base.Connection, base.Connection): :param end_timestamp_op: Optional end time operator, like lt, le. :param metaquery: Optional dict with metadata to match on. :param resource: Optional resource filter. + :param limit: Maximum number of results to return. """ + if limit == 0: + return q = hbase_utils.make_query(metaquery=metaquery, user_id=user, project_id=project, resource_id=resource, source=source) @@ -210,7 +213,8 @@ class Connection(hbase_base.Connection, base.Connection): with self.conn_pool.connection() as conn: resource_table = conn.table(self.RESOURCE_TABLE) LOG.debug(_("Query Resource table: %s") % q) - for resource_id, data in resource_table.scan(filter=q): + for resource_id, data in resource_table.scan(filter=q, + limit=limit): f_res, sources, meters, md = hbase_utils.deserialize_entry( data) resource_id = hbase_utils.encode_unicode(resource_id) diff --git a/ceilometer/storage/impl_mongodb.py b/ceilometer/storage/impl_mongodb.py index adb72576fc..200b53603a 100644 --- a/ceilometer/storage/impl_mongodb.py +++ b/ceilometer/storage/impl_mongodb.py @@ -614,7 +614,7 @@ class Connection(pymongo_base.Connection): def _get_time_constrained_resources(self, query, start_timestamp, start_timestamp_op, end_timestamp, end_timestamp_op, - metaquery, resource): + metaquery, resource, limit): """Return an iterable of models.Resource instances Items are constrained by sample timestamp. @@ -661,7 +661,12 @@ class Connection(pymongo_base.Connection): query=query) try: - for r in self.db[out].find(sort=sort_instructions): + if limit is not None: + results = self.db[out].find(sort=sort_instructions, + limit=limit) + else: + results = self.db[out].find(sort=sort_instructions) + for r in results: resource = r['value'] yield models.Resource( resource_id=r['_id'], @@ -674,7 +679,7 @@ class Connection(pymongo_base.Connection): finally: self.db[out].drop() - def _get_floating_resources(self, query, metaquery, resource): + def _get_floating_resources(self, query, metaquery, resource, limit): """Return an iterable of models.Resource instances Items are unconstrained by timestamp. @@ -693,7 +698,13 @@ class Connection(pymongo_base.Connection): for i in keys] sort_instructions = self._build_sort_instructions(sort_keys)[0] - for r in self.db.resource.find(query, sort=sort_instructions): + if limit is not None: + results = self.db.resource.find(query, sort=sort_instructions, + limit=limit) + else: + results = self.db.resource.find(query, sort=sort_instructions) + + for r in results: yield models.Resource( resource_id=r['_id'], user_id=r['user_id'], @@ -708,7 +719,7 @@ class Connection(pymongo_base.Connection): def get_resources(self, user=None, project=None, source=None, start_timestamp=None, start_timestamp_op=None, end_timestamp=None, end_timestamp_op=None, - metaquery=None, resource=None): + metaquery=None, resource=None, limit=None): """Return an iterable of models.Resource instances :param user: Optional ID for user that owns the resource. @@ -720,7 +731,10 @@ class Connection(pymongo_base.Connection): :param end_timestamp_op: Optional end time operator, like lt, le. :param metaquery: Optional dict with metadata to match on. :param resource: Optional resource filter. + :param limit: Maximum number of results to return. """ + if limit == 0: + return metaquery = pymongo_utils.improve_keys(metaquery, metaquery=True) or {} query = {} @@ -737,9 +751,11 @@ class Connection(pymongo_base.Connection): start_timestamp_op, end_timestamp, end_timestamp_op, - metaquery, resource) + metaquery, resource, + limit) else: - return self._get_floating_resources(query, metaquery, resource) + return self._get_floating_resources(query, metaquery, resource, + limit) def _aggregate_param(self, fragment_key, aggregate): fragment_map = self.STANDARD_AGGREGATES[fragment_key] diff --git a/ceilometer/storage/impl_sqlalchemy.py b/ceilometer/storage/impl_sqlalchemy.py index 53a3857eb5..51a9abf9af 100644 --- a/ceilometer/storage/impl_sqlalchemy.py +++ b/ceilometer/storage/impl_sqlalchemy.py @@ -384,7 +384,7 @@ class Connection(base.Connection): def get_resources(self, user=None, project=None, source=None, start_timestamp=None, start_timestamp_op=None, end_timestamp=None, end_timestamp_op=None, - metaquery=None, resource=None): + metaquery=None, resource=None, limit=None): """Return an iterable of api_models.Resource instances :param user: Optional ID for user that owns the resource. @@ -396,7 +396,10 @@ class Connection(base.Connection): :param end_timestamp_op: Optional end time operator, like lt, le. :param metaquery: Optional dict with metadata to match on. :param resource: Optional resource filter. + :param limit: Maximum number of results to return. """ + if limit == 0: + return s_filter = storage.SampleFilter(user=user, project=project, source=source, @@ -414,7 +417,7 @@ class Connection(base.Connection): models.Sample.resource_id == models.Resource.internal_id) res_q = make_query_from_filter(session, res_q, s_filter, require_meter=False) - + res_q = res_q.limit(limit) if limit else res_q for res_id in res_q.all(): # get max and min sample timestamp value diff --git a/ceilometer/tests/api/v2/test_list_resources_scenarios.py b/ceilometer/tests/api/v2/test_list_resources_scenarios.py index 7422be737e..136af913a8 100644 --- a/ceilometer/tests/api/v2/test_list_resources_scenarios.py +++ b/ceilometer/tests/api/v2/test_list_resources_scenarios.py @@ -20,6 +20,7 @@ import json from oslo_utils import timeutils import six +import webtest.app from ceilometer.publisher import utils from ceilometer import sample @@ -522,3 +523,47 @@ class TestListResources(v2.FunctionalTest, self.assertEqual(links[0]['rel'], 'self') self.assertTrue((self.PATH_PREFIX + '/resources/resource-id') in links[0]['href']) + + +class TestListResourcesRestriction(v2.FunctionalTest, + tests_db.MixinTestsWithBackendScenarios): + + def setUp(self): + super(TestListResourcesRestriction, self).setUp() + self.CONF.set_override('default_api_return_limit', 10, group='api') + for i in range(20): + s = sample.Sample( + 'volume.size', + 'gauge', + 'GiB', + 5 + i, + 'user-id', + 'project1', + 'resource-id%s' % i, + timestamp=(datetime.datetime(2012, 9, 25, 10, 30) + + datetime.timedelta(seconds=i)), + resource_metadata={'display_name': 'test-volume', + 'tag': 'self.sample', + }, + source='source1', + ) + msg = utils.meter_message_from_counter( + s, self.CONF.publisher.telemetry_secret, + ) + self.conn.record_metering_data(msg) + + def test_resource_limit(self): + data = self.get_json('/resources?limit=1') + self.assertEqual(1, len(data)) + + def test_resource_limit_negative(self): + self.assertRaises(webtest.app.AppError, self.get_json, + '/resources?limit=-2') + + def test_resource_limit_bigger(self): + data = self.get_json('/resources?limit=42') + self.assertEqual(20, len(data)) + + def test_resource_default_limit(self): + data = self.get_json('/resources') + self.assertEqual(10, len(data))