Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

psycopg2.errors.UndefinedTable: relation "signal_type_override" does not exist #1507

Closed
sbruens opened this issue Jan 11, 2024 · 8 comments · Fixed by #1563
Closed

psycopg2.errors.UndefinedTable: relation "signal_type_override" does not exist #1507

sbruens opened this issue Jan 11, 2024 · 8 comments · Fixed by #1563
Assignees
Labels
bug hma Items related to the hasher-matcher-actioner system successful reproduction This bug has a consistent reproduction

Comments

@sbruens
Copy link
Collaborator

sbruens commented Jan 11, 2024

@Dcallies Running the stack locally, when I try to hash an image (/h/hash?...) I run into a server error.

[2024-01-11 22:15:14 +0000] [1] [INFO] Starting gunicorn 21.2.0
[2024-01-11 22:15:14 +0000] [1] [INFO] Listening at: http://0.0.0.0:8080 (1)
[2024-01-11 22:15:14 +0000] [1] [INFO] Using worker: sync
[2024-01-11 22:15:14 +0000] [7] [INFO] Booting worker with pid: 7
[2024-01-11 22:15:15,880] WARNING in app: No storage class provided, using the default
[2024-01-11 22:15:48,476] ERROR in app: Exception on /h/hash [GET]
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1969, in _exec_single_context
    self.dialect.do_execute(
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/default.py", line 922, in do_execute
    cursor.execute(statement, parameters)
psycopg2.errors.UndefinedTable: relation "signal_type_override" does not exist
LINE 2: FROM signal_type_override
             ^


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

Traceback (most recent call last):
  File "/usr/local/lib/python3.11/site-packages/flask/app.py", line 1455, in wsgi_app
    response = self.full_dispatch_request()
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/flask/app.py", line 869, in full_dispatch_request
    rv = self.handle_user_exception(e)
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/flask/app.py", line 867, in full_dispatch_request
    rv = self.dispatch_request()
         ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/flask/app.py", line 852, in dispatch_request
    return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/build/OpenMediaMatch/blueprints/hashing.py", line 51, in hash_media
    signal_types = _parse_request_signal_type(content_type)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/build/OpenMediaMatch/blueprints/hashing.py", line 144, in _parse_request_signal_type
    signal_types = get_storage().get_enabled_signal_types_for_content_type(content_type)
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/build/OpenMediaMatch/storage/interface.py", line 120, in get_enabled_signal_types_for_content_type
    for k, v in self.get_signal_type_configs().items()
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/build/OpenMediaMatch/storage/postgres/impl.py", line 102, in get_signal_type_configs
    signal_type_overrides = self._query_signal_type_overrides()
                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/build/OpenMediaMatch/storage/postgres/impl.py", line 133, in _query_signal_type_overrides
    db_records = database.db.session.execute(
                 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/orm/scoping.py", line 782, in execute
    return self._proxied.execute(
           ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/orm/session.py", line 2308, in execute
    return self._execute_internal(
           ^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/orm/session.py", line 2190, in _execute_internal
    result: Result[Any] = compile_state_cls.orm_execute_statement(
                          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/orm/context.py", line 293, in orm_execute_statement
    result = conn.execute(
             ^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1416, in execute
    return meth(
           ^^^^^
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/sql/elements.py", line 517, in _execute_on_connection
    return connection._execute_clauseelement(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1639, in _execute_clauseelement
    ret = self._execute_context(
          ^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1848, in _execute_context
    return self._exec_single_context(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1988, in _exec_single_context
    self._handle_dbapi_exception(
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 2344, in _handle_dbapi_exception
    raise sqlalchemy_exception.with_traceback(exc_info[2]) from e
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/base.py", line 1969, in _exec_single_context
    self.dialect.do_execute(
  File "/usr/local/lib/python3.11/site-packages/sqlalchemy/engine/default.py", line 922, in do_execute
    cursor.execute(statement, parameters)
sqlalchemy.exc.ProgrammingError: (psycopg2.errors.UndefinedTable) relation "signal_type_override" does not exist
LINE 2: FROM signal_type_override
             ^

[SQL: SELECT signal_type_override.id, signal_type_override.name, signal_type_override.enabled_ratio 
FROM signal_type_override]
(Background on this error at: https://sqlalche.me/e/20/f405)

Did something change in the database schema since I last ran it? Any thoughts?

@Dcallies
Copy link
Contributor

Thanks for the report! The database changed a lot during development, and we don't have a test on this.

I'll add a regression test and fix it.

@Dcallies Dcallies self-assigned this Jan 12, 2024
@Dcallies Dcallies added bug hma Items related to the hasher-matcher-actioner system labels Jan 12, 2024
@Dcallies
Copy link
Contributor

Did not reproduce, what I tried:

curl localhost:5000/h/hash?url='https://github.com/facebook/ThreatExchange/blob/main/pdq/data/bridge-mods/aaa-orig.jpg?

raw=true'
{
"pdq": "f8f8f0cee0f4a84f06370a22038f63f0b36e2ed596621e1d33e6b39c4e9c9b22"
}

Wasn't able to reproduce, can you check to see if it's a strange startup issue, or possible a collision with an old deployment?

Can you try the "nuclear" option mentioned in CONTRIBUTING:

cd /workspace/src/OpenMediaMatch
flask reset_all_tables

@Dcallies Dcallies added the did not reproduce We tried to get the bug to happen, but it didn't! Can you provide the steps or more detail to help? label Jan 12, 2024
@Sam-Freeman
Copy link

@Dcallies to flag, this is the same issue I hit while running set up for the first time. My initial suggestion in #1504 was to comment out the blueprints in the create_app function.

I since came back to this and found it's because the tables are initially never created, so you need to run flask --app OpenMediaMatch.app create_tables. However, this also fails because of the blueprints referencing some relations (which is why commenting it out, creating tables, uncommenting and running works). However, there's another fix to add database.db.create_all() after initialising the flask app with storage, and then everything just works™️.

I don't know if this is the best practice, and I haven't had time today to properly look at this and get a PR up.

@Dcallies Dcallies removed the did not reproduce We tried to get the bug to happen, but it didn't! Can you provide the steps or more detail to help? label Jan 16, 2024
@Dcallies
Copy link
Contributor

@Sam-Freeman thanks for the context! Sounds like it should repo with a clean rebuild (which I had done recently without hitting it, but I didn't try since this issue was opened).

This sounds like it's an import dependency problem, which is why I'm not a huge fan of flask's module-level state.

Let me try a clean build and see if I can get it to trigger.

@Dcallies
Copy link
Contributor

Okay, working with @sbruens we have a repro.

The workaround: set TASK_INDEX_CACHE = False in the .devcontainer omm config, run flask reset_all_tables

The problem is that I don't think the database is setup by the time the code in #1499 tries to make a database call.

It's not caught in testing because we we disable that flag in the tests.

Let me dig some more into what's happening and see if I can regtest and fix.

@Dcallies Dcallies added the successful reproduction This bug has a consistent reproduction label Jan 17, 2024
@sbruens
Copy link
Collaborator Author

sbruens commented Jan 17, 2024

Workaround for the Docker stack. Login to the container:

docker exec -it open-media-match-app-1 bash

Then run:

cd OpenMediaMatch && flask reset_all_tables

@juanmrad
Copy link
Collaborator

Looked into this issue further.

This is definitively not an alambic issue, but a index cache issue, but an app set-up issue.

Here:

if app.config.get("TASK_INDEX_CACHE", False):

Which will cause the app to build the cache when the app is started up, but this is not what we want when we do the db upgrade:

flask --app OpenMediaMatch.app db upgrade --directory /workspace/src/OpenMediaMatch/migrations

we do not need these processes running. as such I'll be adding a guard via env variable to this command to prevent it from running the code causing the problem.

@Dcallies any thoughts on this approach?

adding something along the lines of:

if app.config.get("TASK_INDEX_CACHE", False) and not running_migrations:

I can cut a pr in a bit. Also got the initial migration fix so we can keep track of db migrations.

@Dcallies
Copy link
Contributor

Nice work @juanmrad! Even when I disabled the TASK_INDEX_CACHE, I got the same issue unless I set up the DB.

Here's the repo I've used:

  1. Set TASK_INDEX_CACHE = False in omm.cfg
  2. Clean docker build
  3. curl localhost:5000

This will give me a 500 for tables not existing

Does setting up alembic fix this repo?

If so, what do you think about these potential approaches:

  1. Move migrations out of app startup by default (since I assume only development build will do migrations), and into docker build or some pre-app setup step
  2. Reordering setup so migrations happen before background tasks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug hma Items related to the hasher-matcher-actioner system successful reproduction This bug has a consistent reproduction
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants