Ironic index docs/command check backport
Secure RBAC, along with numerous periodics, and even some common API
access patterns heavily access the ironic database and sometimes cause
queries across multiple columns to match nodes to return.
This change is largely intended to provide the upgrade check
call as well as the documentation in the Wallaby release
of ironic so operators can locate it and add the indexes prior
to the upgrade to ironic 18.0.
Note: This change includes the content from
Ifa9da65dc8df5bf9a369d6faeab310386dfd944a which includes a fix for
the status command and the upgrade script as to properly identify and
raise visibility, where the original change to the status command
failed to include the actual value in the field to cause the
standard execution by the upgrade script. Also includes the same
upgrade check handling for ironic-status command in the stable branch
devstack plugin as this change does raise an informational warning
which is non-blocking to ironic's use on this branch to signal
that action may be advisable.
Story: 2008863
Task: 42392
Change-Id: I76821c032180a58d0f99d31110fbe0f844c0cb3f
(cherry picked from commit 205a8b3037
)
This commit is contained in:
parent
77be4c6c69
commit
e0d241fa4d
|
@ -1841,7 +1841,14 @@ function init_ironic {
|
|||
|
||||
# NOTE(rloo): We're not upgrading but want to make sure this command works,
|
||||
# even though we're not parsing the output of this command.
|
||||
$IRONIC_BIN_DIR/ironic-status upgrade check
|
||||
$IRONIC_BIN_DIR/ironic-status upgrade check && ret_val=$? || ret_val=$?
|
||||
if [ $ret_val -gt 1 ]; then
|
||||
# NOTE(TheJulia): The ironic-status check command can return
|
||||
# 0 if all is okay, 1 if there is a warning, or >1 if something
|
||||
# is broken on a major upgrade and thus the process needs to
|
||||
# be aborted.
|
||||
die $LINENO "Ironic db status check failed, returned: $ret_val"
|
||||
fi
|
||||
}
|
||||
|
||||
# _ironic_bm_vm_names() - Generates list of names for baremetal VMs.
|
||||
|
|
|
@ -74,7 +74,15 @@ upgrade_project ironic $RUN_DIR $BASE_DEVSTACK_BRANCH $TARGET_DEVSTACK_BRANCH
|
|||
# NOTE(rloo): make sure it is OK to do an upgrade. Except that we aren't
|
||||
# parsing/checking the output of this command because the output could change
|
||||
# based on the checks it makes.
|
||||
$IRONIC_BIN_DIR/ironic-status upgrade check
|
||||
$IRONIC_BIN_DIR/ironic-status upgrade check && ret_val=$? || ret_val=$?
|
||||
if [ $ret_val -gt 1 ] ; then
|
||||
# NOTE(TheJulia): We need to evaluate the return code from the
|
||||
# upgrade status check as the framework defines
|
||||
# Warnings are permissible and returned as status code 1, errors are
|
||||
# returned as greater than 1 which means there is a major upgrade
|
||||
# stopping issue which needs to be addressed.
|
||||
die $LINENO "Ironic DB Status check failed, returned: $ret_val"
|
||||
fi
|
||||
|
||||
$IRONIC_BIN_DIR/ironic-dbsync --config-file=$IRONIC_CONF_FILE
|
||||
|
||||
|
|
|
@ -113,6 +113,82 @@ perform ten concurrent deployments of images requiring conversion, the memory
|
|||
needed may exceed 10GB. This does however, entirely depend upon image block
|
||||
structure and layout, and what deploy interface is being used.
|
||||
|
||||
Database
|
||||
========
|
||||
|
||||
Query load upon the database is one of the biggest potential bottlenecks which
|
||||
can cascade across a deployment and ultimately degrade service to an Ironic
|
||||
user.
|
||||
|
||||
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
|
||||
--------------
|
||||
|
||||
This example below is specific to MariaDB/MySQL, but the syntax should be
|
||||
easy to modify for operators using PostgreSQL.
|
||||
|
||||
.. code-block:: sql
|
||||
|
||||
use ironic;
|
||||
create index owner_idx on nodes (owner) LOCK = SHARED;
|
||||
create index lessee_idx on nodes (lessee) LOCK = SHARED;
|
||||
create index driver_idx on nodes (driver) LOCK = SHARED;
|
||||
create index provision_state_idx on nodes (provision_state) LOCK = SHARED;
|
||||
create index reservation_idx on nodes (reservation) LOCK = SHARED;
|
||||
create index conductor_group_idx on nodes (conductor_group) LOCK = SHARED;
|
||||
create index resource_class_idx on nodes (resource_class) LOCK = SHARED;
|
||||
|
||||
.. note:: The indexes noted have been added automatically by Xena versions of
|
||||
Ironic and later. They are provided here as an example and operators can
|
||||
add them manually prior with versions of Ironic. The database upgrade for
|
||||
the Xena release of Ironic which adds these indexes are only aware of being
|
||||
able to skip index creation if it already exists on MySQL/MariaDB.
|
||||
|
||||
.. note:: It may be possible to use "LOCK = NONE". Basic testing indicates
|
||||
this takes a little bit longer, but shouldn't result in the database
|
||||
table becoming write locked during the index creation. If the database
|
||||
engine cannot support this, then the index creation will fail.
|
||||
|
||||
Database platforms also have a concept of what is called a "compound index"
|
||||
where the index is aligned with the exact query pattern being submitted to
|
||||
the database. The database is able to use this compound index to attempt to
|
||||
drastically reduce the result set generation time for the remainder of the
|
||||
query. As of the composition of this document, we do not ship compound
|
||||
indexes in Ironic as we feel the most general benefit is single column
|
||||
indexes, and depending on data present, an operator may wish to explore
|
||||
compound indexes with their database administrator, as comound indexes
|
||||
can also have negative performance impacts if improperly constructed.
|
||||
|
||||
.. code-block:: sql
|
||||
|
||||
use ironic;
|
||||
create index my_custom_app_query_index on nodes (reservation, provision_state, driver);
|
||||
|
||||
The risk, and *WHY* you should engage a Database Administrator, is depending on
|
||||
your configuration, the actual index may need to include one or more additional
|
||||
fields such as owner or lessee which may be added on to the index. At the same
|
||||
time, queries with less field matches, or in different orders will exhibit
|
||||
different performance as the compound index may not be able to be consulted.
|
||||
|
||||
Indexes will not fix everything
|
||||
-------------------------------
|
||||
|
||||
Indexes are not a magical cure-all for all API or database performance issues,
|
||||
but they are an increadibly important part depending on data access and query
|
||||
patterns.
|
||||
|
||||
The underlying object layer and data conversions including record pagination
|
||||
do add a substantial amount of overhead to what may otherwise return as a
|
||||
result set on a manual database query. In Ironic's case, due to the object
|
||||
model and the need to extract multiple pieces of data at varying levels
|
||||
of the data model to handle cases such as upgrades, the entire result set
|
||||
is downloaded and transformed which is an overhead you do not experience with
|
||||
a command line database client.
|
||||
|
||||
What can I do?
|
||||
==============
|
||||
|
||||
|
|
|
@ -15,6 +15,8 @@
|
|||
import sys
|
||||
|
||||
from oslo_config import cfg
|
||||
from oslo_db.sqlalchemy import enginefacade
|
||||
from oslo_db.sqlalchemy import utils
|
||||
from oslo_upgradecheck import common_checks
|
||||
from oslo_upgradecheck import upgradecheck
|
||||
|
||||
|
@ -49,6 +51,40 @@ class Checks(upgradecheck.UpgradeCommands):
|
|||
else:
|
||||
return upgradecheck.Result(upgradecheck.Code.FAILURE, details=msg)
|
||||
|
||||
def _check_db_indexes(self):
|
||||
"""Check if indexes exist on heavily used columns.
|
||||
|
||||
Checks the database to see if indexes exist on heavily used columns
|
||||
and provide guidance of action that can be taken to improve ironic
|
||||
database performance.
|
||||
"""
|
||||
engine = enginefacade.reader.get_engine()
|
||||
|
||||
indexes = [
|
||||
('nodes', 'reservation_idx'),
|
||||
('nodes', 'driver_idx'),
|
||||
('nodes', 'provision_state_idx'),
|
||||
('nodes', 'conductor_group_idx'),
|
||||
('nodes', 'resource_class_idx'),
|
||||
('nodes', 'reservation_idx'),
|
||||
('nodes', 'owner_idx'),
|
||||
('nodes', 'lessee_idx'),
|
||||
]
|
||||
missing_indexes = []
|
||||
for table, idx in indexes:
|
||||
if not utils.index_exists(engine, table, idx):
|
||||
missing_indexes.append(idx)
|
||||
|
||||
if missing_indexes:
|
||||
idx_list = ', '.join(missing_indexes)
|
||||
msg = ('Indexes missing for ideal database performance. Please '
|
||||
'consult https://docs.openstack.org/ironic/latest/admin/'
|
||||
'tuning.html for information on indexes. Missing: %s'
|
||||
% idx_list)
|
||||
return upgradecheck.Result(upgradecheck.Code.WARNING, details=msg)
|
||||
else:
|
||||
return upgradecheck.Result(upgradecheck.Code.SUCCESS)
|
||||
|
||||
# A tuple of check tuples of (<name of check>, <check function>).
|
||||
# The name of the check will be used in the output of this command.
|
||||
# The check function takes no arguments and returns an
|
||||
|
@ -59,6 +95,7 @@ class Checks(upgradecheck.UpgradeCommands):
|
|||
# summary will be rolled up at the end of the check() method.
|
||||
_upgrade_checks = (
|
||||
(_('Object versions'), _check_obj_versions),
|
||||
(_('Database Index Status'), _check_db_indexes),
|
||||
# Victoria -> Wallaby migration
|
||||
(_('Policy File JSON to YAML Migration'),
|
||||
(common_checks.check_policy_json, {'conf': CONF})),
|
||||
|
|
Loading…
Reference in New Issue