Improve resource listing efficiency

Cinder's resource tables (volumes, snapshots, backups, groups,
group_snapshots) don't have required indexes to do efficient resource
listings on the database engine.

This forces the database to go through all existing database records for
any listing (even when there are no additional user requested filtering)
and check one by one the conditions, resulting in high CPU load on the
database servers.

As an example a listing for a project with a single volume:

$ cinder list
+--------------------------------------+-----------+------+------+-------------+----------+-------------+
| ID                                   | Status    | Name | Size | Volume Type | Bootable | Attached to |
+--------------------------------------+-----------+------+------+-------------+----------+-------------+
| 8a6b11d5-3343-4c0d-8a64-8e7070d1988e | available | test | 1    | lvmdriver-1 | false    |             |
+--------------------------------------+-----------+------+------+-------------+----------+-------------+

May result in the database going through thousand of records (all
deleted records and all records for other projects), as demonstrated by
the following SQL queries where 10435 rows existed in the database and
had to be checked just to return a single one.

This is the SQL equivalent of the earlier cinder list command:

$ mysql cinder -e 'select id, display_name from volumes where not deleted and project_id="a41464e54125407aab09e0236cce2c3c"'
+--------------------------------------+--------------+
| id                                   | display_name |
+--------------------------------------+--------------+
| 8a6b11d5-3343-4c0d-8a64-8e7070d1988e | test         |
+--------------------------------------+--------------+

Which if we look at the numbers of rows that it hits with `explain` we
can see it hits every single row:

$ mysql cinder -e 'explain select id, display_name from volumes where not deleted and project_id="a41464e54125407aab09e0236cce2c3c"'
+------+-------------+---------+------+---------------+------+---------+------+-------+-------------+
| id   | select_type | table   | type | possible_keys | key  | key_len | ref  | rows  | Extra       |
+------+-------------+---------+------+---------------+------+---------+------+-------+-------------+
|    1 | SIMPLE      | volumes | ALL  | NULL          | NULL | NULL    | NULL | 10435 | Using where |
+------+-------------+---------+------+---------------+------+---------+------+-------+-------------+

This patch introduces a deleted and project_id index for the volumes,
snapshots, groups, group_snapshots, and backups tables, which will allow
the database to do efficient retrieval of records for listings.

The reason why we order first by deleted and then by project_id is
because when an admin does a listing with `--all-tenants` that query
will be able to use the deleted table of the new compound index.

We can see the new index this patch adds and how it allows the DB engine
to efficiently retrieve non deleted volumes from the specific project.

$ mysql cinder -e 'show index from volumes'
+---------+------------+--------------------------------+--------------+---------------------+-----------+-------------+----------+--------+------+------------+---------+---------------+
| Table   | Non_unique | Key_name                       | Seq_in_index | Column_name         | Collation | Cardinality | Sub_part | Packed | Null | Index_type | Comment | Index_comment |
+---------+------------+--------------------------------+--------------+---------------------+-----------+-------------+----------+--------+------+------------+---------+---------------+
| volumes |          0 | PRIMARY                        |            1 | id                  | A         |           1 |     NULL | NULL   |      | BTREE      |         |               |
| volumes |          1 | volumes_service_uuid_idx       |            1 | service_uuid        | A         |           1 |     NULL | NULL   | YES  | BTREE      |         |               |
| volumes |          1 | volumes_service_uuid_idx       |            2 | deleted             | A         |           1 |     NULL | NULL   | YES  | BTREE      |         |               |
| volumes |          1 | ix_volumes_consistencygroup_id |            1 | consistencygroup_id | A         |           1 |     NULL | NULL   | YES  | BTREE      |         |               |
| volumes |          1 | ix_volumes_group_id            |            1 | group_id            | A         |           1 |     NULL | NULL   | YES  | BTREE      |         |               |
| volumes |          1 | volumes_deleted_project_id_idx |            1 | deleted             | A         |           1 |     NULL | NULL   | YES  | BTREE      |         |               |
| volumes |          1 | volumes_deleted_project_id_idx |            2 | project_id          | A         |           1 |     NULL | NULL   | YES  | BTREE      |         |               |
| volumes |          1 | volumes_deleted_host_idx       |            1 | deleted             | A         |           1 |     NULL | NULL   | YES  | BTREE      |         |               |
| volumes |          1 | volumes_deleted_host_idx       |            2 | host                | A         |           1 |     NULL | NULL   | YES  | BTREE      |         |               |
+---------+------------+--------------------------------+--------------+---------------------+-----------+-------------+----------+--------+------+------------+---------+---------------+

$ mysql cinder -e 'explain select id, display_name from volumes where not deleted and project_id="a41464e54125407aab09e0236cce2c3c"'
+------+-------------+---------+------+--------------------------------+--------------------------------+---------+-------------+------+-----------------------+
| id   | select_type | table   | type | possible_keys                  | key                            | key_len | ref         | rows | Extra                 |
+------+-------------+---------+------+--------------------------------+--------------------------------+---------+-------------+------+-----------------------+
|    1 | SIMPLE      | volumes | ref  | volumes_deleted_project_id_idx | volumes_deleted_project_id_idx | 770     | const,const |    1 | Using index condition |
+------+-------------+---------+------+--------------------------------+--------------------------------+---------+-------------+------+-----------------------+

