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

Regression in master: SET transaction_isolation = 'repeatable_read'; #1262

Open
morgo opened this issue Mar 15, 2023 · 7 comments · May be fixed by #1441
Open

Regression in master: SET transaction_isolation = 'repeatable_read'; #1262

morgo opened this issue Mar 15, 2023 · 7 comments · May be fixed by #1441

Comments

@morgo
Copy link
Contributor

morgo commented Mar 15, 2023

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

In this PR da35142

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.

@Uplink03
Copy link

Uplink03 commented Dec 5, 2023

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.

@sj26
Copy link

sj26 commented Dec 11, 2023

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

peterbollen pushed a commit to dnovitski/gh-ost that referenced this issue Apr 3, 2024
peterbollen pushed a commit to dnovitski/gh-ost that referenced this issue Apr 3, 2024
peterbollen pushed a commit to dnovitski/gh-ost that referenced this issue Apr 3, 2024
peterbollen pushed a commit to dnovitski/gh-ost that referenced this issue Apr 3, 2024
@suwalski
Copy link

suwalski commented Jul 4, 2024

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
Copy link

suwalski commented Jul 4, 2024

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.

@timvaillancourt timvaillancourt linked a pull request Aug 14, 2024 that will close this issue
2 tasks
@timvaillancourt
Copy link
Collaborator

timvaillancourt commented Aug 14, 2024

@morgo / @Uplink03 / @sj26 / @suwalski this should be resolved by #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

@Uplink03
Copy link

@morgo / @Uplink03 / @sj26 / @suwalski this should be resolved by #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
Copy link

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)

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 a pull request may close this issue.

5 participants