From 79c602507c93fab04fc77fcc4ec329e6527331ad Mon Sep 17 00:00:00 2001 From: Abhishek Kekane Date: Mon, 8 Feb 2016 05:33:18 -0800 Subject: [PATCH] glance-manage db purge failure for limit MySQL does not support LIMIT in subqueries for certain subquery operators. Ref :- https://dev.mysql.com/doc/mysql-reslimits-excerpt/5.7/en/subquery-restrictions.html Using nested select statements to resolve this issue. NOTE: Added order by clause on 'deleted_at' column so that old records will be deleted first. Closes-bug: #1543092 Change-Id: Id90a6686bed60dea74644057c3929643a4c1d201 --- glance/db/sqlalchemy/api.py | 43 ++++++++++++++++++------ glance/tests/unit/test_glance_manage.py | 44 +++++++++++++++++++++++++ 2 files changed, 77 insertions(+), 10 deletions(-) create mode 100644 glance/tests/unit/test_glance_manage.py diff --git a/glance/db/sqlalchemy/api.py b/glance/db/sqlalchemy/api.py index f0ff8db816..d4e9c433fc 100644 --- a/glance/db/sqlalchemy/api.py +++ b/glance/db/sqlalchemy/api.py @@ -34,8 +34,10 @@ import six # NOTE(jokke): simplified transition to py3, behaves like py2 xrange from six.moves import range import sqlalchemy -from sqlalchemy import MetaData, Table, select +from sqlalchemy.ext.compiler import compiles +from sqlalchemy import MetaData, Table import sqlalchemy.orm as sa_orm +from sqlalchemy import sql import sqlalchemy.sql as sa_sql from glance.common import exception @@ -1247,6 +1249,24 @@ def image_tag_get_all(context, image_id, session=None): return [tag[0] for tag in tags] +class DeleteFromSelect(sa_sql.expression.UpdateBase): + def __init__(self, table, select, column): + self.table = table + self.select = select + self.column = column + + +# NOTE(abhishekk): MySQL doesn't yet support subquery with +# 'LIMIT & IN/ALL/ANY/SOME' We need work around this with nesting select. +@compiles(DeleteFromSelect) +def visit_delete_from_select(element, compiler, **kw): + return "DELETE FROM %s WHERE %s in (SELECT T1.%s FROM (%s) as T1)" % ( + compiler.process(element.table, asfrom=True), + compiler.process(element.column), + element.column.name, + compiler.process(element.select)) + + def purge_deleted_rows(context, age_in_days, max_rows, session=None): """Purges soft deleted rows @@ -1294,16 +1314,19 @@ def purge_deleted_rows(context, age_in_days, max_rows, session=None): _LI('Purging deleted rows older than %(age_in_days)d day(s) ' 'from table %(tbl)s'), {'age_in_days': age_in_days, 'tbl': tbl}) + + column = tab.c.id + deleted_at_column = tab.c.deleted_at + + query_delete = sql.select( + [column], deleted_at_column < deleted_age).order_by( + deleted_at_column).limit(max_rows) + + delete_statement = DeleteFromSelect(tab, query_delete, column) + with session.begin(): - result = session.execute( - tab.delete().where( - tab.columns.id.in_( - select([tab.columns.id]).where( - tab.columns.deleted_at < deleted_age - ).limit(max_rows) - ) - ) - ) + result = session.execute(delete_statement) + rows = result.rowcount LOG.info(_LI('Deleted %(rows)d row(s) from table %(tbl)s'), {'rows': rows, 'tbl': tbl}) diff --git a/glance/tests/unit/test_glance_manage.py b/glance/tests/unit/test_glance_manage.py new file mode 100644 index 0000000000..d7a5c168d9 --- /dev/null +++ b/glance/tests/unit/test_glance_manage.py @@ -0,0 +1,44 @@ +# Copyright 2016 OpenStack Foundation. +# Copyright 2016 NTT Data. +# All Rights Reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); you may +# not use this file except in compliance with the License. You may obtain +# a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +# License for the specific language governing permissions and limitations +# under the License. + +import mock + +from glance.cmd import manage +from glance import context +from glance.db.sqlalchemy import api as db_api +import glance.tests.utils as test_utils + +TENANT1 = '6838eb7b-6ded-434a-882c-b344c77fe8df' +USER1 = '54492ba0-f4df-4e4e-be62-27f4d76b29cf' + + +class DBCommandsTestCase(test_utils.BaseTestCase): + def setUp(self): + super(DBCommandsTestCase, self).setUp() + self.commands = manage.DbCommands() + self.context = context.RequestContext( + user=USER1, tenant=TENANT1) + + @mock.patch.object(db_api, 'purge_deleted_rows') + @mock.patch.object(context, 'get_admin_context') + def test_purge_command(self, mock_context, mock_db_purge): + mock_context.return_value = self.context + self.commands.purge(1, 100) + mock_db_purge.assert_called_once_with(self.context, 1, 100) + + def test_purge_command_negative_rows(self): + exit = self.assertRaises(SystemExit, self.commands.purge, 1, -1) + self.assertEqual("Minimal rows limit is 1.", exit.code)