Skip to content

Commit 75e4c86

Browse files
committed
database: Archive parent and child rows "trees" one at a time
Previously, we archived deleted rows in batches of max_rows parents + their child rows in a single database transaction. Doing it that way limited how high a value of max_rows could be specified by the caller because of the size of the database transaction it could generate. For example, in a large scale deployment with hundreds of thousands of deleted rows and constant server creation and deletion activity, a value of max_rows=1000 might exceed the database's configured maximum packet size or timeout due to a database deadlock, forcing the operator to use a much lower max_rows value like 100 or 50. And when the operator has e.g. 500,000 deleted instances rows (and millions of deleted rows total) they are trying to archive, being forced to use a max_rows value several orders of magnitude lower than the number of rows they need to archive was a poor user experience. This changes the logic to archive one parent row and its foreign key related child rows one at a time in a single database transaction while limiting the total number of rows per table as soon as it reaches >= max_rows. Doing this will allow operators to choose more predictable values for max_rows and get more progress per invocation of archive_deleted_rows. Closes-Bug: #2024258 Change-Id: I2209bf1b3320901cf603ec39163cf923b25b0359 (cherry picked from commit 697fa3c)
1 parent 8823da8 commit 75e4c86

File tree

5 files changed

+203
-48
lines changed

5 files changed

+203
-48
lines changed

nova/cmd/manage.py

Lines changed: 25 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -229,10 +229,10 @@ def version(self):
229229
print(migration.db_version())
230230

