geopython / GeoHealthCheck

Service Status and QoS Checker for OGC Web Services
https://geohealthcheck.org
MIT License
84 stars 71 forks source link

Partial fix for #326: add indexes to DB tables in order to improve page load speed #393

Closed fsteggink closed 3 years ago

fsteggink commented 3 years ago

With this PR indexes are created in the database when it is initialized. I've tested this with SQLite. I'll also check this with PostgreSQL, as well as whether existing databases are upgraded automatically.

fsteggink commented 3 years ago

I've confirmed that the migration script which I just added works both for SQLite and Postgres. However, I wasn't able to test the creation of a new DB. I've got an error message, but it doesn't appear to be related to the indexes.

fsteggink commented 3 years ago

Yes, that was unintended. My apologies.

justb4 commented 3 years ago

Hmm, somehow I always see errors for "web" and "Runner" on each start using latest Docker Image:

python manage.py db upgrade
2021-08-31 10:55:28,733 - init - INFO - created GHC App instance #1
INFO  [alembic.runtime.migration] Context impl PostgresqlImpl.
INFO  [alembic.runtime.migration] Will assume transactional DDL.
INFO  [alembic.runtime.migration] Running upgrade  -> 496427d03f87, empty message
INFO  [alembic.runtime.migration] Running upgrade 496427d03f87 -> 992013af402f, empty message
INFO  [alembic.runtime.migration] Running upgrade 992013af402f -> 2638c2a40625, empty message
INFO  [alembic.runtime.migration] Running upgrade 2638c2a40625 -> bb91fb332c36, empty message
INFO  [alembic.runtime.migration] Running upgrade bb91fb332c36 -> 34531bfd7cab, empty message
INFO  [alembic.runtime.migration] Running upgrade 34531bfd7cab -> 90e1c865a561, empty message
INFO  [alembic.runtime.migration] Running upgrade 90e1c865a561 -> f72ff1ac3967, empty message
INFO  [alembic.runtime.migration] Running upgrade f72ff1ac3967 -> 3381760e2a8c, empty message
Before: DB tables: ['spatial_ref_sys', 'user', 'run', 'tag', 'recipient', 'resource', 'resource_lock', 'resource_tags', 'resourcenotification', 'probe_vars', 'check_vars']
Tables for tag-support already present, will not create them
Tables for Probe-support already present, will not create them
Column report already present in run table
Column active already present in resource table
Tables for recipient support already present, will not create them
Column run_frequency already present in resource table
Table for Resource-locking already present, will not create
Column auth already present in resource table
Traceback (most recent call last):
  File "/venv/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1249, in _execute_context
    cursor, statement, parameters, context
  File "/venv/lib/python3.7/site-packages/sqlalchemy/engine/default.py", line 552, in do_execute
    cursor.execute(statement, parameters)
