From 328435fccdbc0666b2ebec2f116af8b1a7725437 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Wed, 28 Mar 2018 20:21:53 +0800 Subject: [PATCH 1/5] *: modify 'session transaction isolation level' to take effect once 'session transaction isolation level' is not the same with 'session session transaction isolation level', it should take effect only once to the next transaction. --- parser/parser.y | 9 ++++++++- session/session.go | 12 +++++++++++- session/session_fail_test.go | 19 +++++++++++++++++++ sessionctx/variable/session.go | 2 ++ sessionctx/variable/sysvar.go | 1 + 5 files changed, 41 insertions(+), 2 deletions(-) diff --git a/parser/parser.y b/parser/parser.y index a14b3cccd57e6..fa487d8ab6c2b 100644 --- a/parser/parser.y +++ b/parser/parser.y @@ -4556,7 +4556,14 @@ SetStmt: } | "SET" "TRANSACTION" TransactionChars { - $$ = &ast.SetStmt{Variables: $3.([]*ast.VariableAssignment)} + assigns := $3.([]*ast.VariableAssignment) + for i:=0; i 0 { r.autoIncrementIDs = r.autoIncrementIDs[:0] diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index c5fc184904bae..218624af71a90 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -628,6 +628,7 @@ var defaultSysVars = []*SysVar{ {ScopeSession, TIDBMemQuotaTopn, strconv.FormatInt(DefTiDBMemQuotaTopn, 10)}, {ScopeSession, TIDBMemQuotaIndexLookupReader, strconv.FormatInt(DefTiDBMemQuotaIndexLookupReader, 10)}, {ScopeSession, TiDBEnableStreaming, "0"}, + {ScopeSession, "tx_isolation_one_shot", ""}, /* The following variable is defined as session scope but is actually server scope. */ {ScopeSession, TiDBGeneralLog, strconv.Itoa(DefTiDBGeneralLog)}, {ScopeSession, TiDBConfig, ""}, From 4f8adab958b0fd9c2391ad169300673dbe97e321 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Mon, 2 Apr 2018 15:32:39 +0800 Subject: [PATCH 2/5] fix all --- distsql/request_builder.go | 8 +++++++- session/session.go | 20 ++++++++++---------- session/session_fail_test.go | 19 ------------------- session/session_test.go | 21 +++++++++++++++++++++ sessionctx/variable/session.go | 8 ++++++-- 5 files changed, 44 insertions(+), 32 deletions(-) diff --git a/distsql/request_builder.go b/distsql/request_builder.go index 328977a98247b..a703f7fbb8202 100644 --- a/distsql/request_builder.go +++ b/distsql/request_builder.go @@ -125,7 +125,13 @@ func (builder *RequestBuilder) SetKeepOrder(order bool) *RequestBuilder { } func (builder *RequestBuilder) getIsolationLevel(sv *variable.SessionVars) kv.IsoLevel { - isoLevel, _ := sv.GetSystemVar(variable.TxnIsolation) + var isoLevel string + if sv.TxnIsolationLevelOneShot == 2 { + isoLevel, _ = sv.GetSystemVar("tx_isolation_one_shot") + } + if isoLevel == "" { + isoLevel, _ = sv.GetSystemVar(variable.TxnIsolation) + } if isoLevel == ast.ReadCommitted { return kv.RC } diff --git a/session/session.go b/session/session.go index 455703da04557..479bfdad22631 100644 --- a/session/session.go +++ b/session/session.go @@ -350,6 +350,15 @@ func (s *session) doCommitWithRetry(ctx context.Context) error { metrics.StatementPerTransaction.WithLabelValues(metrics.RetLabel(err)).Observe(float64(counter)) metrics.TransactionDuration.WithLabelValues(metrics.RetLabel(err)).Observe(float64(duration)) s.cleanRetryInfo() + fmt.Println("这里重置了...") + switch s.sessionVars.TxnIsolationLevelOneShot { + case 0: + case 1: + s.sessionVars.TxnIsolationLevelOneShot = 2 + case 2: + s.sessionVars.TxnIsolationLevelOneShot = 0 + terror.Log(s.sessionVars.SetSystemVar("tx_isolation_one_shot", "")) + } if err != nil { log.Warnf("[%d] finished txn:%v, %v", s.sessionVars.ConnectionID, s.txn, err) return errors.Trace(err) @@ -1331,16 +1340,7 @@ func (s *session) ActivePendingTxn() error { } s.sessionVars.TxnCtx.StartTS = s.txn.StartTS() - // Move tx_isolation_one_shot to RetryInfo, so it works even transaction retries. - if isoLevelOneShot, ok := s.sessionVars.GetSystemVar("tx_isolation_one_shot"); ok { - terror.Log(s.sessionVars.SetSystemVar("tx_isolation_one_shot", "")) - s.sessionVars.RetryInfo.IsolationOneShot = isoLevelOneShot - } - // Check tx_isolation_one_shot first, then check tx_isolation session variable. - isoLevel := s.sessionVars.RetryInfo.IsolationOneShot - if isoLevel == "" { - isoLevel, _ = s.sessionVars.GetSystemVar(variable.TxnIsolation) - } + isoLevel, _ := s.sessionVars.GetSystemVar(variable.TxnIsolation) if isoLevel == ast.ReadCommitted { s.txn.SetOption(kv.IsolationLevel, kv.RC) } diff --git a/session/session_fail_test.go b/session/session_fail_test.go index 7c082e95ca76d..61eff24744fc9 100644 --- a/session/session_fail_test.go +++ b/session/session_fail_test.go @@ -32,22 +32,3 @@ func (s *testSessionSuite) TestFailStatementCommit(c *C) { c.Assert(err, NotNil) tk.MustQuery(`select * from t`).Check(testkit.Rows()) } - -func (s *testSessionSuite) TestSetTransactionIsolationOneShot(c *C) { - tk1 := testkit.NewTestKitWithInit(c, s.store) - tk1.MustExec("create table t (k int, v int)") - tk1.MustExec("insert t values (1, 42)") - tk1.MustExec("set transaction isolation level read committed") - - tk2 := testkit.NewTestKitWithInit(c, s.store) - gofail.Enable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult", `return("keyError")`) - tk2.Exec("update t set v = 43 where k = 1") - gofail.Disable("github.com/pingcap/tidb/store/mockstore/mocktikv/rpcCommitResult") - - // In RC isolation level, the request meet lock and skip it, get 42. - tk1.MustQuery("select v from t where k = 1").Check(testkit.Rows("42")) - - // set transaction isolation level read committed take effect just one time. - tk2.MustExec("update t set v = 43 where k = 1") - tk1.MustQuery("select v from t where k = 1").Check(testkit.Rows("43")) -} diff --git a/session/session_test.go b/session/session_test.go index 63a8170a66710..cefad5c9711a7 100644 --- a/session/session_test.go +++ b/session/session_test.go @@ -1975,3 +1975,24 @@ func (s *testSessionSuite) TestRollbackOnCompileError(c *C) { } c.Assert(recoverErr, IsTrue) } + +func (s *testSessionSuite) TestSetTransactionIsolationOneShot(c *C) { + tk := testkit.NewTestKitWithInit(c, s.store) + tk.MustExec("create table t (k int, v int)") + tk.MustExec("insert t values (1, 42)") + tk.MustExec("set transaction isolation level read committed") + + // Check isolation level is set to read committed. + ctx := context.WithValue(context.Background(), "CheckSelectRequestHook", func(req *kv.Request) { + c.Assert(req.IsolationLevel, Equals, kv.RC) + }) + fmt.Println("雪融之前") + tk.Se.Execute(ctx, "select * from t where k = 1") + fmt.Println("雪融之前sdjfasdljfasdklf") + + // Check it just take effect for one time. + ctx = context.WithValue(context.Background(), "CheckSelectRequestHook", func(req *kv.Request) { + c.Assert(req.IsolationLevel, Equals, kv.SI) + }) + tk.Se.Execute(ctx, "select * from t where k = 1") +} diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 73d409df0cd24..ab8fbb56860e0 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -49,12 +49,10 @@ type RetryInfo struct { DroppedPreparedStmtIDs []uint32 currRetryOff int autoIncrementIDs []int64 - IsolationOneShot string } // Clean does some clean work. func (r *RetryInfo) Clean() { - r.IsolationOneShot = "" r.currRetryOff = 0 if len(r.autoIncrementIDs) > 0 { r.autoIncrementIDs = r.autoIncrementIDs[:0] @@ -164,6 +162,10 @@ type SessionVars struct { // Should be reset on transaction finished. TxnCtx *TransactionContext + // TxnIsolationLevelOneShot is used to implements "set transaction isolation level ..." + // Value 0 means default, 1 means set but not used, 2 means used. + TxnIsolationLevelOneShot int + // Following variables are special for current session. Status uint16 @@ -441,6 +443,8 @@ func (s *SessionVars) deleteSystemVar(name string) error { // SetSystemVar sets the value of a system variable. func (s *SessionVars) SetSystemVar(name string, val string) error { switch name { + case "tx_isolation_one_shot": + s.TxnIsolationLevelOneShot = 1 case TimeZone: tz, err := parseTimeZone(val) if err != nil { From b5d25ac1df3db8067ecc24753cdea7ec8f9f224b Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Mon, 2 Apr 2018 15:54:27 +0800 Subject: [PATCH 3/5] address comment --- distsql/request_builder.go | 4 ++-- session/session.go | 19 ++++++++++--------- session/session_test.go | 2 -- sessionctx/variable/session.go | 12 +++++++++--- 4 files changed, 21 insertions(+), 16 deletions(-) diff --git a/distsql/request_builder.go b/distsql/request_builder.go index a703f7fbb8202..83161d0f25dd7 100644 --- a/distsql/request_builder.go +++ b/distsql/request_builder.go @@ -126,8 +126,8 @@ func (builder *RequestBuilder) SetKeepOrder(order bool) *RequestBuilder { func (builder *RequestBuilder) getIsolationLevel(sv *variable.SessionVars) kv.IsoLevel { var isoLevel string - if sv.TxnIsolationLevelOneShot == 2 { - isoLevel, _ = sv.GetSystemVar("tx_isolation_one_shot") + if sv.TxnIsolationLevelOneShot.State == 2 { + isoLevel = sv.TxnIsolationLevelOneShot.Value } if isoLevel == "" { isoLevel, _ = sv.GetSystemVar(variable.TxnIsolation) diff --git a/session/session.go b/session/session.go index 6e4c0202b56ac..9cc2335565351 100644 --- a/session/session.go +++ b/session/session.go @@ -350,15 +350,17 @@ func (s *session) doCommitWithRetry(ctx context.Context) error { metrics.StatementPerTransaction.WithLabelValues(metrics.RetLabel(err)).Observe(float64(counter)) metrics.TransactionDuration.WithLabelValues(metrics.RetLabel(err)).Observe(float64(duration)) s.cleanRetryInfo() - fmt.Println("这里重置了...") - switch s.sessionVars.TxnIsolationLevelOneShot { - case 0: - case 1: - s.sessionVars.TxnIsolationLevelOneShot = 2 - case 2: - s.sessionVars.TxnIsolationLevelOneShot = 0 - terror.Log(s.sessionVars.SetSystemVar("tx_isolation_one_shot", "")) + + if isoLevelOneShot := &s.sessionVars.TxnIsolationLevelOneShot; isoLevelOneShot.State != 0 { + switch isoLevelOneShot.State { + case 1: + isoLevelOneShot.State = 2 + case 2: + isoLevelOneShot.State = 0 + isoLevelOneShot.Value = "" + } } + if err != nil { log.Warnf("[%d] finished txn:%v, %v", s.sessionVars.ConnectionID, s.txn, err) return errors.Trace(err) @@ -1340,7 +1342,6 @@ func (s *session) ActivePendingTxn() error { return errors.Trace(err) } s.sessionVars.TxnCtx.StartTS = s.txn.StartTS() - isoLevel, _ := s.sessionVars.GetSystemVar(variable.TxnIsolation) if isoLevel == ast.ReadCommitted { s.txn.SetOption(kv.IsolationLevel, kv.RC) diff --git a/session/session_test.go b/session/session_test.go index 066b67ced5a48..b3e1c24fb33b4 100644 --- a/session/session_test.go +++ b/session/session_test.go @@ -1986,9 +1986,7 @@ func (s *testSessionSuite) TestSetTransactionIsolationOneShot(c *C) { ctx := context.WithValue(context.Background(), "CheckSelectRequestHook", func(req *kv.Request) { c.Assert(req.IsolationLevel, Equals, kv.RC) }) - fmt.Println("雪融之前") tk.Se.Execute(ctx, "select * from t where k = 1") - fmt.Println("雪融之前sdjfasdljfasdklf") // Check it just take effect for one time. ctx = context.WithValue(context.Background(), "CheckSelectRequestHook", func(req *kv.Request) { diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 2896ea79a975a..1e2e5e3cea5a4 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -163,8 +163,13 @@ type SessionVars struct { TxnCtx *TransactionContext // TxnIsolationLevelOneShot is used to implements "set transaction isolation level ..." - // Value 0 means default, 1 means set but not used, 2 means used. - TxnIsolationLevelOneShot int + TxnIsolationLevelOneShot struct { + // state 0 means default + // state 1 means it's set in current transaction. + // state 2 means it should be used in current transaction. + State int + Value string + } // Following variables are special for current session. @@ -453,7 +458,8 @@ func (s *SessionVars) deleteSystemVar(name string) error { func (s *SessionVars) SetSystemVar(name string, val string) error { switch name { case "tx_isolation_one_shot": - s.TxnIsolationLevelOneShot = 1 + s.TxnIsolationLevelOneShot.State = 1 + s.TxnIsolationLevelOneShot.Value = val case TimeZone: tz, err := parseTimeZone(val) if err != nil { From 070a65617f63090a759b6fc5f9b33b225ed294f2 Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Mon, 2 Apr 2018 20:13:39 +0800 Subject: [PATCH 4/5] address comment --- executor/executor.go | 49 +++++++++++++++++++++++--------------------- executor/set.go | 3 +++ 2 files changed, 29 insertions(+), 23 deletions(-) diff --git a/executor/executor.go b/executor/executor.go index 65f7bc068b6bc..a12f0b261383b 100644 --- a/executor/executor.go +++ b/executor/executor.go @@ -66,29 +66,31 @@ var ( // Error instances. var ( - ErrUnknownPlan = terror.ClassExecutor.New(codeUnknownPlan, "Unknown plan") - ErrPrepareMulti = terror.ClassExecutor.New(codePrepareMulti, "Can not prepare multiple statements") - ErrPrepareDDL = terror.ClassExecutor.New(codePrepareDDL, "Can not prepare DDL statements") - ErrPasswordNoMatch = terror.ClassExecutor.New(CodePasswordNoMatch, "Can't find any matching row in the user table") - ErrResultIsEmpty = terror.ClassExecutor.New(codeResultIsEmpty, "result is empty") - ErrBuildExecutor = terror.ClassExecutor.New(codeErrBuildExec, "Failed to build executor") - ErrBatchInsertFail = terror.ClassExecutor.New(codeBatchInsertFail, "Batch insert failed, please clean the table and try again.") - ErrWrongValueCountOnRow = terror.ClassExecutor.New(codeWrongValueCountOnRow, "Column count doesn't match value count at row %d") - ErrPasswordFormat = terror.ClassExecutor.New(codePasswordFormat, "The password hash doesn't have the expected format. Check if the correct password algorithm is being used with the PASSWORD() function.") + ErrUnknownPlan = terror.ClassExecutor.New(codeUnknownPlan, "Unknown plan") + ErrPrepareMulti = terror.ClassExecutor.New(codePrepareMulti, "Can not prepare multiple statements") + ErrPrepareDDL = terror.ClassExecutor.New(codePrepareDDL, "Can not prepare DDL statements") + ErrPasswordNoMatch = terror.ClassExecutor.New(CodePasswordNoMatch, "Can't find any matching row in the user table") + ErrResultIsEmpty = terror.ClassExecutor.New(codeResultIsEmpty, "result is empty") + ErrBuildExecutor = terror.ClassExecutor.New(codeErrBuildExec, "Failed to build executor") + ErrBatchInsertFail = terror.ClassExecutor.New(codeBatchInsertFail, "Batch insert failed, please clean the table and try again.") + ErrWrongValueCountOnRow = terror.ClassExecutor.New(codeWrongValueCountOnRow, "Column count doesn't match value count at row %d") + ErrPasswordFormat = terror.ClassExecutor.New(codePasswordFormat, "The password hash doesn't have the expected format. Check if the correct password algorithm is being used with the PASSWORD() function.") + ErrCantChangeTxCharacteristics = terror.ClassExecutor.New(codeErrCantChangeTxCharacteristics, "Transaction characteristics can't be changed while a transaction is in progress") ) // Error codes. const ( - codeUnknownPlan terror.ErrCode = 1 - codePrepareMulti terror.ErrCode = 2 - codePrepareDDL terror.ErrCode = 7 - codeResultIsEmpty terror.ErrCode = 8 - codeErrBuildExec terror.ErrCode = 9 - codeBatchInsertFail terror.ErrCode = 10 - CodePasswordNoMatch terror.ErrCode = 1133 // MySQL error code - CodeCannotUser terror.ErrCode = 1396 // MySQL error code - codeWrongValueCountOnRow terror.ErrCode = 1136 // MySQL error code - codePasswordFormat terror.ErrCode = 1827 // MySQL error code + codeUnknownPlan terror.ErrCode = 1 + codePrepareMulti terror.ErrCode = 2 + codePrepareDDL terror.ErrCode = 7 + codeResultIsEmpty terror.ErrCode = 8 + codeErrBuildExec terror.ErrCode = 9 + codeBatchInsertFail terror.ErrCode = 10 + CodePasswordNoMatch terror.ErrCode = 1133 // MySQL error code + CodeCannotUser terror.ErrCode = 1396 // MySQL error code + codeWrongValueCountOnRow terror.ErrCode = 1136 // MySQL error code + codePasswordFormat terror.ErrCode = 1827 // MySQL error code + codeErrCantChangeTxCharacteristics terror.ErrCode = 1568 ) // Row represents a result set row, it may be returned from a table, a join, or a projection. @@ -604,10 +606,11 @@ func init() { } } tableMySQLErrCodes := map[terror.ErrCode]uint16{ - CodeCannotUser: mysql.ErrCannotUser, - CodePasswordNoMatch: mysql.ErrPasswordNoMatch, - codeWrongValueCountOnRow: mysql.ErrWrongValueCountOnRow, - codePasswordFormat: mysql.ErrPasswordFormat, + CodeCannotUser: mysql.ErrCannotUser, + CodePasswordNoMatch: mysql.ErrPasswordNoMatch, + codeWrongValueCountOnRow: mysql.ErrWrongValueCountOnRow, + codePasswordFormat: mysql.ErrPasswordFormat, + codeErrCantChangeTxCharacteristics: mysql.ErrCantChangeTxCharacteristics, } terror.ErrClassToMySQLCodes[terror.ClassExecutor] = tableMySQLErrCodes } diff --git a/executor/set.go b/executor/set.go index ee4f9fcff943e..1883a5de32f71 100644 --- a/executor/set.go +++ b/executor/set.go @@ -150,6 +150,9 @@ func (e *SetExecutor) setSysVariable(name string, v *expression.VarAssignment) e return errors.Trace(err) } oldSnapshotTS := sessionVars.SnapshotTS + if name == "tx_isolation_one_shot" && sessionVars.InTxn() { + return errors.Trace(ErrCantChangeTxCharacteristics) + } err = variable.SetSessionSystemVar(sessionVars, name, value) if err != nil { return errors.Trace(err) From 67e65e3171712d1c5b6fa00632c8e0ef559653ae Mon Sep 17 00:00:00 2001 From: tiancaiamao Date: Wed, 4 Apr 2018 14:40:44 +0800 Subject: [PATCH 5/5] address comment --- executor/set.go | 2 +- session/session_test.go | 5 +++++ sessionctx/variable/session.go | 3 ++- sessionctx/variable/sysvar.go | 2 +- 4 files changed, 9 insertions(+), 3 deletions(-) diff --git a/executor/set.go b/executor/set.go index 1883a5de32f71..c173fe1564e2e 100644 --- a/executor/set.go +++ b/executor/set.go @@ -150,7 +150,7 @@ func (e *SetExecutor) setSysVariable(name string, v *expression.VarAssignment) e return errors.Trace(err) } oldSnapshotTS := sessionVars.SnapshotTS - if name == "tx_isolation_one_shot" && sessionVars.InTxn() { + if name == variable.TxnIsolationOneShot && sessionVars.InTxn() { return errors.Trace(ErrCantChangeTxCharacteristics) } err = variable.SetSessionSystemVar(sessionVars, name, value) diff --git a/session/session_test.go b/session/session_test.go index 0a131807b2da8..8ca018e0d5339 100644 --- a/session/session_test.go +++ b/session/session_test.go @@ -1993,6 +1993,11 @@ func (s *testSessionSuite) TestSetTransactionIsolationOneShot(c *C) { c.Assert(req.IsolationLevel, Equals, kv.SI) }) tk.Se.Execute(ctx, "select * from t where k = 1") + + // Can't change isolation level when it's inside a transaction. + tk.MustExec("begin") + _, err := tk.Se.Execute(ctx, "set transaction isolation level read committed") + c.Assert(err, NotNil) } func (s *testSessionSuite) TestDBUserNameLength(c *C) { diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 1e2e5e3cea5a4..d0e0ad4e1b556 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -457,7 +457,7 @@ func (s *SessionVars) deleteSystemVar(name string) error { // SetSystemVar sets the value of a system variable. func (s *SessionVars) SetSystemVar(name string, val string) error { switch name { - case "tx_isolation_one_shot": + case TxnIsolationOneShot: s.TxnIsolationLevelOneShot.State = 1 s.TxnIsolationLevelOneShot.Value = val case TimeZone: @@ -548,6 +548,7 @@ const ( MaxAllowedPacket = "max_allowed_packet" TimeZone = "time_zone" TxnIsolation = "tx_isolation" + TxnIsolationOneShot = "tx_isolation_one_shot" ) // TableDelta stands for the changed count for one table. diff --git a/sessionctx/variable/sysvar.go b/sessionctx/variable/sysvar.go index 0d412577a684b..7f42f4bc07542 100644 --- a/sessionctx/variable/sysvar.go +++ b/sessionctx/variable/sysvar.go @@ -631,7 +631,7 @@ var defaultSysVars = []*SysVar{ {ScopeSession, TIDBMemQuotaIndexLookupJoin, strconv.FormatInt(DefTiDBMemQuotaIndexLookupJoin, 10)}, {ScopeSession, TIDBMemQuotaNestedLoopApply, strconv.FormatInt(DefTiDBMemQuotaNestedLoopApply, 10)}, {ScopeSession, TiDBEnableStreaming, "0"}, - {ScopeSession, "tx_isolation_one_shot", ""}, + {ScopeSession, TxnIsolationOneShot, ""}, /* The following variable is defined as session scope but is actually server scope. */ {ScopeSession, TiDBGeneralLog, strconv.Itoa(DefTiDBGeneralLog)}, {ScopeSession, TiDBConfig, ""},