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

airflow db upgrade failed, no generic 'DROP CONSTRAINT' in MySQL #13222

Closed
rkhaled0 opened this issue Dec 21, 2020 · 13 comments
Closed

airflow db upgrade failed, no generic 'DROP CONSTRAINT' in MySQL #13222

rkhaled0 opened this issue Dec 21, 2020 · 13 comments
Labels
affected_version:2.0 Issues Reported for 2.0 area:upgrade Facilitating migration to a newer version of Airflow kind:bug This is a clearly a bug
Milestone

Comments

@rkhaled0
Copy link
Contributor

Apache Airflow version: 2.0.0

Kubernetes version (if you are using kubernetes) (use kubectl version): N/A

Environment: docker swarm

  • Cloud provider or hardware configuration:
  • OS (e.g. from /etc/os-release): Ubuntu 18.04.5 LTS (Bionic Beaver)
  • Kernel (e.g. uname -a):
  • Install tools: mysql-client=5.7
  • Others:

What happened:

When I try to upgrade airflow db with this following command:

airflow db upgrade

Something wrong happened :

initdb_1  | DB: mysql://**:***@x.x.x.x:****/mydb
initdb_1  | [2020-12-21 15:32:09,044] {db.py:678} INFO - Creating tables
initdb_1  | INFO  [alembic.runtime.migration] Context impl MySQLImpl.
initdb_1  | INFO  [alembic.runtime.migration] Will assume non-transactional DDL.
initdb_1  | INFO  [alembic.runtime.migration] Running upgrade 03afc6b6f902 -> cf5dc11e79ad, drop_user_and_chart
initdb_1  | Traceback (most recent call last):
initdb_1  |   File "/usr/local/bin/airflow", line 8, in <module>
initdb_1  |     sys.exit(main())
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/airflow/__main__.py", line 40, in main
initdb_1  |     args.func(args)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/airflow/cli/cli_parser.py", line 48, in command
initdb_1  |     return func(*args, **kwargs)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/airflow/utils/cli.py", line 89, in wrapper
initdb_1  |     return f(*args, **kwargs)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/airflow/cli/commands/db_command.py", line 48, in upgradedb
initdb_1  |     db.upgradedb()
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/airflow/utils/db.py", line 688, in upgradedb
initdb_1  |     command.upgrade(config, 'heads')
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/alembic/command.py", line 298, in upgrade
initdb_1  |     script.run_env()
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/alembic/script/base.py", line 489, in run_env
initdb_1  |     util.load_python_file(self.dir, "env.py")
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/alembic/util/pyfiles.py", line 98, in load_python_file
initdb_1  |     module = load_module_py(module_id, path)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/alembic/util/compat.py", line 184, in load_module_py
initdb_1  |     spec.loader.exec_module(module)
initdb_1  |   File "<frozen importlib._bootstrap_external>", line 728, in exec_module
initdb_1  |   File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/airflow/migrations/env.py", line 108, in <module>
initdb_1  |     run_migrations_online()
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/airflow/migrations/env.py", line 102, in run_migrations_online
initdb_1  |     context.run_migrations()
initdb_1  |   File "<string>", line 8, in run_migrations
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/alembic/runtime/environment.py", line 846, in run_migrations
initdb_1  |     self.get_context().run_migrations(**kw)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/alembic/runtime/migration.py", line 522, in run_migrations
initdb_1  |     step.migration_fn(**kw)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/airflow/migrations/versions/cf5dc11e79ad_drop_user_and_chart.py", line 49, in upgrade
initdb_1  |     op.drop_constraint('known_event_user_id_fkey', 'known_event')
initdb_1  |   File "<string>", line 8, in drop_constraint
initdb_1  |   File "<string>", line 3, in drop_constraint
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/alembic/operations/ops.py", line 159, in drop_constraint
initdb_1  |     return operations.invoke(op)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/alembic/operations/base.py", line 373, in invoke
initdb_1  |     return fn(self, operation)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/alembic/operations/toimpl.py", line 163, in drop_constraint
initdb_1  |     schema=operation.schema,
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/alembic/ddl/mysql.py", line 113, in drop_constraint
initdb_1  |     super(MySQLImpl, self).drop_constraint(const)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/alembic/ddl/impl.py", line 248, in drop_constraint
initdb_1  |     self._exec(schema.DropConstraint(const))
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/alembic/ddl/impl.py", line 141, in _exec
initdb_1  |     return conn.execute(construct, *multiparams, **params)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1011, in execute
initdb_1  |     return meth(self, multiparams, params)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/sql/ddl.py", line 72, in _execute_on_connection
initdb_1  |     return connection._execute_ddl(self, multiparams, params)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/engine/base.py", line 1066, in _execute_ddl
initdb_1  |     else None,
initdb_1  |   File "<string>", line 1, in <lambda>
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/sql/elements.py", line 481, in compile
initdb_1  |     return self._compiler(dialect, bind=bind, **kw)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/sql/ddl.py", line 29, in _compiler
initdb_1  |     return dialect.ddl_compiler(dialect, self, **kw)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 322, in __init__
initdb_1  |     self.string = self.process(self.statement, **compile_kwargs)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/sql/compiler.py", line 352, in process
initdb_1  |     return obj._compiler_dispatch(self, **kwargs)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/ext/compiler.py", line 441, in <lambda>
initdb_1  |     lambda *arg, **kw: existing(*arg, **kw),
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/sqlalchemy/ext/compiler.py", line 486, in __call__
initdb_1  |     return fn(element, compiler, **kw)
initdb_1  |   File "/usr/local/lib/python3.7/site-packages/alembic/ddl/mysql.py", line 394, in _mysql_drop_constraint
initdb_1  |     "No generic 'DROP CONSTRAINT' in MySQL - "
initdb_1  | NotImplementedError: No generic 'DROP CONSTRAINT' in MySQL - please specify constraint type