We also add another missing index for the volumes that is used by the
create volume from image.

The patch also updates 3 tests that were expecting the result from a
query to be in a specific order when there is no actual ORDER BY in the
query.

Closes-Bug: #1952443
Change-Id: I8456a9f82bdf18ada76874dc0c4f59542e1c03ab
This commit is contained in:
Gorka Eguileor 2021-11-26 10:38:47 +01:00 committed by Rajat Dhasmana
parent 7c79e2115a
commit bbe42df26c
7 changed files with 168 additions and 12 deletions

View File

@ -0,0 +1,60 @@
# 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.
"""Add resource indexes
Revision ID: daa98075b90d
Revises: 9c74c1c6971f
Create Date: 2021-11-26 10:26:41.883072
"""
from alembic import op
from oslo_db.sqlalchemy import enginefacade
from oslo_db.sqlalchemy import utils
from oslo_log import log as logging
LOG = logging.getLogger(__name__)
# revision identifiers, used by Alembic.
revision = 'daa98075b90d'
down_revision = 'c92a3e68beed'
branch_labels = None
depends_on = None
INDEXES = (
('groups', 'groups_deleted_project_id_idx', ('deleted', 'project_id')),
('group_snapshots', 'group_snapshots_deleted_project_id_idx',
('deleted', 'project_id')),
('volumes', 'volumes_deleted_project_id_idx', ('deleted', 'project_id')),
('volumes', 'volumes_deleted_host_idx', ('deleted', 'host')),
('backups', 'backups_deleted_project_id_idx', ('deleted', 'project_id')),
('snapshots', 'snapshots_deleted_project_id_idx', ('deleted',
'project_id')),
)
def upgrade():
engine = enginefacade.reader.get_engine()
is_mysql = engine.dialect.name == 'mysql'
for table, idx_name, fields in INDEXES:
# Skip creation in mysql if it already has the index
if is_mysql and utils.index_exists(engine, table, idx_name):
LOG.info('Skipping index %s, already exists', idx_name)
else:
op.create_index(idx_name, table, fields)

View File

@ -218,6 +218,11 @@ class Group(BASE, CinderBase):
"""Represents a generic volume group."""
__tablename__ = 'groups'
__table_args__ = (
# Speed up normal listings
sa.Index('groups_deleted_project_id_idx', 'deleted', 'project_id'),
CinderBase.__table_args__,
)
id = sa.Column(sa.String(36), primary_key=True)
@ -272,6 +277,12 @@ class GroupSnapshot(BASE, CinderBase):
"""Represents a group snapshot."""
__tablename__ = 'group_snapshots'
__table_args__ = (
# Speed up normal listings
sa.Index('group_snapshots_deleted_project_id_idx',
'deleted', 'project_id'),
CinderBase.__table_args__,
)
id = sa.Column(sa.String(36), primary_key=True)
@ -304,6 +315,11 @@ class Volume(BASE, CinderBase):
__tablename__ = 'volumes'
__table_args__ = (
sa.Index('volumes_service_uuid_idx', 'deleted', 'service_uuid'),
# Speed up normal listings
sa.Index('volumes_deleted_project_id_idx', 'deleted', 'project_id'),
# Speed up service start, create volume from image when using direct
# urls, host REST API, and the cinder-manage update host cmd
sa.Index('volumes_deleted_host_idx', 'deleted', 'host'),
CinderBase.__table_args__,
)
@ -894,6 +910,11 @@ class Snapshot(BASE, CinderBase):
"""Represents a snapshot of volume."""
__tablename__ = 'snapshots'
__table_args__ = (
# Speed up normal listings
sa.Index('snapshots_deleted_project_id_idx', 'deleted', 'project_id'),
CinderBase.__table_args__,
)
id = sa.Column(sa.String(36), primary_key=True)
# TODO: (Y release) Change nullable to False
@ -996,6 +1017,11 @@ class Backup(BASE, CinderBase):
"""Represents a backup of a volume to Swift."""
__tablename__ = 'backups'
__table_args__ = (
# Speed up normal listings
sa.Index('backups_deleted_project_id_idx', 'deleted', 'project_id'),
CinderBase.__table_args__,
)
id = sa.Column(sa.String(36), primary_key=True)
# Backups don't have use_quota field since we don't have temporary backups

View File