231231
@args('--max_rows', type=int, metavar='<number>', dest='max_rows',
232-
help='Maximum number of deleted rows to archive. Defaults to 1000. '
233-
'Note that this number does not include the corresponding '
234-
'rows, if any, that are removed from the API database for '
235-
'deleted instances.')
232+
help='Maximum number of deleted rows to archive per table. Defaults '
233+
'to 1000. Note that this number is a soft limit and does not '
234+
'include the corresponding rows, if any, that are removed '
235+
'from the API database for deleted instances.')
236236
@args('--before', metavar='<date>',
237237
help=('Archive rows that have been deleted before this date. '
238238
'Accepts date strings in the default format output by the '
@@ -404,7 +404,10 @@ def _do_archive(
404404
'cell1.instances': 5}
405405
:param cctxt: Cell-targeted nova.context.RequestContext if archiving
406406
across all cells
407-
:param max_rows: Maximum number of deleted rows to archive
407+
:param max_rows: Maximum number of deleted rows to archive per table.
408+
Note that this number is a soft limit and does not include the
409+
corresponding rows, if any, that are removed from the API database
410+
for deleted instances.
408411
:param until_complete: Whether to run continuously until all deleted
409412
rows are archived
410413
:param verbose: Whether to print how many rows were archived per table
@@ -417,15 +420,26 @@ def _do_archive(
417420
"""
418421
ctxt = context.get_admin_context()
419422
while True:
420-
run, deleted_instance_uuids, total_rows_archived = \
423+
# table_to_rows = {table_name: number_of_rows_archived}
424+
# deleted_instance_uuids = ['uuid1', 'uuid2', ...]
425+
table_to_rows, deleted_instance_uuids, total_rows_archived = \
421426
db.archive_deleted_rows(
422427
cctxt, max_rows, before=before_date, task_log=task_log)
423-
for table_name, rows_archived in run.items():
428+
429+
for table_name, rows_archived in table_to_rows.items():
424430
if cell_name:
425431
table_name = cell_name + '.' + table_name
426432
table_to_rows_archived.setdefault(table_name, 0)
427433
table_to_rows_archived[table_name] += rows_archived
428-
if deleted_instance_uuids:
434+
435+
# deleted_instance_uuids does not necessarily mean that any
436+
# instances rows were archived because it is obtained by a query
437+
# separate from the archive queries. For example, if a
438+
# DBReferenceError was raised while processing the instances table,
439+
# we would have skipped the table and had 0 rows archived even
440+
# though deleted instances rows were found.
441+
instances_archived = table_to_rows.get('instances', 0)
442+
if deleted_instance_uuids and instances_archived:
429443
table_to_rows_archived.setdefault(
430444
'API_DB.instance_mappings', 0)
431445
table_to_rows_archived.setdefault(
@@ -448,8 +462,9 @@ def _do_archive(
448462

449463
# If we're not archiving until there is nothing more to archive, we
450464
# have reached max_rows in this cell DB or there was nothing to
451-
# archive.
452-
if not until_complete or not run:
465+
# archive. We check the values() in case we get something like
466+
# table_to_rows = {'instances': 0} back somehow.
467+
if not until_complete or not any(table_to_rows.values()):
453468
break
454469
if verbose:
455470
sys.stdout.write('.')

nova/db/main/api.py

Lines changed: 93 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -4407,6 +4407,9 @@ def _archive_deleted_rows_for_table(
44074407
select = select.order_by(column).limit(max_rows)
44084408
with conn.begin():
44094409
rows = conn.execute(select).fetchall()
4410+
4411+
# This is a list of IDs of rows that should be archived from this table,
4412+
# limited to a length of max_rows.
44104413
records = [r[0] for r in rows]
44114414

44124415
# We will archive deleted rows for this table and also generate insert and
@@ -4419,51 +4422,103 @@ def _archive_deleted_rows_for_table(
44194422

44204423
# Keep track of any extra tablenames to number of rows that we archive by
44214424
# following FK relationships.
4422-
# {tablename: extra_rows_archived}
4425+
#
4426+
# extras = {tablename: number_of_extra_rows_archived}
44234427
extras = collections.defaultdict(int)
4424-
if records:
4425-
insert = shadow_table.insert().from_select(
4426-
columns, sql.select(table).where(column.in_(records))
4427-
).inline()
4428-
delete = table.delete().where(column.in_(records))
4428+
4429+
if not records:
4430+
# Nothing to archive, so return.
4431+
return rows_archived, deleted_instance_uuids, extras
4432+
4433+
# Keep track of how many rows we accumulate for the insert+delete database
4434+
# transaction and cap it as soon as it is >= max_rows. Because we will
4435+
# archive all child rows of a parent row along with the parent at the same
4436+
# time, we end up with extra rows to archive in addition to len(records).
4437+
num_rows_in_batch = 0
4438+
# The sequence of query statements we will execute in a batch. These are
4439+
# ordered: [child1, child1, parent1, child2, child2, child2, parent2, ...]
4440+
# Parent + child "trees" are kept together to avoid FK constraint
4441+
# violations.
4442+
statements_in_batch = []
4443+
# The list of records in the batch. This is used for collecting deleted
4444+
# instance UUIDs in the case of the 'instances' table.
4445+
records_in_batch = []
4446+
4447+
# (melwitt): We will gather rows related by foreign key relationship for
4448+
# each deleted row, one at a time. We do it this way to keep track of and
4449+
# limit the total number of rows that will be archived in a single database
4450+
# transaction. In a large scale database with potentially hundreds of
4451+
# thousands of deleted rows, if we don't limit the size of the transaction
4452+
# based on max_rows, we can get into a situation where we get stuck not
4453+
# able to make much progress. The value of max_rows has to be 1) small
4454+
# enough to not exceed the database's max packet size limit or timeout with
4455+
# a deadlock but 2) large enough to make progress in an environment with a
4456+
# constant high volume of create and delete traffic. By archiving each
4457+
# parent + child rows tree one at a time, we can ensure meaningful progress
4458+
# can be made while allowing the caller to predictably control the size of
4459+
# the database transaction with max_rows.
4460+
for record in records:
44294461
# Walk FK relationships and add insert/delete statements for rows that
44304462
# refer to this table via FK constraints. fk_inserts and fk_deletes
44314463
# will be prepended to by _get_fk_stmts if referring rows are found by
44324464
# FK constraints.
44334465
fk_inserts, fk_deletes = _get_fk_stmts(
4434-
metadata, conn, table, column, records)
4435-
4436-
# NOTE(tssurya): In order to facilitate the deletion of records from
4437-
# instance_mappings, request_specs and instance_group_member tables in
4438-
# the nova_api DB, the rows of deleted instances from the instances
4439-
# table are stored prior to their deletion. Basically the uuids of the
4440-
# archived instances are queried and returned.
4441-
if tablename == "instances":
4442-
query_select = sql.select(table.c.uuid).where(
4443-
table.c.id.in_(records)
4444-
)
4445-
with conn.begin():
4446-
rows = conn.execute(query_select).fetchall()
4447-
deleted_instance_uuids = [r[0] for r in rows]
4466+
metadata, conn, table, column, [record])
4467+
statements_in_batch.extend(fk_inserts + fk_deletes)
4468+
# statement to add parent row to shadow table
4469+
insert = shadow_table.insert().from_select(
4470+
columns, sql.select(table).where(column.in_([record]))).inline()
4471+
statements_in_batch.append(insert)
4472+
# statement to remove parent row from main table
4473+
delete = table.delete().where(column.in_([record]))
4474+
statements_in_batch.append(delete)
44484475

4449-
try:
4450-
# Group the insert and delete in a transaction.
4451-
with conn.begin():
4452-
for fk_insert in fk_inserts:
4453-
conn.execute(fk_insert)
4454-
for fk_delete in fk_deletes:
4455-
result_fk_delete = conn.execute(fk_delete)
4456-
extras[fk_delete.table.name] += result_fk_delete.rowcount
4457-
conn.execute(insert)
4458-
result_delete = conn.execute(delete)
4459-
rows_archived += result_delete.rowcount
4460-
except db_exc.DBReferenceError as ex:
4461-
# A foreign key constraint keeps us from deleting some of
4462-
# these rows until we clean up a dependent table. Just
4463-
# skip this table for now; we'll come back to it later.
4464-
LOG.warning("IntegrityError detected when archiving table "
4465-
"%(tablename)s: %(error)s",
4466-
{'tablename': tablename, 'error': str(ex)})
4476+
records_in_batch.append(record)
4477+
4478+
# Check whether were have a full batch >= max_rows. Rows are counted as
4479+
# the number of rows that will be moved in the database transaction.
4480+
# So each insert+delete pair represents one row that will be moved.
4481+
# 1 parent + its fks
4482+
num_rows_in_batch += 1 + len(fk_inserts)
4483+
4484+
if max_rows is not None and num_rows_in_batch >= max_rows:
4485+
break
4486+
4487+
# NOTE(tssurya): In order to facilitate the deletion of records from
4488+
# instance_mappings, request_specs and instance_group_member tables in the
4489+
# nova_api DB, the rows of deleted instances from the instances table are
4490+
# stored prior to their deletion. Basically the uuids of the archived
4491+
# instances are queried and returned.
4492+
if tablename == "instances":
4493+
query_select = sql.select(table.c.uuid).where(
4494+
table.c.id.in_(records_in_batch))
4495+
with conn.begin():
4496+
rows = conn.execute(query_select).fetchall()
4497+
# deleted_instance_uuids = ['uuid1', 'uuid2', ...]
4498+
deleted_instance_uuids = [r[0] for r in rows]
4499+
4500+
try:
4501+
# Group the insert and delete in a transaction.
4502+
with conn.begin():
4503+
for statement in statements_in_batch:
4504+
result = conn.execute(statement)
4505+
result_tablename = statement.table.name
4506+
# Add to archived row counts if not a shadow table.
4507+
if not result_tablename.startswith(_SHADOW_TABLE_PREFIX):
4508+
if result_tablename == tablename:
4509+
# Number of tablename (parent) rows archived.
4510+
rows_archived += result.rowcount
4511+
else:
4512+
# Number(s) of child rows archived.
4513+
extras[result_tablename] += result.rowcount
4514+
4515+
except db_exc.DBReferenceError as ex:
4516+
# A foreign key constraint keeps us from deleting some of these rows
4517+
# until we clean up a dependent table. Just skip this table for now;
4518+
# we'll come back to it later.
4519+
LOG.warning("IntegrityError detected when archiving table "
4520+
"%(tablename)s: %(error)s",
4521+
{'tablename': tablename, 'error': str(ex)})
44674522

44684523
conn.close()
44694524

nova/tests/functional/db/test_archive.py

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -174,6 +174,50 @@ def test_archive_deleted_rows_incomplete(self):
174174
break
175175
self.assertFalse(exceptions)
176176

177+
def test_archive_deleted_rows_parent_child_trees_one_at_time(self):
178+
"""Test that we are archiving parent + child rows "trees" one at a time
179+
180+
Previously, we archived deleted rows in batches of max_rows parents +
181+
their child rows in a single database transaction. Doing it that way
182+
limited how high a value of max_rows could be specified by the caller
183+
because of the size of the database transaction it could generate.
184+
185+
For example, in a large scale deployment with hundreds of thousands of
186+
deleted rows and constant server creation and deletion activity, a
187+
value of max_rows=1000 might exceed the database's configured maximum
188+
packet size or timeout due to a database deadlock, forcing the operator
189+
to use a much lower max_rows value like 100 or 50.
190+
191+
And when the operator has e.g. 500,000 deleted instances rows (and
192+
millions of deleted rows total) they are trying to archive, being
193+
forced to use a max_rows value serveral orders of magnitude lower than
194+
the number of rows they need to archive was a poor user experience.
195+
196+
This tests that we are archiving each parent plus their child rows one
197+
at a time while limiting the total number of rows per table in a batch
198+
as soon as the total number of archived rows is >= max_rows.
199+
"""
200+
# Boot two servers and delete them, then try to archive rows.
201+
for i in range(2):
202+
server = self._create_server()
203+
self._delete_server(server)
204+
205+
# Use max_rows=5 to limit the archive to one complete parent +
206+
# child rows tree.
207+
table_to_rows, _, _ = db.archive_deleted_rows(max_rows=5)
208+
209+
# We should have archived only one instance because the parent +
210+
# child tree will have >= max_rows of 5.
211+
self.assertEqual(1, table_to_rows['instances'])
212+
213+
# Archive once more to archive the other instance (and its child rows).
214+
table_to_rows, _, _ = db.archive_deleted_rows(max_rows=5)
215+
self.assertEqual(1, table_to_rows['instances'])
216+
217+
# There should be nothing else to archive.
218+
_, _, total_rows_archived = db.archive_deleted_rows(max_rows=100)
219+
self.assertEqual(0, total_rows_archived)
220+
177221
def _get_table_counts(self):
178222
engine = db.get_engine()
179223
conn = engine.connect()

nova/tests/unit/cmd/test_manage.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -501,6 +501,26 @@ def test_archive_deleted_rows_verbose_no_results(self, mock_get_all,
501501
self.assertIn('Nothing was archived.', output)
502502
self.assertEqual(0, result)
503503

504+
@mock.patch.object(db, 'archive_deleted_rows')
505+
@mock.patch.object(objects.CellMappingList, 'get_all')
506+
def test_archive_deleted_rows_deleted_instances_no_rows(self, mock_get_all,
507+
mock_db_archive):
508+
# Simulate a scenario where we didn't archive any rows (example:
509+
# DBReferenceError was raised while processing the instances table) but
510+
# the separate query for deleted_instance_uuids found rows.
511+
mock_db_archive.return_value = (
512+
{}, [uuidsentinel.instance1, uuidsentinel.instance2], 0)
513+
514+
result = self.commands.archive_deleted_rows(20, verbose=True)
515+
516+
mock_db_archive.assert_called_once_with(
517+
test.MatchType(context.RequestContext), 20, before=None,
518+
task_log=False)
519+
output = self.output.getvalue()
520+
# If nothing was archived, there should be no purge messages
521+
self.assertIn('Nothing was archived.', output)
522+
self.assertEqual(0, result)
523+
504524
@mock.patch.object(db, 'archive_deleted_rows')
505525
@mock.patch.object(objects.RequestSpec, 'destroy_bulk')
506526
@mock.patch.object(objects.InstanceGroup, 'destroy_members_bulk')
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
---
2+
fixes:
3+
- |
4+
`Bug #2024258`_: Fixes an issue with performance degradation archiving
5+
databases with large numbers of foreign key related records.
6+
7+
Previously, deleted rows were archived in batches of max_rows parents +
8+
their child rows in a single database transaction. It limited how high
9+
a value of max_rows could be specified by the user because of the size of
10+
the database transaction it could generate. Symptoms of the behavior were
11+
exceeding the maximum configured packet size of the database or timing out
12+
due to a deadlock.
13+
14+
The behavior has been changed to archive batches of complete parent +
15+
child rows trees while limiting each batch when it has reached >= max_rows
16+
records. This allows the size of the database transaction to be controlled
17+
by the user and enables more rows to be archived per invocation of
18+
``nova-manage db archive_deleted_rows`` when there are a large number of
19+
foreign key related records.
20+
21+
.. _Bug #2024258: https://bugs.launchpad.net/nova/+bug/2024258

0 commit comments

Comments
 (0)