What you expected to happen:

database successfully upgraded (migration rules applied without errors)

How to reproduce it:

Upgrade existing database with mysql dialect from 1.10.14 to 2.0.0

Anything else we need to know:

every times

@rkhaled0 rkhaled0 added the kind:bug This is a clearly a bug label Dec 21, 2020
@potiuk
Copy link
Member

potiuk commented Dec 21, 2020

Which Database/Version do you have ? Is it MariaDB (and which version?). You only mentioned the client version in your report.

Note that Airflow 2.0 has a bit higher requirements than Airflow 1.10 when it comes to the DB. MariaDB is not supported, neither MySQL 5.6 (5.7 is supported with some limitations, only MySQL 5.8 version is fully supported).

If you are using non-supported version of DB you will have to upgrade/migrate to supported DB before attempting to upgrade to 2.0.

See the requirements here: https://github.com/apache/airflow#requirements

@rkhaled0
Copy link
Contributor Author

@potiuk , wow sorry, I missed specify that.

innodb_version: 5.7.31-34
version: Percona XtraDB Cluster (GPL), Release rel34

So, I could upgrade to 2.0 ?

@potiuk
Copy link
Member

potiuk commented Dec 21, 2020

I believe you should be able to, yes.

Which version of Airflow you migrated from ? You know that you should first migrate to 1.10.14 first and only then migrate to 2.0 ? Not sure if that solves the problem, but It would be great to know if that solves the problem?

https://airflow.apache.org/docs/apache-airflow/stable/upgrading-to-2.html

@rkhaled0
Copy link
Contributor Author

@potiuk sure,

I want to migrate from 1.10.14.

I've already read this documentation and that's why I've migrated to 1.10.14 at first.

Furthermore, the airflow upgrade_check confirms that check check for latest versions of apache-airflow and checker is succeeded.

So, upgrading to airflow 2 still broken .. 😕

@potiuk
Copy link
Member

potiuk commented Dec 21, 2020

