From 6f63347eef802cd593cf63b6dcc130f99c651af8 Mon Sep 17 00:00:00 2001 From: Gorka Eguileor Date: Mon, 6 Jun 2016 13:47:48 +0200 Subject: [PATCH] Fix MySQL DB error in Delete CG When running a delete CG operation on a MySQL DB engine we'll get an error from the DB: "You can't specify target table 'consistencygroups' for update in FROM clause". This is caused by a non standard behavior only in MySQL, as SQLite and PostgreSQL work as expected, on filter `cg_creating_from_src` that checks that we are not using the CG we are trying to delete to create another CG at this very moment, CGs that were previously created from this CG are not considered. This patch changes the way we perform the filter using a select subquery as a workaround for MySQL unexpected behavior and updates devref to warn about this unexpected MySQL behavior. Change-Id: Ic10de411ffeceb00f1e8525906995bd8b2f49777 Closes-Bug: #1588487 --- cinder/db/sqlalchemy/api.py | 22 +++++--- .../api/contrib/test_consistencygroups.py | 23 +++++++++ doc/source/devref/api_conditional_updates.rst | 51 ++++++++++++++++++- 3 files changed, 88 insertions(+), 8 deletions(-) diff --git a/cinder/db/sqlalchemy/api.py b/cinder/db/sqlalchemy/api.py index aa925842098..2d39eac4f1f 100644 --- a/cinder/db/sqlalchemy/api.py +++ b/cinder/db/sqlalchemy/api.py @@ -4232,13 +4232,23 @@ def cg_has_volumes_filter(attached_or_with_snapshots=False): def cg_creating_from_src(cg_id=None, cgsnapshot_id=None): - model = aliased(models.ConsistencyGroup) - conditions = [~model.deleted, model.status == 'creating'] + # NOTE(geguileo): As explained in devref api_conditional_updates we use a + # subquery to trick MySQL into using the same table in the update and the + # where clause. + subq = sql.select([models.ConsistencyGroup]).where( + and_(~models.ConsistencyGroup.deleted, + models.ConsistencyGroup.status == 'creating')).alias('cg2') + if cg_id: - conditions.append(model.source_cgid == cg_id) - if cgsnapshot_id: - conditions.append(model.cgsnapshot_id == cgsnapshot_id) - return sql.exists().where(and_(*conditions)) + match_id = subq.c.source_cgid == cg_id + elif cgsnapshot_id: + match_id = subq.c.cgsnapshot_id == cgsnapshot_id + else: + msg = _('cg_creating_from_src must be called with cg_id or ' + 'cgsnapshot_id parameter.') + raise exception.ProgrammingError(reason=msg) + + return sql.exists([subq]).where(match_id) ############################### diff --git a/cinder/tests/unit/api/contrib/test_consistencygroups.py b/cinder/tests/unit/api/contrib/test_consistencygroups.py index 69ca24e5038..6d5621b2193 100644 --- a/cinder/tests/unit/api/contrib/test_consistencygroups.py +++ b/cinder/tests/unit/api/contrib/test_consistencygroups.py @@ -466,6 +466,29 @@ class ConsistencyGroupsAPITestCase(test.TestCase): consistencygroup.destroy() cg2.destroy() + def test_delete_consistencygroup_available_used_as_source_success(self): + consistencygroup = self._create_consistencygroup( + status=fields.ConsistencyGroupStatus.AVAILABLE) + req = webob.Request.blank('/v2/%s/consistencygroups/%s/delete' % + (fake.PROJECT_ID, consistencygroup.id)) + # The other CG used the first CG as source, but it's no longer in + # creating status, so we should be able to delete it. + cg2 = self._create_consistencygroup( + status=fields.ConsistencyGroupStatus.AVAILABLE, + source_cgid=consistencygroup.id) + req.method = 'POST' + req.headers['Content-Type'] = 'application/json' + req.body = jsonutils.dump_as_bytes({}) + res = req.get_response(fakes.wsgi_app()) + + consistencygroup = objects.ConsistencyGroup.get_by_id( + self.ctxt, consistencygroup.id) + self.assertEqual(202, res.status_int) + self.assertEqual('deleting', consistencygroup.status) + + consistencygroup.destroy() + cg2.destroy() + def test_delete_consistencygroup_available_no_force(self): consistencygroup = self._create_consistencygroup(status='available') req = webob.Request.blank('/v2/%s/consistencygroups/%s/delete' % diff --git a/doc/source/devref/api_conditional_updates.rst b/doc/source/devref/api_conditional_updates.rst index c5a0d9db617..b77afd2bcd5 100644 --- a/doc/source/devref/api_conditional_updates.rst +++ b/doc/source/devref/api_conditional_updates.rst @@ -347,12 +347,59 @@ Limitations ----------- We can only use functionality that works on **all** supported DBs, and that's -why we don't allow multi table updates and will raise DBError exception even -when the code is running against a DB engine that supports this functionality. +why we don't allow multi table updates and will raise ProgrammingError +exception even when the code is running against a DB engine that supports this +functionality. This way we make sure that we don't inadvertently add a multi table update that works on MySQL but will surely fail on PostgreSQL. +MySQL DB engine also has some limitations that we should be aware of when +creating our filters. + +One that is very common is when we are trying to check if there is a row that +matches a specific criteria in the same table that we are updating. For +example, when deleting a Consistency Group we want to check that it is not +being used as the source for a Consistency Group that is in the process of +being created. + +The straightforward way of doing this is using the core exists expression and +use an alias to differentiate general query fields and the exists subquery. +Code would look like this: + +.. code:: python + + def cg_creating_from_src(cg_id): + model = aliased(models.ConsistencyGroup) + return sql.exists().where(and_( + ~model.deleted, + model.status == 'creating', + conditions.append(model.source_cgid == cg_id)) + +While this will work in SQLite and PostgreSQL, it will not work on MySQL and an +error will be raised when the query is executed: "You can't specify target +table 'consistencygroups' for update in FROM clause". + +To solve this we have 2 options: + +- Create a specific query for MySQL using a feature only available in MySQL, + which is an update with a left self join. +- Use a trick -using a select subquery- that will work on all DBs. + +Considering that it's always better to have only 1 way of doing things and that +SQLAlchemy doesn't support MySQL's non standard behavior we should generate +these filters using the select subquery method like this: + +.. code:: python + + def cg_creating_from_src(cg_id): + subq = sql.select([models.ConsistencyGroup]).where(and_( + ~model.deleted, + model.status == 'creating')).alias('cg2') + + return sql.exists([subq]).where(subq.c.source_cgid == cgid) + + Considerations for new ORM & Versioned Objects ----------------------------------------------