gh-ost icon indicating copy to clipboard operation
gh-ost copied to clipboard

Regression in master: SET transaction_isolation = 'repeatable_read';

Open morgo opened this issue 2 years ago • 7 comments

This is the place to report a bug, ask a question, or suggest an enhancement.

In this PR https://github.com/github/gh-ost/commit/da3514253f5cd848c1cbcaad63ad794427ed1833

GetDBUri() will execute a statement such as the following:

set transaction_isolation = 'repeatable-read';

This variable was only introduced in MySQL 5.7.20, so if you are using an earlier version such as what Aurora 5.7 is, it will result in an error:

mysql> set transaction_isolation = 'repeatable-read';
ERROR 1193 (HY000): Unknown system variable 'transaction_isolation'

Instead the statement needs to be set tx_isolation = 'repeatable-read', but this statement is not supported by MySQL 8.0. So some version-specific logic is probably required.

morgo avatar Mar 15 '23 21:03 morgo

I encountered this because I use MariaDB 10.5 (not my choice, and I know it's not officially supported by gh-ost).

Thought I'd mention that the commit that introduced this change is actually 3f44e043, not the one mentioned by OP (although it's the same line OP saw).

For my needs, I edited go/mysql/connection.go, replaced transaction_isolation with tx_isolation, and rebuilt gh-ost from source.

Uplink03 avatar Dec 05 '23 09:12 Uplink03

For those trapped on MySQL 5.7, downgrading to gh-ost 1.1.5 works:

https://github.com/github/gh-ost/releases/tag/v1.1.5

sj26 avatar Dec 11 '23 08:12 sj26

What is the solution to this? Version 1.1.6 is unusable on MariaDB, and 1.1.5 can't convert tables to UTF-8 space, per #1158...

suwalski avatar Jul 04 '24 14:07 suwalski

What is the solution to this? Version 1.1.6 is unusable on MariaDB, and 1.1.5 can't convert tables to UTF-8 space, per #1158...

My solution was to hexedit the gh-ost binary like so:

0038E800   73 65 74 3D  75 74 66 2D  38 74 72 61  6E 73 61 63  set=utf-8transac
0038E810   74 69 6F 6E  5F 69 73 6F  6C 61 74 69  6F 6E 3D 25  tion_isolation=%
0038E820   71 75 6E 65  78 70 65 63  74 65 64 20  62 75 66 66  qunexpected buff

to:

0038E800   73 65 74 3D  75 74 66 2D  38 74 78 5F  69 73 6F 6C  set=utf-8tx_isol
0038E810   61 74 69 6F  6E 20 20 20  20 20 20 20  20 20 3D 25  ation         =%
0038E820   71 75 6E 65  78 70 65 63  74 65 64 20  62 75 66 66  qunexpected buff

But that's a pretty lame workaround, though I was able to do the character set migration. Would love to see this addressed, even if it's with a command-line parameter.

suwalski avatar Jul 04 '24 14:07 suwalski

@morgo / @Uplink03 / @sj26 / @suwalski this should be resolved by https://github.com/github/gh-ost/pull/1441. If you could validate this before/after that's merged that would be helpful 🙇

EDIT: you'll need to set --transaction-isolation READ-COMMITTED on the command line

timvaillancourt avatar Aug 14 '24 22:08 timvaillancourt

@morgo / @Uplink03 / @sj26 / @suwalski this should be resolved by https://github.com/github/gh-ost/pull/1441. If you could validate this before/after that's merged that would be helpful 🙇

EDIT: you'll need to set --transaction-isolation READ-COMMITTED on the command line

I'm away from the computer so I can't build the code, but I had a look at the diff for that PR and unless I didn't see it, it's not addressing this issue.

The problem isn't that the transaction isolation value was wrong or hard coded or something, but that the settings variable name was changed between database server versions. gh-ost is using the new name, and so far the only way to make it use the old name is to patch the file I mentioned in my comment.

This is what I said:

I edited go/mysql/connection.go, replaced transaction_isolation with tx_isolation, and rebuilt gh-ost from source.

If that was addressed, it's not in the mentioned PR.

As far as I can get on my mobile GitHub app right now, that file hasn't changed in two years.

Uplink03 avatar Aug 16 '24 13:08 Uplink03

I'll try to have a think about adding a parameter that can specify the name of the setting on request or in config. If I come up with an idea I like, and nobody does it before I do (timeline undetermined), I should submit a PR. It shouldn't be too much work, right? (Famous last words)

Uplink03 avatar Aug 16 '24 13:08 Uplink03