From 3531df91b95270ad80c4f411e2bffe8d6aab0665 Mon Sep 17 00:00:00 2001 From: zimulala Date: Tue, 15 Sep 2015 10:24:20 +0800 Subject: [PATCH 01/10] stmt: set ServerStatusInTrans --- sessionctx/variable/session.go | 8 ++++++++ stmt/stmts/transaction.go | 3 +++ 2 files changed, 11 insertions(+) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 9344c9b49a..c90c1f307a 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -102,6 +102,14 @@ func (s *SessionVars) SetStatus(status uint16) { s.Status = status } +func (s *SessionVars) SetStatusInTrans(isInTrans bool) { + if isInTrans { + s.Status = s.Status | mysql.ServerStatusInTrans + return + } + s.Status = s.Status & (^mysql.ServerStatusInTrans) +} + // GetNextPreparedStmtID generates and return the next session scope prepared statement id func (s *SessionVars) GetNextPreparedStmtID() uint32 { s.preparedStmtID++ diff --git a/stmt/stmts/transaction.go b/stmt/stmts/transaction.go index 2b5c8e7a1c..1f9ea9a508 100644 --- a/stmt/stmts/transaction.go +++ b/stmt/stmts/transaction.go @@ -64,6 +64,7 @@ func (s *BeginStmt) Exec(ctx context.Context) (_ rset.Recordset, err error) { // the transaction with COMMIT or ROLLBACK. The autocommit mode then // reverts to its previous state. variable.GetSessionVars(ctx).DisableAutocommit = true + variable.GetSessionVars(ctx).SetStatusInTrans(true) return } @@ -97,6 +98,7 @@ func (s *CommitStmt) SetText(text string) { func (s *CommitStmt) Exec(ctx context.Context) (_ rset.Recordset, err error) { err = ctx.FinishTxn(false) variable.GetSessionVars(ctx).DisableAutocommit = false + variable.GetSessionVars(ctx).SetStatusInTrans(false) return } @@ -130,5 +132,6 @@ func (s *RollbackStmt) SetText(text string) { func (s *RollbackStmt) Exec(ctx context.Context) (_ rset.Recordset, err error) { err = ctx.FinishTxn(true) variable.GetSessionVars(ctx).DisableAutocommit = false + variable.GetSessionVars(ctx).SetStatusInTrans(false) return } From 2287542d64888e75b1b27a01d9917dae07d63751 Mon Sep 17 00:00:00 2001 From: zimulala Date: Tue, 15 Sep 2015 10:27:30 +0800 Subject: [PATCH 02/10] sessionctx: add ServerStatusInTrans and comments --- sessionctx/variable/session.go | 1 + 1 file changed, 1 insertion(+) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index c90c1f307a..c4783a1a74 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -102,6 +102,7 @@ func (s *SessionVars) SetStatus(status uint16) { s.Status = status } +// SetStatusInTrans sets the status flags about SERVER_STATUS_IN_TRANS func (s *SessionVars) SetStatusInTrans(isInTrans bool) { if isInTrans { s.Status = s.Status | mysql.ServerStatusInTrans From a11fa52306076e4299c411e9ad93228dc65a77a8 Mon Sep 17 00:00:00 2001 From: xia Date: Tue, 15 Sep 2015 15:00:53 +0800 Subject: [PATCH 03/10] sessionctx: update comments --- sessionctx/variable/session.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index c4783a1a74..02b9a92bbe 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -102,7 +102,7 @@ func (s *SessionVars) SetStatus(status uint16) { s.Status = status } -// SetStatusInTrans sets the status flags about SERVER_STATUS_IN_TRANS +// SetStatusInTrans sets the status flags about ServerStatusInTrans. func (s *SessionVars) SetStatusInTrans(isInTrans bool) { if isInTrans { s.Status = s.Status | mysql.ServerStatusInTrans From 88243211c402544e55d238910fbe9d36c53ef50b Mon Sep 17 00:00:00 2001 From: xia Date: Tue, 15 Sep 2015 16:09:10 +0800 Subject: [PATCH 04/10] sessionctx: update format --- sessionctx/variable/session.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index 02b9a92bbe..df789cb9ce 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -105,10 +105,10 @@ func (s *SessionVars) SetStatus(status uint16) { // SetStatusInTrans sets the status flags about ServerStatusInTrans. func (s *SessionVars) SetStatusInTrans(isInTrans bool) { if isInTrans { - s.Status = s.Status | mysql.ServerStatusInTrans + s.Status |= mysql.ServerStatusInTrans return } - s.Status = s.Status & (^mysql.ServerStatusInTrans) + s.Status &= (^mysql.ServerStatusInTrans) } // GetNextPreparedStmtID generates and return the next session scope prepared statement id From 39fcb95567c6810c6644957e33c2ad6594498618 Mon Sep 17 00:00:00 2001 From: xia Date: Tue, 15 Sep 2015 16:09:48 +0800 Subject: [PATCH 05/10] tidb: add SetStatusInTrans test --- tidb_test.go | 39 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+) diff --git a/tidb_test.go b/tidb_test.go index 4fc5d7bbaf..ce50c5e097 100644 --- a/tidb_test.go +++ b/tidb_test.go @@ -513,6 +513,45 @@ func (s *testSessionSuite) TestAutoicommit(c *C) { mustExecSQL(c, se, s.dropDBSQL) } +func checkInTrans(c *C, se Session, stmt string, isNil bool, expect uint16) { + mustExecSQL(c, se, stmt) + if isNil { + c.Assert(se.(*session).txn, IsNil) + } else { + c.Assert(se.(*session).txn, NotNil) + } + ret := variable.GetSessionVars(se.(*session)).Status & mysql.ServerStatusInTrans + c.Assert(ret, Equals, expect) +} + +// See: https://dev.mysql.com/doc/internals/en/status-flags.html +func (s *testSessionSuite) TestInTrans(c *C) { + store := newStore(c, s.dbName) + se := newSession(c, store, s.dbName) + mustExecSQL(c, se, "drop table if exists t") + c.Assert(se.(*session).txn, IsNil) + checkInTrans(c, se, "create table t (id BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL)", true, 0) + checkInTrans(c, se, "insert t values ()", true, 0) + checkInTrans(c, se, "begin", false, 1) + checkInTrans(c, se, "insert t values ()", false, 1) + se.Execute("drop table if exists t;") + c.Assert(se.(*session).txn, IsNil) + c.Assert(variable.GetSessionVars(se.(*session)).Status&mysql.ServerStatusInTrans, Equals, uint16(1)) + checkInTrans(c, se, "create table t (id BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL)", true, 1) + checkInTrans(c, se, "insert t values ()", false, 1) + checkInTrans(c, se, "commit", true, 0) + checkInTrans(c, se, "insert t values ()", true, 0) + + se.Execute("drop table if exists t;") + mustExecSQL(c, se, "create table t (id BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL)") + c.Assert(variable.GetSessionVars(se.(*session)).Status&mysql.ServerStatusInTrans, Equals, uint16(0)) + checkInTrans(c, se, "begin", false, 1) + checkInTrans(c, se, "insert t values ()", false, 1) + checkInTrans(c, se, "rollback", true, 0) + + mustExecSQL(c, se, s.dropDBSQL) +} + // See: http://dev.mysql.com/doc/refman/5.7/en/commit.html func (s *testSessionSuite) TestRowLock(c *C) { store := newStore(c, s.dbName) From c745049abfad890e7a33487a767cc844c8033812 Mon Sep 17 00:00:00 2001 From: xia Date: Tue, 15 Sep 2015 18:53:40 +0800 Subject: [PATCH 06/10] tidb: fix a bug and update test --- session.go | 1 + tidb_test.go | 6 +++--- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/session.go b/session.go index 8662ecd6a9..2b1e9e0ceb 100644 --- a/session.go +++ b/session.go @@ -139,6 +139,7 @@ func (s *session) FinishTxn(rollback bool) error { s.txn = nil }() + variable.GetSessionVars(s).SetStatusInTrans(false) if rollback { return s.txn.Rollback() } diff --git a/tidb_test.go b/tidb_test.go index ce50c5e097..f0cee2d3d2 100644 --- a/tidb_test.go +++ b/tidb_test.go @@ -536,9 +536,9 @@ func (s *testSessionSuite) TestInTrans(c *C) { checkInTrans(c, se, "insert t values ()", false, 1) se.Execute("drop table if exists t;") c.Assert(se.(*session).txn, IsNil) - c.Assert(variable.GetSessionVars(se.(*session)).Status&mysql.ServerStatusInTrans, Equals, uint16(1)) - checkInTrans(c, se, "create table t (id BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL)", true, 1) - checkInTrans(c, se, "insert t values ()", false, 1) + c.Assert(variable.GetSessionVars(se.(*session)).Status&mysql.ServerStatusInTrans, Equals, uint16(0)) + checkInTrans(c, se, "create table t (id BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL)", true, 0) + checkInTrans(c, se, "insert t values ()", false, 0) checkInTrans(c, se, "commit", true, 0) checkInTrans(c, se, "insert t values ()", true, 0) From b435e3e7e1271a4337748ed171115ade83bae914 Mon Sep 17 00:00:00 2001 From: xia Date: Tue, 15 Sep 2015 18:58:52 +0800 Subject: [PATCH 07/10] tidb: update format --- session.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/session.go b/session.go index 2b1e9e0ceb..4faec013ef 100644 --- a/session.go +++ b/session.go @@ -137,9 +137,9 @@ func (s *session) FinishTxn(rollback bool) error { } defer func() { s.txn = nil + variable.GetSessionVars(s).SetStatusInTrans(false) }() - variable.GetSessionVars(s).SetStatusInTrans(false) if rollback { return s.txn.Rollback() } From ee2a4cb93c76d285a2d99031fabe5e466b1eb406 Mon Sep 17 00:00:00 2001 From: xia Date: Wed, 16 Sep 2015 10:36:23 +0800 Subject: [PATCH 08/10] tidb: fix a bug --- session.go | 3 +++ tidb_test.go | 38 +++++++++++++++++--------------------- 2 files changed, 20 insertions(+), 21 deletions(-) diff --git a/session.go b/session.go index 4faec013ef..095c7978b0 100644 --- a/session.go +++ b/session.go @@ -341,11 +341,13 @@ func (s *session) GetTxn(forceNew bool) (kv.Transaction, error) { return nil, err } + variable.GetSessionVars(s).SetStatusInTrans(true) log.Infof("New txn:%s in session:%d", s.txn, s.sid) return s.txn, nil } if forceNew { err = s.txn.Commit() + variable.GetSessionVars(s).SetStatusInTrans(false) if err != nil { return nil, err } @@ -354,6 +356,7 @@ func (s *session) GetTxn(forceNew bool) (kv.Transaction, error) { if err != nil { return nil, err } + variable.GetSessionVars(s).SetStatusInTrans(true) log.Warnf("Force new txn:%s in session:%d", s.txn, s.sid) } return s.txn, nil diff --git a/tidb_test.go b/tidb_test.go index f0cee2d3d2..4c16eb0cad 100644 --- a/tidb_test.go +++ b/tidb_test.go @@ -513,9 +513,9 @@ func (s *testSessionSuite) TestAutoicommit(c *C) { mustExecSQL(c, se, s.dropDBSQL) } -func checkInTrans(c *C, se Session, stmt string, isNil bool, expect uint16) { +func checkInTrans(c *C, se Session, stmt string, expect uint16) { mustExecSQL(c, se, stmt) - if isNil { + if expect == 0 { c.Assert(se.(*session).txn, IsNil) } else { c.Assert(se.(*session).txn, NotNil) @@ -528,26 +528,22 @@ func checkInTrans(c *C, se Session, stmt string, isNil bool, expect uint16) { func (s *testSessionSuite) TestInTrans(c *C) { store := newStore(c, s.dbName) se := newSession(c, store, s.dbName) - mustExecSQL(c, se, "drop table if exists t") - c.Assert(se.(*session).txn, IsNil) - checkInTrans(c, se, "create table t (id BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL)", true, 0) - checkInTrans(c, se, "insert t values ()", true, 0) - checkInTrans(c, se, "begin", false, 1) - checkInTrans(c, se, "insert t values ()", false, 1) - se.Execute("drop table if exists t;") - c.Assert(se.(*session).txn, IsNil) - c.Assert(variable.GetSessionVars(se.(*session)).Status&mysql.ServerStatusInTrans, Equals, uint16(0)) - checkInTrans(c, se, "create table t (id BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL)", true, 0) - checkInTrans(c, se, "insert t values ()", false, 0) - checkInTrans(c, se, "commit", true, 0) - checkInTrans(c, se, "insert t values ()", true, 0) + checkInTrans(c, se, "drop table if exists t;", 0) + checkInTrans(c, se, "create table t (id BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL)", 0) + checkInTrans(c, se, "insert t values ()", 0) + checkInTrans(c, se, "begin", 1) + checkInTrans(c, se, "insert t values ()", 1) + checkInTrans(c, se, "drop table if exists t;", 0) + checkInTrans(c, se, "create table t (id BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL)", 0) + checkInTrans(c, se, "insert t values ()", 1) + checkInTrans(c, se, "commit", 0) + checkInTrans(c, se, "insert t values ()", 0) - se.Execute("drop table if exists t;") - mustExecSQL(c, se, "create table t (id BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL)") - c.Assert(variable.GetSessionVars(se.(*session)).Status&mysql.ServerStatusInTrans, Equals, uint16(0)) - checkInTrans(c, se, "begin", false, 1) - checkInTrans(c, se, "insert t values ()", false, 1) - checkInTrans(c, se, "rollback", true, 0) + checkInTrans(c, se, "drop table if exists t;", 0) + checkInTrans(c, se, "create table t (id BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL)", 0) + checkInTrans(c, se, "begin", 1) + checkInTrans(c, se, "insert t values ()", 1) + checkInTrans(c, se, "rollback", 0) mustExecSQL(c, se, s.dropDBSQL) } From 73df308a03870049481e5113c2077133c5d5c718 Mon Sep 17 00:00:00 2001 From: xia Date: Wed, 16 Sep 2015 15:31:29 +0800 Subject: [PATCH 09/10] *: remove DisableAutocommit and rename IsAutocommit to ShouldAutocommit --- sessionctx/variable/session.go | 9 +++------ stmt/stmts/select.go | 2 +- stmt/stmts/transaction.go | 3 --- tidb.go | 2 +- 4 files changed, 5 insertions(+), 11 deletions(-) diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index df789cb9ce..d54f9375eb 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -38,9 +38,6 @@ type SessionVars struct { // Client Capability ClientCapability uint32 // Client capability - // Disable autocommit - DisableAutocommit bool - // Found rows FoundRows uint64 } @@ -117,12 +114,12 @@ func (s *SessionVars) GetNextPreparedStmtID() uint32 { return s.preparedStmtID } -// IsAutocommit checks if it is in autocommit enviroment -func IsAutocommit(ctx context.Context) bool { +// ShouldAutocommit checks if it is in autocommit enviroment +func ShouldAutocommit(ctx context.Context) bool { // With START TRANSACTION, autocommit remains disabled until you end // the transaction with COMMIT or ROLLBACK. if GetSessionVars(ctx).Status&mysql.ServerStatusAutocommit > 0 && - !GetSessionVars(ctx).DisableAutocommit { + GetSessionVars(ctx).Status&mysql.ServerStatusInTrans > 0 { return true } return false diff --git a/stmt/stmts/select.go b/stmt/stmts/select.go index 6782f91862..1c930365b1 100644 --- a/stmt/stmts/select.go +++ b/stmt/stmts/select.go @@ -147,7 +147,7 @@ func (s *SelectStmt) Plan(ctx context.Context) (plan.Plan, error) { } } lock := s.Lock - if variable.IsAutocommit(ctx) { + if variable.ShouldAutocommit(ctx) { // Locking of rows for update using SELECT FOR UPDATE only applies when autocommit // is disabled (either by beginning transaction with START TRANSACTION or by setting // autocommit to 0. If autocommit is enabled, the rows matching the specification are not locked. diff --git a/stmt/stmts/transaction.go b/stmt/stmts/transaction.go index 1f9ea9a508..4780a5ad9f 100644 --- a/stmt/stmts/transaction.go +++ b/stmt/stmts/transaction.go @@ -63,7 +63,6 @@ func (s *BeginStmt) Exec(ctx context.Context) (_ rset.Recordset, err error) { // With START TRANSACTION, autocommit remains disabled until you end // the transaction with COMMIT or ROLLBACK. The autocommit mode then // reverts to its previous state. - variable.GetSessionVars(ctx).DisableAutocommit = true variable.GetSessionVars(ctx).SetStatusInTrans(true) return } @@ -97,7 +96,6 @@ func (s *CommitStmt) SetText(text string) { // Exec implements the stmt.Statement Exec interface. func (s *CommitStmt) Exec(ctx context.Context) (_ rset.Recordset, err error) { err = ctx.FinishTxn(false) - variable.GetSessionVars(ctx).DisableAutocommit = false variable.GetSessionVars(ctx).SetStatusInTrans(false) return } @@ -131,7 +129,6 @@ func (s *RollbackStmt) SetText(text string) { // Exec implements the stmt.Statement Exec interface. func (s *RollbackStmt) Exec(ctx context.Context) (_ rset.Recordset, err error) { err = ctx.FinishTxn(true) - variable.GetSessionVars(ctx).DisableAutocommit = false variable.GetSessionVars(ctx).SetStatusInTrans(false) return } diff --git a/tidb.go b/tidb.go index b6542b9c1a..c55dc9df31 100644 --- a/tidb.go +++ b/tidb.go @@ -161,7 +161,7 @@ func runStmt(ctx context.Context, s stmt.Statement, args ...interface{}) (rset.R stmt.ClearExecArgs(ctx) } // MySQL DDL should be auto-commit - if err == nil && (s.IsDDL() || variable.IsAutocommit(ctx)) { + if err == nil && (s.IsDDL() || variable.ShouldAutocommit(ctx)) { err = ctx.FinishTxn(false) } return rs, errors.Trace(err) From 127b0fba0e120d70cec2546982c04d123f776f79 Mon Sep 17 00:00:00 2001 From: xia Date: Wed, 16 Sep 2015 17:40:47 +0800 Subject: [PATCH 10/10] *: clean up and update test --- session.go | 2 -- sessionctx/variable/session.go | 3 ++- tidb_test.go | 3 ++- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/session.go b/session.go index 095c7978b0..908a994233 100644 --- a/session.go +++ b/session.go @@ -341,7 +341,6 @@ func (s *session) GetTxn(forceNew bool) (kv.Transaction, error) { return nil, err } - variable.GetSessionVars(s).SetStatusInTrans(true) log.Infof("New txn:%s in session:%d", s.txn, s.sid) return s.txn, nil } @@ -356,7 +355,6 @@ func (s *session) GetTxn(forceNew bool) (kv.Transaction, error) { if err != nil { return nil, err } - variable.GetSessionVars(s).SetStatusInTrans(true) log.Warnf("Force new txn:%s in session:%d", s.txn, s.sid) } return s.txn, nil diff --git a/sessionctx/variable/session.go b/sessionctx/variable/session.go index d54f9375eb..fdfc9c7916 100644 --- a/sessionctx/variable/session.go +++ b/sessionctx/variable/session.go @@ -119,8 +119,9 @@ func ShouldAutocommit(ctx context.Context) bool { // With START TRANSACTION, autocommit remains disabled until you end // the transaction with COMMIT or ROLLBACK. if GetSessionVars(ctx).Status&mysql.ServerStatusAutocommit > 0 && - GetSessionVars(ctx).Status&mysql.ServerStatusInTrans > 0 { + GetSessionVars(ctx).Status&mysql.ServerStatusInTrans == 0 { return true } + return false } diff --git a/tidb_test.go b/tidb_test.go index 2367f6a3e8..a24283feea 100644 --- a/tidb_test.go +++ b/tidb_test.go @@ -28,6 +28,7 @@ import ( "github.com/pingcap/tidb/kv" mysql "github.com/pingcap/tidb/mysqldef" "github.com/pingcap/tidb/rset" + "github.com/pingcap/tidb/sessionctx/variable" "github.com/pingcap/tidb/util/errors2" ) @@ -484,7 +485,7 @@ func (s *testSessionSuite) TestInTrans(c *C) { checkInTrans(c, se, "insert t values ()", 1) checkInTrans(c, se, "drop table if exists t;", 0) checkInTrans(c, se, "create table t (id BIGINT PRIMARY KEY AUTO_INCREMENT NOT NULL)", 0) - checkInTrans(c, se, "insert t values ()", 1) + checkInTrans(c, se, "insert t values ()", 0) checkInTrans(c, se, "commit", 0) checkInTrans(c, se, "insert t values ()", 0)