psycopg2.ProgrammingError: relation "ix_check_vars_probe_vars_identifier" already exists

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
  File "manage.py", line 53, in <module>
    manager.run()
  File "/venv/lib/python3.7/site-packages/flask_script/__init__.py", line 417, in run
    result = self.handle(argv[0], argv[1:])
  File "/venv/lib/python3.7/site-packages/flask_script/__init__.py", line 386, in handle
    res = handle(*args, **config)
  File "/venv/lib/python3.7/site-packages/flask_script/commands.py", line 216, in __call__
    return self.run(*args, **kwargs)
  File "/venv/lib/python3.7/site-packages/flask_migrate/__init__.py", line 95, in wrapped
    f(*args, **kwargs)
  File "/venv/lib/python3.7/site-packages/flask_migrate/__init__.py", line 280, in upgrade
    command.upgrade(config, revision, sql=sql, tag=tag)
  File "/venv/lib/python3.7/site-packages/alembic/command.py", line 294, in upgrade
    script.run_env()
  File "/venv/lib/python3.7/site-packages/alembic/script/base.py", line 490, in run_env
    util.load_python_file(self.dir, "env.py")
  File "/venv/lib/python3.7/site-packages/alembic/util/pyfiles.py", line 97, in load_python_file
    module = load_module_py(module_id, path)
  File "/venv/lib/python3.7/site-packages/alembic/util/compat.py", line 184, in load_module_py
    spec.loader.exec_module(module)
  File "<frozen importlib._bootstrap_external>", line 728, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "migrations/env.py", line 87, in <module>
    run_migrations_online()
  File "migrations/env.py", line 80, in run_migrations_online
    context.run_migrations()
  File "<string>", line 8, in run_migrations
  File "/venv/lib/python3.7/site-packages/alembic/runtime/environment.py", line 813, in run_migrations
    self.get_context().run_migrations(**kw)
  File "/venv/lib/python3.7/site-packages/alembic/runtime/migration.py", line 561, in run_migrations
    step.migration_fn(**kw)
  File "/GeoHealthCheck/GeoHealthCheck/migrations/versions/3381760e2a8c_.py", line 21, in upgrade
    op.create_index(op.f('ix_check_vars_probe_vars_identifier'), 'check_vars', ['probe_vars_identifier'], unique=False)
  File "<string>", line 8, in create_index
  File "<string>", line 3, in create_index
  File "/venv/lib/python3.7/site-packages/alembic/operations/ops.py", line 829, in create_index
    return operations.invoke(op)
  File "/venv/lib/python3.7/site-packages/alembic/operations/base.py", line 354, in invoke
    return fn(self, operation)
  File "/venv/lib/python3.7/site-packages/alembic/operations/toimpl.py", line 88, in create_index
    operations.impl.create_index(idx)
  File "/venv/lib/python3.7/site-packages/alembic/ddl/impl.py", line 300, in create_index
    self._exec(schema.CreateIndex(index))
  File "/venv/lib/python3.7/site-packages/alembic/ddl/impl.py", line 146, in _exec
    return conn.execute(construct, multiparams)
  File "/venv/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 988, in execute
    return meth(self, multiparams, params)
  File "/venv/lib/python3.7/site-packages/sqlalchemy/sql/ddl.py", line 72, in _execute_on_connection
    return connection._execute_ddl(self, multiparams, params)
  File "/venv/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1050, in _execute_ddl
    compiled,
  File "/venv/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1253, in _execute_context
    e, statement, parameters, cursor, context
  File "/venv/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1473, in _handle_dbapi_exception
    util.raise_from_cause(sqlalchemy_exception, exc_info)
  File "/venv/lib/python3.7/site-packages/sqlalchemy/util/compat.py", line 398, in raise_from_cause
    reraise(type(exception), exception, tb=exc_tb, cause=cause)
  File "/venv/lib/python3.7/site-packages/sqlalchemy/util/compat.py", line 152, in reraise
    raise value.with_traceback(tb)
  File "/venv/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1249, in _execute_context
    cursor, statement, parameters, context
  File "/venv/lib/python3.7/site-packages/sqlalchemy/engine/default.py", line 552, in do_execute
    cursor.execute(statement, parameters)
sqlalchemy.exc.ProgrammingError: (psycopg2.ProgrammingError) relation "ix_check_vars_probe_vars_identifier" already exists

[SQL: CREATE INDEX ix_check_vars_probe_vars_identifier ON check_vars (probe_vars_identifier)]
(Background on this error at: http://sqlalche.me/e/f405)

Captured Task Output:
---------------------

---> pavement.upgrade
Upgrading database...
cd /GeoHealthCheck/GeoHealthCheck
python manage.py db upgrade

Build failed running pavement.upgrade: Subprocess return code: 1

In Docker each start will also call paver upgrade which does nothing if (Alembic) upgrades were already done. But somehow it always fails so all upgrades are attempted on each start. IMO the Alembic script should check if the indexes exist and only create them. See also the other Alembic .py scripts under migrations/versions. See also migrations/alembic_helpers.py which has util functions that check presence of columns, tables etc. Here an index_exists() function may be added.

justb4 commented 3 years ago

Will open a new issue for this...