diff --git a/.github/workflows/db_indexes.yaml b/.github/workflows/db_indexes.yaml new file mode 100644 index 00000000000..846f2cc5956 --- /dev/null +++ b/.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 diff --git a/check_model.sh b/check_model.sh new file mode 100755 index 00000000000..67a2f33720e --- /dev/null +++ b/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; diff --git a/lib/galaxy/model/mapping.py b/lib/galaxy/model/mapping.py index 11e84951eca..7d68dd3a8d4 100644 --- a/lib/galaxy/model/mapping.py +++ b/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), ) diff --git a/lib/galaxy/model/migrate/check.py b/lib/galaxy/model/migrate/check.py index d45974e2c73..9b7eaf355b1 100644 --- a/lib/galaxy/model/migrate/check.py +++ b/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. diff --git a/lib/galaxy/model/migrate/versions/0170_add_more_missing_indexes.py b/lib/galaxy/model/migrate/versions/0170_add_more_missing_indexes.py new file mode 100644 index 00000000000..d5f5fa54703 --- /dev/null +++ b/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) diff --git a/lib/galaxy/model/migrate/versions/util.py b/lib/galaxy/model/migrate/versions/util.py index cb6a7d4af82..c737d18efec 100644 --- a/lib/galaxy/model/migrate/versions/util.py +++ b/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) diff --git a/scripts/check_model.py b/scripts/check_model.py new file mode 100644 index 00000000000..65e6d7cb4f4 --- /dev/null +++ b/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) diff --git a/tox.ini b/tox.ini index d3fe0fe0ec9..de8922ed99b 100644 --- a/tox.ini +++ b/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