search

ifsnop / mysqldump-php

PHP version of mysqldump cli that comes with MySQL
https://github.com/ifsnop/mysqldump-php
GNU General Public License v3.0
1.25k stars 300 forks source link

Mysqldump-php generates inconsistent snapshot of Database. #267

Closed sherifabdlnaby closed 1 year ago

sherifabdlnaby commented 1 year ago

I originally reported this in gdpr-dump repo https://github.com/Smile-SA/gdpr-dump/issues/86 but I traced the bug Mysqldump-php.

When working with large databases, we noticed that generated dump is inconsistent. Some tables will include rows that were inserted after the dump was initiated. This problem became a dealbreaker with bigger databases.

By tracing this further, it turns out that Mysqldump-php starts a transaction per table and not for the entire dump process. This is unlike the behavior of the mysqldump command.

ifsnop commented 1 year ago

By tracing this further, it turns out that Mysqldump-php starts a transaction per table https://github.com/ifsnop/mysqldump-php/blob/master/src/Ifsnop/Mysqldump/Mysqldump.php#L1219 and not for the entire dump process. This is unlike the behavior of the mysqldump command.

Thanks for your report. Let me check and try to provide a fix for this. Probably this would lead to a global lock of the database, stopping all inserts. Are you available to test fixes?

sherifabdlnaby commented 1 year ago

@ifsnop In the mysqldump command you don't have to lock the entire database if you pass --single-transaction (and use InnoDB). Then mysqldump command will basically start a transaction at the beginning of the dump process.

sherifabdlnaby commented 1 year ago

@ifsnop Yes, we can help testing this!

ifsnop commented 1 year ago

@ifsnop https://github.com/ifsnop In the mysqldump command you don't have to lock the entire database if you pass --single-transaction (and use InnoDB). Then mysqldump command will basically start a transaction at the beginning of the dump process.

This is already implemented. The switch "single-transaction" is true in the default configuration. Could you check if you modified this option?

sherifabdlnaby commented 1 year ago

@ifsnop It's implemented but I am reporting that it's wrongly implemented. The transactions are applied per table instead of for the entire dump.

ifsnop commented 1 year ago

Ok, understood. That part was written long ago.

@ifsnop https://github.com/ifsnop It's implemented but I am reporting that it's wrongly implemented. The transactions are applied per table instead of for the entire dump.

ifsnop commented 1 year ago

I have uploaded a fix:

https://github.com/ifsnop/mysqldump-php/tree/transaction-fix

sherifabdlnaby commented 1 year ago

@ifsnop We tested this fix internally and it is working. Can we get it merged ?

ifsnop commented 1 year ago

Done.