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

support Flyway for TiDB #19449

Closed
bb7133 opened this issue Aug 25, 2020 · 8 comments
Closed

support Flyway for TiDB #19449

bb7133 opened this issue Aug 25, 2020 · 8 comments
Labels
feature/discussing This feature request is discussing among product managers type/feature-request Categorizes issue or PR as related to a new feature.

Comments

@bb7133
Copy link
Member

bb7133 commented Aug 25, 2020

Feature Request

Is your feature request related to a problem? Please describe:
Flyway is an open-source framework for database migration. various databases have been supported by it including MySQL and MariaDB.

For now, Flyway reports an error when doing the migration on TiDB:

[bb7133@bb7133-manjaro flyway-6.5.3]$ flyway migrate
Flyway Community Edition 6.5.3 by Redgate
Database: jdbc:mysql://127.0.0.1:4000/test (MySQL 5.7)
Successfully validated 2 migrations (execution time 00:00.023s)
ERROR: Unable to release MySQL named lock: Flyway--562162009

java.sql.SQLSyntaxErrorException: function release_lock has only noop implementation in tidb now, use tidb_enable_noop_functions to enable these functions

The error is caused by #14994 (GET_LOCK and RELEASE_LOCK, the "named lock"), by setting set global tidb_enable_noop_functions = 1, Flyway is able to work with the ignorance of the named lock:

[bb7133@bb7133-manjaro flyway-6.5.3]$ flyway migrate
Flyway Community Edition 6.5.3 by Redgate
Database: jdbc:mysql://127.0.0.1:4000/test (MySQL 5.7)
Successfully validated 2 migrations (execution time 00:00.021s)
Creating Schema History table `test`.`flyway_schema_history` ...
Current version of schema `test`: << Empty Schema >>
Migrating schema `test` to version 1 - Add accounts table
Migrating schema `test` to version 2 - Alter accounts pk

Successfully applied 2 migrations to schema `test` (execution time 00:00.148s)

Describe the feature you'd like:
Since TiDB aims to be compatible with MySQL, it should be supported by Flyway as well.

Possible solutions:

  1. Close UCP: Support get_lock and release_lock functions #14994
    This is a solution that fits the goals of 'MySQL compatibility', but the implementation is not trivial. GET_LOCK() and RELEASE_LOCK() in MySQL is relying on the 'metadata lock' while there's no such lock in TiDB. After some discussions with @cfzjywxk , we believe that the named lock should be implemented in a similar way like the pessimistic lock, but some details must be handled carefully.
    For now, we don't treat it as a short-term solution.

  2. Propose an implementation for TiDB in Flyway
    By adding a specific implementation for TiDB, we don't have to implement GET_LOCK() and RELEASE_LOCK(). Actually this is the way for some MySQL-based solutions like XtraDB Cluster(Percona XtraDB Cluster support flyway/flyway#1556).
    Possible implementations:
    a) Read/write pessimistic lock, and this is the way XtraDB Cluster works.
    Pros: the implementation is simple.
    Cons: the pessimistic lock in TiDB is with a TTL, and its default value is [10 minutes[(https://docs.pingcap.com/tidb/stable/tidb-configuration-file#max-txn-ttl). The TTL configuration must be set accordingly with the estimated migration duration.
    b) Table lock: *: Support LOCK/UNLOCK TABLES feature #10343
    Pros: the implementation is simple, and there is no TTL limitation.
    Cons:

    • Table lock in TiDB is disabled by default.
    • When the TiDB server does not gracefully shutdown(for example, killed because of OOM), the table lock will be unexpected left and users must clean it with an explicit UNLOCK TABLE ... statement. This is why 'table lock' is not public yet.

Teachability, Documentation, Adoption, Migration Strategy:

@bb7133 bb7133 added the type/feature-request Categorizes issue or PR as related to a new feature. label Aug 25, 2020
@ghost
Copy link

ghost commented Aug 25, 2020

There is another issue with flyway in #15131 (comment)

It issues a query select @@foreign_key_checks, which is typed in MySQL. I'm not sure if this is something in older versions, or only becomes evident after setting tidb_enable_noop_functions=1?

@bb7133
Copy link
Member Author

bb7133 commented Aug 25, 2020

There is another issue with flyway in #15131 (comment)

@nullnotnil Looks that the issue exists for all versions and we should deal with it as well. Thanks!

@ghost
Copy link

ghost commented Aug 26, 2020

I like 2(a) as an initial solution because it is an easy PR to suggest to upstream (handle TiDB the same as PXC).

2(b) Requires more work on upstream, and likely MySQL behavior differences because calling LOCK TABLES subsequent times releases previous locks.

Users will just have to extend the pessimistic lock time to >10minutes if they have long-running DDL. They currently have to extend gc to >10 minutes to create a backup too, so this is not a new problem - I would actually consider the backup one to be more serious.

@zz-jason zz-jason added the feature/discussing This feature request is discussing among product managers label Aug 28, 2020
@bb7133
Copy link
Member Author

bb7133 commented Sep 3, 2020

I've filed a PR to Flyway: flyway/flyway#2918

@scsldb
Copy link

scsldb commented Sep 4, 2020

To be determined, while additional frameworks need to be considered for support.

@morgo
Copy link
Contributor

morgo commented Sep 23, 2021

This has been added in Flyway 7.11
flyway/flyway@77b50ee

@morgo morgo closed this as completed Sep 23, 2021
@xyziven
Copy link

xyziven commented Mar 8, 2024

Looks like it is broken for flyway newer than 8.2.1

@dveeden
Copy link
Contributor

dveeden commented May 15, 2024

Looks like it is broken for flyway newer than 8.2.1

@xyziven Can you give some more details on this? Then we can open a new issue for this if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature/discussing This feature request is discussing among product managers type/feature-request Categorizes issue or PR as related to a new feature.
Projects
None yet
Development

No branches or pull requests

6 participants