Transaction isolation levels #619
Conversation
| if err != nil { | ||
| return nil, err | ||
| } | ||
| mc.finish() |
methane
Jun 14, 2017
Member
You shouldn't use mc.finish() here. It should be paried with mc.watchCancel().
Move whole this if statement after watchCancel(), and remove mc.finish().
You shouldn't use mc.finish() here. It should be paried with mc.watchCancel().
Move whole this if statement after watchCancel(), and remove mc.finish().
shogo82148
Jun 14, 2017
Contributor
Move whole this if statement after watchCancel(), and remove mc.finish().
In addition, please don't forget to call mc.finish() before return nil, err.
mc.finish() must called before all return.
Move whole this if statement after
watchCancel(), and removemc.finish().
In addition, please don't forget to call mc.finish() before return nil, err.
mc.finish() must called before all return.
|
@methane @shogo82148 I've applied your comments. Do you guys think defering |
|
@zimnx not ok. ... but, calling select {
default:
case <-ctx.Done():
tx.Rollback()
return nil, ctx.Err()
}this code comes from ctxutil.go. @methane How do you think which is better calling |
| @@ -468,3 +469,100 @@ func TestContextCancelBegin(t *testing.T) { | |||
| } | |||
| }) | |||
| } | |||
|
|
|||
| func TestContextBeginIsolationLevel(t *testing.T) { | |||
shogo82148
Jun 14, 2017
Contributor
nice test👍
but, this test can be written more concisely, can't it?
for example
tx1, err := dbt.db.BeginTx(ctx, &sql.TxOptions{
Isolation: sql.LevelRepeatableRead,
})
tx2, err := dbt.db.BeginTx(ctx, &sql.TxOptions{
Isolation: sql.LevelReadCommitted,
})
row := tx2.QueryRowContext(ctx, "SELECT COUNT(*) FROM test")
// check row.
_, err = tx1.ExecContext(ctx, "INSERT INTO test VALUES (1)")
row = tx2.QueryRowContext(ctx, "SELECT COUNT(*) FROM test")
// check row.
nice test
but, this test can be written more concisely, can't it?
for example
tx1, err := dbt.db.BeginTx(ctx, &sql.TxOptions{
Isolation: sql.LevelRepeatableRead,
})
tx2, err := dbt.db.BeginTx(ctx, &sql.TxOptions{
Isolation: sql.LevelReadCommitted,
})
row := tx2.QueryRowContext(ctx, "SELECT COUNT(*) FROM test")
// check row.
_, err = tx1.ExecContext(ctx, "INSERT INTO test VALUES (1)")
row = tx2.QueryRowContext(ctx, "SELECT COUNT(*) FROM test")
// check row.
zimnx
Jun 14, 2017
Author
Contributor
Done, thanks for advice.
Done, thanks for advice.
| @@ -61,6 +61,7 @@ Xiangyu Hu <xiangyu.hu at outlook.com> | |||
| Xiaobing Jiang <s7v7nislands at gmail.com> | |||
| Xiuming Chen <cc at cxm.cc> | |||
| Zhenye Xie <xiezhenye at gmail.com> | |||
| Maciej Zimnoch <[email protected]> | |||
methane
Jun 14, 2017
Member
Please insert your name to alphabetical order
Please insert your name to alphabetical order
|
@shogo82148 +1 to just remove it. |
|
@methane Done. Do you have any more comments? I would like to squash commits. |
|
No need to manual squash. We use "Squash and merge". I'll review whole pull request again in tomorrow. Please wait. |
|
Sure, thanks! |
| case <-ctx.Done(): | ||
| tx.Rollback() | ||
| return nil, ctx.Err() | ||
| } | ||
| return tx, err |
methane
Jun 14, 2017
Member
return mc.Begin() is preferred Go style.
return mc.Begin() is preferred Go style.
|
thanks for your opinion. |
|
Any news on this? Literally just got an error about this not working. |
Description
Support setting transaction isolation level
Currently supported isolation levels are:
Which are all available in InnoDB (https://dev.mysql.com/doc/refman/5.7/en/innodb-transaction-isolation-levels.html)
Checklist