From 5a65b4de0fe46967e76832c2acf4924a53b18612 Mon Sep 17 00:00:00 2001 From: ZhiQiang Fan Date: Sat, 7 Feb 2015 16:23:20 +0800 Subject: [PATCH] metering data ttl sql backend breaks resource metadata The implementation for sql clear_expired_metering_data() uses delete() on join() for four metatada tables, which will cause clear whole tables no matter what join returns. And it associates metadata id with sample id which makes no sense, the metadata id is resource's internal id. Change-Id: I98219bf27a0a765838d45386b7ae6b6b980b1ae9 Closes-Bug: #1419239 --- ceilometer/storage/impl_sqlalchemy.py | 28 ++++++++++--------- .../tests/storage/test_impl_sqlalchemy.py | 11 ++++++-- 2 files changed, 23 insertions(+), 16 deletions(-) diff --git a/ceilometer/storage/impl_sqlalchemy.py b/ceilometer/storage/impl_sqlalchemy.py index 48b4e13c8..8af8b38db 100644 --- a/ceilometer/storage/impl_sqlalchemy.py +++ b/ceilometer/storage/impl_sqlalchemy.py @@ -357,23 +357,25 @@ class Connection(base.Connection): end = timeutils.utcnow() - datetime.timedelta(seconds=ttl) sample_q = (session.query(models.Sample) .filter(models.Sample.timestamp < end)) - - sample_subq = sample_q.subquery() - for table in [models.MetaText, models.MetaBigInt, - models.MetaFloat, models.MetaBool]: - (session.query(table) - .join(sample_subq, sample_subq.c.id == table.id) - .delete()) - rows = sample_q.delete() + LOG.info(_("%d samples removed from database"), rows) + # remove Meter definitions with no matching samples (session.query(models.Meter) .filter(~models.Meter.samples.any()) - .delete(synchronize_session='fetch')) - (session.query(models.Resource) - .filter(~models.Resource.samples.any()) - .delete(synchronize_session='fetch')) - LOG.info(_("%d samples removed from database"), rows) + .delete(synchronize_session=False)) + + # remove resources with no matching samples + resource_q = (session.query(models.Resource.internal_id) + .filter(~models.Resource.samples.any())) + resource_subq = resource_q.subquery() + # remove metadata of cleaned resources + for table in [models.MetaText, models.MetaBigInt, + models.MetaFloat, models.MetaBool]: + (session.query(table) + .filter(table.id.in_(resource_subq)) + .delete(synchronize_session=False)) + resource_q.delete(synchronize_session=False) def get_resources(self, user=None, project=None, source=None, start_timestamp=None, start_timestamp_op=None, diff --git a/ceilometer/tests/storage/test_impl_sqlalchemy.py b/ceilometer/tests/storage/test_impl_sqlalchemy.py index a082b1e8f..38e4694d2 100644 --- a/ceilometer/tests/storage/test_impl_sqlalchemy.py +++ b/ceilometer/tests/storage/test_impl_sqlalchemy.py @@ -180,14 +180,19 @@ class RelationshipTest(scenarios.DBTestBase): self.conn.clear_expired_metering_data(3 * 60) session = self.conn._engine_facade.get_session() + self.assertEqual(5, session.query(sql_models.Sample).count()) + + resource_ids = (session.query(sql_models.Resource.internal_id) + .group_by(sql_models.Resource.internal_id)) meta_tables = [sql_models.MetaText, sql_models.MetaFloat, sql_models.MetaBigInt, sql_models.MetaBool] + s = set() for table in meta_tables: self.assertEqual(0, (session.query(table) - .filter(~table.id.in_( - session.query(sql_models.Sample.id) - .group_by(sql_models.Sample.id))).count() + .filter(~table.id.in_(resource_ids)).count() )) + s.update(session.query(table.id).all()) + self.assertEqual(set(resource_ids.all()), s) class CapabilitiesTest(test_base.BaseTestCase):