@ -216,6 +216,17 @@ class MigrationsWalk(
# But it's nullable
self.assertTrue(table.c.shared_targets.nullable)
def _check_daa98075b90d(self, connection):
"""Test resources have indexes."""
for table in ('groups', 'group_snapshots', 'volumes', 'snapshots',
'backups'):
db_utils.index_exists(connection,
table,
f'{table}_deleted_project_id_idx')
db_utils.index_exists(connection,
'volumes', 'volumes_deleted_host_idx')
class TestMigrationsWalkSQLite(
MigrationsWalk,

View File

@ -187,9 +187,8 @@ class GetActiveByWindowTestCase(base.BaseVolumeTestCase):
datetime.datetime(1, 4, 1, 1, 1, 1),
project_id=fake.PROJECT_ID)
self.assertEqual(3, len(volumes))
self.assertEqual(fake.VOLUME2_ID, volumes[0].id)
self.assertEqual(fake.VOLUME3_ID, volumes[1].id)
self.assertEqual(fake.VOLUME4_ID, volumes[2].id)
self.assertEqual({fake.VOLUME2_ID, fake.VOLUME3_ID, fake.VOLUME4_ID},
{v.id for v in volumes})
def test_snapshot_get_all_active_by_window(self):
# Find all all snapshots valid within a timeframe window.
@ -229,12 +228,11 @@ class GetActiveByWindowTestCase(base.BaseVolumeTestCase):
datetime.datetime(1, 3, 1, 1, 1, 1),
datetime.datetime(1, 4, 1, 1, 1, 1)).objects
self.assertEqual(3, len(snapshots))
self.assertEqual(snap2.id, snapshots[0].id)
self.assertEqual(fake.VOLUME_ID, snapshots[0].volume_id)
self.assertEqual(snap3.id, snapshots[1].id)
self.assertEqual(fake.VOLUME_ID, snapshots[1].volume_id)
self.assertEqual(snap4.id, snapshots[2].id)
self.assertEqual(fake.VOLUME_ID, snapshots[2].volume_id)
self.assertEqual({snap2.id, snap3.id, snap4.id},
{s.id for s in snapshots})
self.assertEqual({fake.VOLUME_ID},
{s.volume_id for s in snapshots})
def test_backup_get_all_active_by_window(self):
# Find all backups valid within a timeframe window.
@ -266,6 +264,5 @@ class GetActiveByWindowTestCase(base.BaseVolumeTestCase):
project_id=fake.PROJECT_ID
)
self.assertEqual(3, len(backups))
self.assertEqual(fake.BACKUP2_ID, backups[0].id)
self.assertEqual(fake.BACKUP3_ID, backups[1].id)
self.assertEqual(fake.BACKUP4_ID, backups[2].id)
self.assertEqual({fake.BACKUP2_ID, fake.BACKUP3_ID, fake.BACKUP4_ID},
{b.id for b in backups})

View File

@ -18,3 +18,4 @@ Storage installation.
ts-no-emulator-x86-64.rst
ts-non-existent-host.rst
ts-non-existent-vlun.rst
ts-db-cpu-spikes.rst

View File

@ -0,0 +1,37 @@
=====================================
Database CPU spikes during operations
=====================================
Query load upon the database can become a bottleneck that cascades across a
deployment and ultimately degrades not only the Cinder service but also the
whole OpenStack deployment.
Often, depending on load, query patterns, periodic tasks, and so on and so
forth, additional indexes may be needed to help provide hints to the database
so it can most efficently attempt to reduce the number of rows which need to
be examined in order to return a result set.
Adding indexes
--------------
In older releases, before 2023.1 (Antelope), there were some tables that
performed poorly in the presence of a large number of deleted resources
(volumes, snapshots, backups, etc) which resulted in high CPU loads on the DB
servers not only when listing those resources, but also when doing some
operations on them. This was resolved by adding appropriate indexes to them.
This example below is specific to MariaDB/MySQL, but the syntax should be easy
to modify for operators using PostgreSQL, and it represents the changes that
older releases could add to resolve these DB server CPU spikes in such a way
that they would not conflict with the ones that Cinder introduced in 2023.1
(Antelope).
.. code-block:: sql
use cinder;
create index groups_deleted_project_id_idx on groups (deleted, project_id);
create index group_snapshots_deleted_project_id_idx on groups (deleted, project_id);
create index volumes_deleted_project_id_idx on volumes (deleted, project_id);
create index volumes_deleted_host_idx on volumes (deleted, host);
create index snapshots_deleted_project_id_idx on snapshots (deleted, project_id);
create index backups_deleted_project_id_idx on backups (deleted, project_id);

View File

@ -0,0 +1,24 @@
---
upgrade:
- |
The ``cinder-manage db sync`` command for this verison of cinder will add
additional database indexes. Depending on database size and complexity,
this will take time to complete for every single index to be created. On
MySQL or MariaDB, these indexes will only be created if an index does not
already exist with the same name:
* ``groups_deleted_project_id_idx``
* ``group_snapshots_deleted_project_id_idx``
* ``volumes_deleted_project_id_idx``
* ``volumes_deleted_host_idx``
* ``snapshots_deleted_project_id_idx``
* ``backups_deleted_project_id_idx``
An example of the SQL commands to generate these indexes can be found
in the `specific troubleshooting guide
<htts://docs.openstack.org/cinder/latest/admin/ts-db-cpu-spikes.html>`_.
fixes:
- |
`Bug #1952443 <https://bugs.launchpad.net/cinder/+bug/1952443>`_: Improve
performance for creating volume from image, listing volumes, snapshots,
backups, groups, and group_snapshots.