Actually looks like Percona implements their own MySQL replacement (https://www.percona.com/doc/percona-server/5.7/index.html) , so this might be non fully-compatible MySQL engine..

Can you please escalate to them (CC: this ticket and comment) and see whether they can confirm it?

It looks like MySQL client that you use might simply not recognize the constraint type that is being dropped. So this might be both - problem with the version of your client libraries or with the Percona SQL Server not able to provide the constraint type to the client.

Below is the relevant code within Alembic that is responsible for the error. It looks like the foreign key constraint read from the schema ('known_event_user_id_fkey') is not properly recognized as one of the known constraint types (should be recognized as schema.ForeignKeyConstraint).

It might also be a problem with some old MySQL client libraries you are using in your Airflow installation.

Seems there is a similar problem reported by someone which was gone after upgrading the client libraries: https://www.reddit.com/r/flask/comments/2puang/error_using_sqlalchemy_alembic_and_flaskmigrate/

Here is the code that raises the problem:

@compiles(schema.DropConstraint, "mysql")
def _mysql_drop_constraint(element, compiler, **kw):
    """Redefine SQLAlchemy's drop constraint to
    raise errors for invalid constraint type."""

    constraint = element.element
    if isinstance(
        constraint,
        (
            schema.ForeignKeyConstraint,
            schema.PrimaryKeyConstraint,
            schema.UniqueConstraint,
        ),
    ):
        return compiler.visit_drop_constraint(element, **kw)
    elif isinstance(constraint, schema.CheckConstraint):
        # note that SQLAlchemy as of 1.2 does not yet support
        # DROP CONSTRAINT for MySQL/MariaDB, so we implement fully
        # here.
        if _is_mariadb(compiler.dialect):
            return "ALTER TABLE %s DROP CONSTRAINT %s" % (
                compiler.preparer.format_table(constraint.table),
                compiler.preparer.format_constraint(constraint),
            )
        else:
            return "ALTER TABLE %s DROP CHECK %s" % (
                compiler.preparer.format_table(constraint.table),
                compiler.preparer.format_constraint(constraint),
            )
    else:
        raise NotImplementedError(
            "No generic 'DROP CONSTRAINT' in MySQL - "
            "please specify constraint type"
        )

@potiuk potiuk added the invalid label Dec 21, 2020
@potiuk
Copy link
Member

potiuk commented Dec 21, 2020

I am closing that as 'invalid' for now, but please comment if you find that none of the provided directions work (or if you find that it worked, please post here confirming what worked).

@potiuk potiuk closed this as completed Dec 21, 2020
@rkhaled0
Copy link
Contributor Author

@potiuk Thank you for your explanations but I don't think so.

Why:

Existing airflow migration rules specify _type arg when calling op.drop_constraint alembic operation method.

But, this migration rule cf5dc11e79ad_drop_user_and_chart.py DOES NOT set _type arg value (https://github.com/apache/airflow/blob/master/airflow/migrations/versions/cf5dc11e79ad_drop_user_and_chart.py#L49).

Additionally, you can find a mention into the drop_constraint docstring method here which advise that _type required on MySQL.

It may be fixed by adding _type=foreignkey ?

Any opinion about this ?

@potiuk
Copy link
Member

potiuk commented Dec 21, 2020

Nice find - can you try it out please if it works ?

@potiuk potiuk removed the invalid label Dec 21, 2020
@potiuk potiuk reopened this Dec 21, 2020
@potiuk potiuk added this to the Airflow 2.0.1 milestone Dec 21, 2020
@rkhaled0
Copy link
Contributor Author

@potiuk so it's a bug ^^ ?

It's fixed but I've changed another stuff.

The constraint name known_event_user_id_fkey may not exist anymore in airflow 1.10.14.

So, if the goal of this migration rule is to drop all foreign keys referred to users table (before drop it), we can achieve that with this following logic:

def upgrade():  # noqa: D103
    # We previously had a KnownEvent's table, but we deleted the table without
    # a down migration to remove it (so we didn't delete anyone's data if they
    # were happing to use the feature.
    #
    # But before we can delete the users table we need to drop the FK

    conn = op.get_bind()
    inspector = Inspector.from_engine(conn)
    tables = inspector.get_table_names()

    if 'known_event' in tables:
        for fkey in inspector.get_foreign_keys(table_name="known_event", referred_table="users"):
            op.drop_constraint(
                fkey['name'], 'known_event', type_="foreignkey"
            )

    if "chart" in tables:
        op.drop_table(
            "chart",
        )

    if "users" in tables:
        op.drop_table("users")

With these modifications, next migration rules are applied without any regression bugs. So, I can successfully upgrade to airflow 2.

@rkhaled0
Copy link
Contributor Author

@potiuk I can create a PR for that if you approve these changes.

@potiuk
Copy link
Member

potiuk commented Dec 21, 2020

Happy to do so !

@rkhaled0
Copy link
Contributor Author

@potiuk 👋 done.

@potiuk
Copy link
Member

potiuk commented Dec 22, 2020

Nice! Thanks!

Fixed by #13239

@potiuk potiuk closed this as completed Dec 22, 2020
@vikramkoka vikramkoka added affected_version:2.0 Issues Reported for 2.0 area:upgrade Facilitating migration to a newer version of Airflow labels Feb 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected_version:2.0 Issues Reported for 2.0 area:upgrade Facilitating migration to a newer version of Airflow kind:bug This is a clearly a bug
Projects
None yet
Development

No branches or pull requests

3 participants