From b392f8a8f6d5a704bd4e1321e48894077230d2e3 Mon Sep 17 00:00:00 2001 From: bhagyashris Date: Fri, 15 Jul 2016 19:40:46 +0530 Subject: [PATCH] Fix db purge for quality_of_service_specs FK constraint db purge command raises FK constraint while purging 1. quality_of_service_specs because the "id" of quality_of_service_specs is used as reference key in same table as "specs_id". 2. volume_types and quality_of_service_specs deleted rows Delete 1. reference/child records first before deleting the parent record from quality_of_service_specs. 2. volume_types records before deleting quality_of_service_specs to solve this issue. Closes-Bug: #1599830 Change-Id: Ic4cb24b1573ab06a90ba47479f69468c4adc3dcb --- cinder/db/sqlalchemy/api.py | 11 ++++++- cinder/tests/unit/db/test_purge.py | 52 ++++++++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 3 deletions(-) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index f345a2d8863..e5c4764c3bb 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -4733,7 +4733,8 @@ def purge_deleted_rows(context, age_in_days): # Reorder the list so the volumes and volume_types tables are last # to avoid FK constraints - for table in ("volume_types", "snapshots", "volumes", "clusters"): + for table in ("volume_types", "quality_of_service_specs", + "snapshots", "volumes", "clusters"): tables.remove(table) tables.append(table) @@ -4745,6 +4746,14 @@ def purge_deleted_rows(context, age_in_days): deleted_age = timeutils.utcnow() - dt.timedelta(days=age_in_days) try: with session.begin(): + # Delete child records first from quality_of_service_specs + # table to avoid FK constraints + if table == "quality_of_service_specs": + session.query(models.QualityOfServiceSpecs).filter( + and_(models.QualityOfServiceSpecs.specs_id.isnot( + None), models.QualityOfServiceSpecs.deleted == 1, + models.QualityOfServiceSpecs.deleted_at < + deleted_age)).delete() result = session.execute( t.delete() .where(t.c.deleted_at < deleted_age)) diff --git a/cinder/tests/unit/db/test_purge.py b/cinder/tests/unit/db/test_purge.py index 7e0cedcaef3..65eddbcc3a5 100644 --- a/cinder/tests/unit/db/test_purge.py +++ b/cinder/tests/unit/db/test_purge.py @@ -60,6 +60,9 @@ class PurgeDeletedTest(test.TestCase): self.vgm = sqlalchemyutils.get_table( self.engine, "volume_glance_metadata") + self.qos = sqlalchemyutils.get_table( + self.engine, "quality_of_service_specs") + self.uuidstrs = [] for unused in range(6): self.uuidstrs.append(uuid.uuid4().hex) @@ -89,6 +92,19 @@ class PurgeDeletedTest(test.TestCase): snapshot_id=uuidstr, key='image_name', value='test') self.conn.execute(ins_stmt) + ins_stmt = self.qos.insert().values( + id=uuidstr, key='QoS_Specs_Name', value='test') + self.conn.execute(ins_stmt) + + ins_stmt = self.vol_types.insert().values( + id=uuid.uuid4().hex, qos_specs_id=uuidstr) + self.conn.execute(ins_stmt) + + ins_stmt = self.qos.insert().values( + id=uuid.uuid4().hex, specs_id=uuidstr, key='desc', + value='test') + self.conn.execute(ins_stmt) + # Set 4 of them deleted, 2 are 60 days ago, 2 are 20 days ago old = timeutils.utcnow() - datetime.timedelta(days=20) older = timeutils.utcnow() - datetime.timedelta(days=60) @@ -145,6 +161,25 @@ class PurgeDeletedTest(test.TestCase): where(self.vgm.c.snapshot_id.in_(self.uuidstrs[4:6]))\ .values(deleted_at=older) + make_qos_old = self.qos.update().where( + self.qos.c.id.in_(self.uuidstrs[1:3])).values(deleted_at=old) + make_qos_older = self.qos.update().where( + self.qos.c.id.in_(self.uuidstrs[4:6])).values(deleted_at=older) + + make_qos_child_record_old = self.qos.update().where( + self.qos.c.specs_id.in_(self.uuidstrs[1:3])).values( + deleted_at=old) + make_qos_child_record_older = self.qos.update().where( + self.qos.c.specs_id.in_(self.uuidstrs[4:6])).values( + deleted_at=older) + + make_vol_types1_old = self.vol_types.update().where( + self.vol_types.c.qos_specs_id.in_(self.uuidstrs[1:3])).values( + deleted_at=old) + make_vol_types1_older = self.vol_types.update().where( + self.vol_types.c.qos_specs_id.in_(self.uuidstrs[4:6])).values( + deleted_at=older) + self.conn.execute(make_vol_old) self.conn.execute(make_vol_older) self.conn.execute(make_vol_meta_old) @@ -165,6 +200,15 @@ class PurgeDeletedTest(test.TestCase): self.conn.execute(make_snap_glance_meta_old) self.conn.execute(make_snap_glance_meta_older) + self.conn.execute(make_qos_old) + self.conn.execute(make_qos_older) + + self.conn.execute(make_qos_child_record_old) + self.conn.execute(make_qos_child_record_older) + + self.conn.execute(make_vol_types1_old) + self.conn.execute(make_vol_types1_older) + def test_purge_deleted_rows_old(self): dialect = self.engine.url.get_dialect() if dialect == sqlite.dialect: @@ -186,15 +230,17 @@ class PurgeDeletedTest(test.TestCase): snap_rows = self.session.query(self.snapshots).count() snap_meta_rows = self.session.query(self.sm).count() vol_glance_meta_rows = self.session.query(self.vgm).count() + qos_rows = self.session.query(self.qos).count() # Verify that we only deleted 2 self.assertEqual(4, vol_rows) self.assertEqual(4, vol_meta_rows) - self.assertEqual(4, vol_type_rows) + self.assertEqual(8, vol_type_rows) self.assertEqual(4, vol_type_proj_rows) self.assertEqual(4, snap_rows) self.assertEqual(4, snap_meta_rows) self.assertEqual(8, vol_glance_meta_rows) + self.assertEqual(8, qos_rows) def test_purge_deleted_rows_older(self): dialect = self.engine.url.get_dialect() @@ -217,15 +263,17 @@ class PurgeDeletedTest(test.TestCase): snap_rows = self.session.query(self.snapshots).count() snap_meta_rows = self.session.query(self.sm).count() vol_glance_meta_rows = self.session.query(self.vgm).count() + qos_rows = self.session.query(self.qos).count() # Verify that we only have 2 rows now self.assertEqual(2, vol_rows) self.assertEqual(2, vol_meta_rows) - self.assertEqual(2, vol_type_rows) + self.assertEqual(4, vol_type_rows) self.assertEqual(2, vol_type_proj_rows) self.assertEqual(2, snap_rows) self.assertEqual(2, snap_meta_rows) self.assertEqual(4, vol_glance_meta_rows) + self.assertEqual(4, qos_rows) def test_purge_deleted_rows_bad_args(self): # Test with no age argument