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

TRUNCATE and MySQL driver. #584

Open
martinarrieta opened this issue Jun 18, 2021 · 10 comments
Open

TRUNCATE and MySQL driver. #584

martinarrieta opened this issue Jun 18, 2021 · 10 comments
Labels
database Updates database drivers

Comments

@martinarrieta
Copy link
Contributor

Describe the Bug
MySQL does not support DDL statements inside a transaction, so, when you drop all the rows inside the migrations table with a TRUNCATE, MySQL will automatically commit that transaction and create a new one for the insert that is after that.

Also, there are some issues with truncate and MySQL 5.7 when you have a large buffer pool that is not solved yet. The bug report was reported for the version 5.5.28 but in the TRUNCATE documentation say that it is still affect the latest 5.7 version.

Steps to Reproduce
Steps to reproduce the behavior:

  1. Setup golang-migrate inside a large MySQL instance (buffer pool bigger than 150G)
  2. Run the migrations multiple times
  3. See how MySQL stalls.

Expected Behavior
I'd like to change that TRUNCATE for a DELETE, which is transactional and for a small number of rows it is not an issue.

Migrate Version
v4.14.1

Loaded Database Drivers
MySQL

Additional context
Add any other context about the problem here.

@dhui
Copy link
Member

dhui commented Jun 23, 2021

Thanks for reporting the issue!

For reference, the MySQL 8 docs:

In MySQL 5.7 and earlier, on a system with a large buffer pool and innodb_adaptive_hash_index enabled, a TRUNCATE TABLE operation could cause a temporary drop in system performance due to an LRU scan that occurred when removing the table's adaptive hash index entries (Bug #68184). The remapping of TRUNCATE TABLE to DROP TABLE and CREATE TABLE in MySQL 8.0 avoids the problematic LRU scan.

@m00dawg
Copy link

m00dawg commented Jun 23, 2021

Other thing to point out though, even with that fix, TRUNCATE also messes around with the filesystem more than a DELETE. We ran into this with the migrations table itself, which only contained a single row. That means, even if TRUNCATE "works" it will be slower than a DELETE for a single row and doesn't provide any other benefits (like reclaiming disk space) as a result. I think it's worth of evaluating when/where DELETE vs TRUNCATE should be used in the tool (and I'd definitely recommend DELETE in the case of the migration table, or any other tables with a mere handful of rows).

@dhui
Copy link
Member

dhui commented Jun 23, 2021

What about replacing TRUNCATE with DROP TABLE + CREATE TABLE, which is what MySQL 8 does according to the docs?

@m00dawg
Copy link

m00dawg commented Jun 23, 2021

That'll avoid the issue pre-8.0 (e.g. 5.7) though still does file operations you can avoid with the DELETE. I should be clear, DELETE is much worse performance-wise when the table is large, so I'm mostly referring to the performance differences for small tables (like the migration table)

@martinarrieta
Copy link
Contributor Author

Also, CREATE and DROP table are not transactional in MySQL (all versions), so that means that if you use any DDL, MySQL will automatically commit the transaction and it will leave the insert that is after that statement in a different transaction.

@dhui
Copy link
Member

dhui commented Jun 23, 2021

Given that the migration table is small, we could run DELETE + INSERT in the same transaction.
The advisory lock should prevent any race conditions. I don't think the transaction provides much value unless the isolation level is SERIALIZABLE.

@dhui
Copy link
Member

dhui commented Jun 23, 2021

@martinarrieta also mentioned:

Also, delete will create a lock over all the rows in the table, but not a TABLE LOCK, if that is what you need (lock the table), you could use LOCK TABLES instead but I don't think that it is really needed because all other transactions will be waiting for the rowlock anyway.

Let's try running DELETE + INSERT in a single SERIALIZABLE transaction.

@cinaglia
Copy link

I ran into this issue today. Our database migration tool relied on there always being records in the schema_migrations table, which was not the case for a very brief period (between the TRUNCATE and the INSERT statements).

The solution proposed in #585 seems like a reasonable one to me. Is there anything blocking it from being merged? Let us know if there anything we can do to help move things along.

@dhui
Copy link
Member

dhui commented Aug 20, 2021

@cinaglia the transaction needs to be made SERIALIZABLE

@dhui
Copy link
Member

dhui commented Dec 18, 2021

Fixed by #656. Will close this issue after the next release. In the meanwhile, if you're impacted by this issue, use the master branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database Updates database drivers
Projects
None yet
Development

No branches or pull requests

5 participants