Skip to content

correct logic for database backup use of single-transaction#14925

Merged
timkelty merged 1 commit intocraftcms:bugfix/single-transactionfrom
fortrabbit:bugfix/single-transaction
May 2, 2024
Merged

correct logic for database backup use of single-transaction#14925
timkelty merged 1 commit intocraftcms:bugfix/single-transactionfrom
fortrabbit:bugfix/single-transaction

Conversation

@erinbit
Copy link
Copy Markdown
Contributor

@erinbit erinbit commented May 2, 2024

Description

The recent release of 4.9.0 and 5.1.0 has a bug which makes MySQL backups fail. Because in this commit, the logic of when to use --single-transaction was reversed.
97da6ca#diff-83d39a4592b31403e3830a3100e22c596322d8dd9a240e07f6ecf173ec1d7bb7

Breaking change in detail

Before the commit if mysql version is newer than 5.7.41 or 8.0.32 results in FALSE

if (($isMySQL5 && version_compare($serverVersion, '5.7.41', '>=')) ||
     ($isMySQL8 && version_compare($serverVersion, '8.0.32', '>='))) {
     $useSingleTransaction = false;
 }

After the commit if mysql version is newer than 5.7.41 or 8.0.32 results in TRUE

 $useSingleTransaction =
     ($isMySQL5 && version_compare($serverVersion, '5.7.41', '>=')) ||
     ($isMySQL8 && version_compare($serverVersion, '8.0.32', '>='));

Why do we even check MySQL versions?

This check is done because after those versions, mysqldump will require the FLUSH TABLES privileges to run, which shared hosting usually will not give, as it is considered system privileges. Thus breaking any use of mysqldump. (See #12557)

Related issues

This fixes: #14922

@timkelty
Copy link
Copy Markdown
Contributor

timkelty commented May 2, 2024

@erinbit doh! Thanks so much for catching this 😍

@timkelty timkelty changed the base branch from 4.x to bugfix/single-transaction May 2, 2024 17:05
@timkelty timkelty merged commit d0f30af into craftcms:bugfix/single-transaction May 2, 2024
timkelty added a commit that referenced this pull request May 2, 2024
* correct logic for database backup single-transaction (#14925)
* Changelog
* Simplify condition
* Changelog tweak

---------

Co-authored-by: Erin <[email protected]>
@brandonkelly
Copy link
Copy Markdown
Member

Craft 4.9.1 and 5.1.1 are out with that fix. Thanks again @erinbit!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants