Merge "Ironic index docs/command check backport" into stable/wallaby

devstack/lib/ironic

@@ -1845,7 +1845,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.

devstack/upgrade/upgrade.sh

@@ -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
 

doc/source/admin/tuning.rst

@@ -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?
 ==============
 

ironic/cmd/status.py

@@ -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})),