Merge pull request #10350 from ic4f/dev_missing_indexes_script

Script for finding missing db indexes

.github/workflows/db_indexes.yaml

@@ -0,0 +1,45 @@
+name: Database indexes
+on: [push, pull_request]
+defaults:
+  run:
+    working-directory: 'galaxy root'
+jobs:
+  check:
+    name: Check database indexes
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        python-version: [3.7]
+        db: ['postgresql', 'sqlite']
+    services:
+      postgres:
+        image: postgres:11
+        env:
+          POSTGRES_USER: postgres
+          POSTGRES_PASSWORD: postgres
+          POSTGRES_DB: postgres
+        ports:
+          - 5432:5432
+    steps:
+    - uses: actions/checkout@v2
+      with:
+        path: 'galaxy root'
+    - uses: actions/setup-python@v1
+      with:
+        python-version: ${{ matrix.python-version }}
+    - name: Cache pip dir
+      uses: actions/cache@v1
+      id: pip-cache
+      with:
+        path: ~/.cache/pip
+        key: pip-cache-${{ matrix.python-version }}-${{ hashFiles('galaxy root/requirements.txt') }}
+    - name: Install tox
+      run: pip install tox
+    - name: Check indexes on postgresql
+      if: matrix.db == 'postgresql'
+      env:
+        GALAXY_CONFIG_OVERRIDE_DATABASE_CONNECTION: 'postgres://postgres:postgres@localhost:5432/galaxy?client_encoding=utf8'
+      run: tox -e check_indexes
+    - name: Check indexes on sqlite
+      if: matrix.db == 'sqlite'
+      run: tox -e check_indexes

check_model.sh

@@ -0,0 +1,12 @@
+#!/bin/sh
+
+cd "$(dirname "$0")"
+
+. ./scripts/common_startup_functions.sh
+
+setup_python
+
+python ./scripts/check_model.py "$@"
+
+[ $? -ne 0 ] && exit 1;
+exit 0;

lib/galaxy/model/mapping.py

@@ -1125,18 +1125,18 @@ model.WorkflowInvocationOutputDatasetAssociation.table = Table(
     "workflow_invocation_output_dataset_association", metadata,
     Column("id", Integer, primary_key=True),
     Column("workflow_invocation_id", Integer, ForeignKey("workflow_invocation.id"), index=True),
-    Column("workflow_step_id", Integer, ForeignKey("workflow_step.id")),
+    Column("workflow_step_id", Integer, ForeignKey("workflow_step.id"), index=True),
     Column("dataset_id", Integer, ForeignKey("history_dataset_association.id"), index=True),
-    Column("workflow_output_id", Integer, ForeignKey("workflow_output.id")),
+    Column("workflow_output_id", Integer, ForeignKey("workflow_output.id"), index=True),
 )
 
 model.WorkflowInvocationOutputDatasetCollectionAssociation.table = Table(
     "workflow_invocation_output_dataset_collection_association", metadata,
     Column("id", Integer, primary_key=True),
     Column("workflow_invocation_id", Integer, ForeignKey("workflow_invocation.id", name='fk_wiodca_wii'), index=True),
-    Column("workflow_step_id", Integer, ForeignKey("workflow_step.id", name='fk_wiodca_wsi')),
+    Column("workflow_step_id", Integer, ForeignKey("workflow_step.id", name='fk_wiodca_wsi'), index=True),
     Column("dataset_collection_id", Integer, ForeignKey("history_dataset_collection_association.id", name='fk_wiodca_dci'), index=True),
-    Column("workflow_output_id", Integer, ForeignKey("workflow_output.id", name='fk_wiodca_woi')),
+    Column("workflow_output_id", Integer, ForeignKey("workflow_output.id", name='fk_wiodca_woi'), index=True),
 )
 
 model.WorkflowInvocationOutputValue.table = Table(
@@ -1160,7 +1160,7 @@ model.WorkflowInvocationStepOutputDatasetCollectionAssociation.table = Table(
     "workflow_invocation_step_output_dataset_collection_association", metadata,
     Column("id", Integer, primary_key=True),
     Column("workflow_invocation_step_id", Integer, ForeignKey("workflow_invocation_step.id", name='fk_wisodca_wisi'), index=True),
-    Column("workflow_step_id", Integer, ForeignKey("workflow_step.id", name='fk_wisodca_wsi')),
+    Column("workflow_step_id", Integer, ForeignKey("workflow_step.id", name='fk_wisodca_wsi'), index=True),
     Column("dataset_collection_id", Integer, ForeignKey("history_dataset_collection_association.id", name='fk_wisodca_dci'), index=True),
     Column("output_name", String(255), nullable=True),
 )

lib/galaxy/model/migrate/check.py

@@ -62,12 +62,13 @@ def create_or_verify_database(url, galaxy_config_file, engine_options={}, app=No
         migrate_to_current_version(engine, db_schema)
 
     def migrate_from_scratch():
-        log.info("Creating new database from scratch, skipping migrations")
-        current_version = migrate_repository.version().version
-        mapping.init(file_path='/tmp', url=url, map_install_models=map_install_models, create_tables=True)
-        schema.ControlledSchema.create(engine, migrate_repository, version=current_version)
-        db_schema = schema.ControlledSchema(engine, migrate_repository)
-        assert db_schema.version == current_version
+        if not os.environ.get("GALAXY_TEST_FORCE_DATABASE_MIGRATION"):
+            log.info("Creating new database from scratch, skipping migrations")
+            current_version = migrate_repository.version().version
+            mapping.init(file_path='/tmp', url=url, map_install_models=map_install_models, create_tables=True)
+            schema.ControlledSchema.create(engine, migrate_repository, version=current_version)
+            db_schema = schema.ControlledSchema(engine, migrate_repository)
+            assert db_schema.version == current_version
         migrate()
         if app:
             # skips the tool migration process.

lib/galaxy/model/migrate/versions/0170_add_more_missing_indexes.py

@@ -0,0 +1,60 @@
+"""
+Migration script to create missing indexes.  Adding new columns to existing tables via SQLAlchemy does not create the index, even if the column definition includes index=True.
+"""
+
+import logging
+
+from sqlalchemy import MetaData
+
+from galaxy.model.migrate.versions.util import (
+    add_index,
+    drop_index
+)
+
+log = logging.getLogger(__name__)
+metadata = MetaData()
+
+indexes = [
+    [
+        "ix_workflow_invocation_output_dataset_association_workflow_output_id",
+        "workflow_invocation_output_dataset_association",
+        "workflow_output_id"
+    ],
+    [
+        "ix_workflow_invocation_output_dataset_association_workflow_step_id",
+        "workflow_invocation_output_dataset_association",
+        "workflow_step_id"
+    ],
+    [
+        "ix_workflow_invocation_output_dataset_collection_association_workflow_output_id",
+        "workflow_invocation_output_dataset_collection_association",
+        "workflow_output_id"
+    ],
+    [
+        "ix_workflow_invocation_output_dataset_collection_association_workflow_step_id",
+        "workflow_invocation_output_dataset_collection_association",
+        "workflow_step_id"
+    ],
+    [
+        "ix_workflow_invocation_step_output_dataset_collection_association_workflow_step_id",
+        "workflow_invocation_step_output_dataset_collection_association",
+        "workflow_step_id"
+    ],
+]
+
+
+def upgrade(migrate_engine):
+    print(__doc__)
+    metadata.bind = migrate_engine
+    metadata.reflect()
+
+    for ix, table, col in indexes:
+        add_index(ix, table, col, metadata)
+
+
+def downgrade(migrate_engine):
+    metadata.bind = migrate_engine
+    metadata.reflect()
+
+    for ix, table, col in indexes:
+        drop_index(ix, table, col, metadata)

lib/galaxy/model/migrate/versions/util.py

@@ -86,6 +86,7 @@ def add_column(column, table, metadata, **kwds):
     :type metadata: :class:`Metadata`
     """
     try:
+        index_to_create = None
         migrate_engine = metadata.bind
         if not isinstance(table, Table):
             table = Table(table, metadata, autoload=True)
@@ -93,10 +94,14 @@ def add_column(column, table, metadata, **kwds):
             # SQLAlchemy Migrate has a bug when adding a column with both a
             # ForeignKey and an index in SQLite. Since SQLite creates an index
             # anyway, we can drop the explicit index creation.
+            # TODO: this is hacky, but it solves this^ problem. Needs better solution.
+            index_to_create = (kwds['index_name'], table, column.name)
             del kwds['index_name']
             column.index = False
         column.create(table, **kwds)
         assert column is table.c[column.name]
+        if index_to_create:
+            add_index(*index_to_create)
     except Exception:
         log.exception("Adding column '%s' to table '%s' failed.", column, table)
 

scripts/check_model.py

@@ -0,0 +1,57 @@
+"""
+Check for db indexes defined in mapping.py but missing in the database.
+Note: pass '-c /path/to/galaxy.yml' to use the database_connection set in galaxy.yml.
+Otherwise the default sqlite database will be used.
+"""
+import json
+import os
+import sys
+from collections import namedtuple
+
+sys.path.insert(1, os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir, 'lib')))
+
+from sqlalchemy import create_engine, MetaData
+
+from galaxy.model import mapping
+from galaxy.model.orm.scripts import get_config
+
+IndexTuple = namedtuple('IndexTuple', 'table column_names')
+
+
+def tuple_from_index(index):
+    columns = tuple([getattr(index.columns, c).name for c in dir(index.columns) if not c.startswith('__')])
+    if len(columns) == 1:
+        columns = columns[0]
+    return IndexTuple(index.table.name, columns)
+
+
+def find_missing_indexes():
+
+    def load_indexes(metadata):
+        indexes = {}
+        for t in metadata.tables.values():
+            for index in t.indexes:
+                index_tuple = tuple_from_index(index)
+                indexes[index_tuple] = index.name
+        return indexes
+
+    # load metadata from mapping.py
+    metadata = mapping.metadata
+    mapping_indexes = load_indexes(metadata)
+
+    # create EMPTY metadata, then load from database
+    db_url = get_config(sys.argv)['db_url']
+    metadata = MetaData(bind=create_engine(db_url))
+    metadata.reflect()
+    indexes_in_db = load_indexes(metadata)
+
+    missing_indexes = set(mapping_indexes.keys()) - set(indexes_in_db.keys())
+    if missing_indexes:
+        return [(mapping_indexes[index], index.table, index.column_names) for index in missing_indexes]
+
+
+if __name__ == '__main__':
+    indexes = find_missing_indexes()
+    if indexes:
+        print(json.dumps(indexes, indent=4, sort_keys=True))
+        sys.exit(1)

tox.ini

@@ -13,7 +13,9 @@ commands =
 
     unit: bash run_tests.sh -u
 whitelist_externals = bash
-passenv = CI CONDA_EXE
+passenv = 
+    CI CONDA_EXE
+    GALAXY_CONFIG_OVERRIDE_DATABASE_CONNECTION
 setenv =
     first_startup: GALAXY_PYTHON=python
     first_startup: GALAXY_CONFIG_DATABASE_AUTO_MIGRATE=true
@@ -21,6 +23,8 @@ setenv =
     unit: GALAXY_VIRTUAL_ENV={envdir}
     unit: GALAXY_ENABLE_BETA_COMPRESSED_GENBANK_SNIFFING=1
     mulled: GALAXY_TEST_INCLUDE_SLOW=1
+    check_indexes: GALAXY_TEST_FORCE_DATABASE_MIGRATION=1
+    check_indexes: GALAXY_SKIP_CLIENT_BUILD=1
 deps =
     lint,lint_docstring,lint_docstring_include_list: -rlib/galaxy/dependencies/pipfiles/flake8/pinned-requirements.txt
     unit: mock-ssh-server
@@ -46,3 +50,9 @@ commands = bash .ci/validate_test_tools.sh
 
 [testenv:web_controller_line_count]
 commands = bash .ci/check_controller.sh
+
+[testenv:check_indexes]
+commands = 
+    bash scripts/common_startup.sh
+    bash create_db.sh
+    bash check_model.sh