diff --git a/go/mysql/fakesqldb/server.go b/go/mysql/fakesqldb/server.go index ada111ffd47..b71a5c83695 100644 --- a/go/mysql/fakesqldb/server.go +++ b/go/mysql/fakesqldb/server.go @@ -98,6 +98,8 @@ type DB struct { patternData []exprResult // queryCalled keeps track of how many times a query was called. queryCalled map[string]int + // querylog keeps track of all called queries + querylog []string // This next set of fields is used when ordering of the queries matters. @@ -340,6 +342,7 @@ func (db *DB) HandleQuery(c *mysql.Conn, query string, callback func(*sqltypes.R db.mu.Lock() defer db.mu.Unlock() db.queryCalled[key]++ + db.querylog = append(db.querylog, key) // Check if we should close the connection and provoke errno 2013. if db.shouldClose { @@ -523,6 +526,11 @@ func (db *DB) GetQueryCalledNum(query string) int { return num } +//QueryLog returns the query log in a semicomma separated string +func (db *DB) QueryLog() string { + return strings.Join(db.querylog, ";") +} + // EnableConnFail makes connection to this fake DB fail. func (db *DB) EnableConnFail() { db.mu.Lock() diff --git a/go/pools/numbered.go b/go/pools/numbered.go index ef4e3ea3064..f1e76488164 100644 --- a/go/pools/numbered.go +++ b/go/pools/numbered.go @@ -51,6 +51,7 @@ func (u *unregistered) Size() int { return 1 } +//NewNumbered creates a new numbered func NewNumbered() *Numbered { n := &Numbered{ resources: make(map[int64]*numberedWrapper), @@ -86,7 +87,7 @@ func (nu *Numbered) Register(id int64, val interface{}, enforceTimeout bool) err // Unregister forgets the specified resource. If the resource is not present, it's ignored. func (nu *Numbered) Unregister(id int64, reason string) { - success := nu.unregister(id, reason) + success := nu.unregister(id) if success { nu.recentlyUnregistered.Set( fmt.Sprintf("%v", id), &unregistered{reason: reason, timeUnregistered: time.Now()}) @@ -95,7 +96,7 @@ func (nu *Numbered) Unregister(id int64, reason string) { // unregister forgets the resource, if it exists. Returns whether or not the resource existed at // time of Unregister. -func (nu *Numbered) unregister(id int64, reason string) bool { +func (nu *Numbered) unregister(id int64) bool { nu.mu.Lock() defer nu.mu.Unlock() @@ -199,11 +200,13 @@ func (nu *Numbered) WaitForEmpty() { } } +//StatsJSON returns stats in JSON format func (nu *Numbered) StatsJSON() string { return fmt.Sprintf("{\"Size\": %v}", nu.Size()) } -func (nu *Numbered) Size() (size int64) { +//Size returns the current size +func (nu *Numbered) Size() int64 { nu.mu.Lock() defer nu.mu.Unlock() return int64(len(nu.resources)) diff --git a/go/vt/vttablet/endtoend/batch_test.go b/go/vt/vttablet/endtoend/batch_test.go index 40118d1e6c0..7f4dac26caa 100644 --- a/go/vt/vttablet/endtoend/batch_test.go +++ b/go/vt/vttablet/endtoend/batch_test.go @@ -20,6 +20,7 @@ import ( "reflect" "testing" + "github.com/stretchr/testify/require" "vitess.io/vitess/go/sqltypes" querypb "vitess.io/vitess/go/vt/proto/query" "vitess.io/vitess/go/vt/vttablet/endtoend/framework" @@ -118,16 +119,14 @@ func TestBatchRead(t *testing.T) { want := []sqltypes.Result{qr1, qr2} qrl, err := client.ExecuteBatch(queries, false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) if !reflect.DeepEqual(qrl, want) { t.Errorf("ExecueBatch: \n%#v, want \n%#v", prettyPrintArr(qrl), prettyPrintArr(want)) } } func TestBatchTransaction(t *testing.T) { + client := framework.NewClient() queries := []*querypb.BoundQuery{{ Sql: "insert into vitess_test values(4, null, null, null)", @@ -148,20 +147,14 @@ func TestBatchTransaction(t *testing.T) { // Not in transaction, AsTransaction false qrl, err := client.ExecuteBatch(queries, false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) if !reflect.DeepEqual(qrl[1].Rows, wantRows) { t.Errorf("Rows: \n%#v, want \n%#v", qrl[1].Rows, wantRows) } // Not in transaction, AsTransaction true qrl, err = client.ExecuteBatch(queries, true) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) if !reflect.DeepEqual(qrl[1].Rows, wantRows) { t.Errorf("Rows: \n%#v, want \n%#v", qrl[1].Rows, wantRows) } @@ -169,16 +162,10 @@ func TestBatchTransaction(t *testing.T) { // In transaction, AsTransaction false func() { err = client.Begin(false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) defer client.Commit() qrl, err = client.ExecuteBatch(queries, false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) if !reflect.DeepEqual(qrl[1].Rows, wantRows) { t.Errorf("Rows: \n%#v, want \n%#v", qrl[1].Rows, wantRows) } @@ -187,15 +174,10 @@ func TestBatchTransaction(t *testing.T) { // In transaction, AsTransaction true func() { err = client.Begin(false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) defer client.Rollback() qrl, err = client.ExecuteBatch(queries, true) want := "cannot start a new transaction in the scope of an existing one" - if err == nil || err.Error() != want { - t.Errorf("Error: %v, want %s", err, want) - } + require.EqualError(t, err, want) }() } diff --git a/go/vt/vttablet/endtoend/config_test.go b/go/vt/vttablet/endtoend/config_test.go index 026c2855e69..851a45a6b3f 100644 --- a/go/vt/vttablet/endtoend/config_test.go +++ b/go/vt/vttablet/endtoend/config_test.go @@ -17,12 +17,13 @@ limitations under the License. package endtoend import ( - "fmt" "strings" "sync" "testing" "time" + "github.com/stretchr/testify/require" + "vitess.io/vitess/go/sqltypes" querypb "vitess.io/vitess/go/vt/proto/query" topodatapb "vitess.io/vitess/go/vt/proto/topodata" @@ -33,88 +34,15 @@ import ( ) // compareIntDiff returns an error if end[tag] != start[tag]+diff. -func compareIntDiff(end map[string]interface{}, tag string, start map[string]interface{}, diff int) error { - return verifyIntValue(end, tag, framework.FetchInt(start, tag)+diff) +func compareIntDiff(t *testing.T, end map[string]interface{}, tag string, start map[string]interface{}, diff int) { + t.Helper() + verifyIntValue(t, end, tag, framework.FetchInt(start, tag)+diff) } // verifyIntValue returns an error if values[tag] != want. -func verifyIntValue(values map[string]interface{}, tag string, want int) error { - got := framework.FetchInt(values, tag) - if got != want { - return fmt.Errorf("%s: %d, want %d", tag, got, want) - } - return nil -} - -func TestConfigVars(t *testing.T) { - currentConfig := tabletenv.NewCurrentConfig() - vars := framework.DebugVars() - cases := []struct { - tag string - val int - }{{ - tag: "ConnPoolAvailable", - val: currentConfig.OltpReadPool.Size, - }, { - tag: "ConnPoolCapacity", - val: currentConfig.OltpReadPool.Size, - }, { - tag: "ConnPoolIdleTimeout", - val: currentConfig.OltpReadPool.IdleTimeoutSeconds * 1e9, - }, { - tag: "ConnPoolMaxCap", - val: currentConfig.OltpReadPool.Size, - }, { - tag: "MaxResultSize", - val: currentConfig.Oltp.MaxRows, - }, { - tag: "WarnResultSize", - val: currentConfig.Oltp.WarnRows, - }, { - tag: "QueryCacheCapacity", - val: currentConfig.QueryCacheSize, - }, { - tag: "QueryTimeout", - val: int(currentConfig.Oltp.QueryTimeoutSeconds * 1e9), - }, { - tag: "SchemaReloadTime", - val: int(currentConfig.SchemaReloadIntervalSeconds * 1e9), - }, { - tag: "StreamBufferSize", - val: currentConfig.StreamBufferSize, - }, { - tag: "StreamConnPoolAvailable", - val: currentConfig.OlapReadPool.Size, - }, { - tag: "StreamConnPoolCapacity", - val: currentConfig.OlapReadPool.Size, - }, { - tag: "StreamConnPoolIdleTimeout", - val: currentConfig.OlapReadPool.IdleTimeoutSeconds * 1e9, - }, { - tag: "StreamConnPoolMaxCap", - val: currentConfig.OlapReadPool.Size, - }, { - tag: "TransactionPoolAvailable", - val: currentConfig.TxPool.Size, - }, { - tag: "TransactionPoolCapacity", - val: currentConfig.TxPool.Size, - }, { - tag: "TransactionPoolIdleTimeout", - val: currentConfig.TxPool.IdleTimeoutSeconds * 1e9, - }, { - tag: "TransactionPoolMaxCap", - val: currentConfig.TxPool.Size, - }, { - tag: "TransactionTimeout", - val: int(currentConfig.Oltp.TxTimeoutSeconds * 1e9), - }} - for _, tcase := range cases { - if err := verifyIntValue(vars, tcase.tag, int(tcase.val)); err != nil { - t.Error(err) - } - } +func verifyIntValue(t *testing.T, values map[string]interface{}, tag string, want int) { + t.Helper() + require.Equal(t, want, framework.FetchInt(values, tag), tag) } func TestPoolSize(t *testing.T) { @@ -122,9 +50,7 @@ func TestPoolSize(t *testing.T) { framework.Server.SetPoolSize(1) vstart := framework.DebugVars() - if err := verifyIntValue(vstart, "ConnPoolCapacity", 1); err != nil { - t.Error(err) - } + verifyIntValue(t, vstart, "ConnPoolCapacity", 1) var wg sync.WaitGroup wg.Add(2) @@ -270,34 +196,19 @@ func TestQueryPlanCache(t *testing.T) { _, _ = client.Execute("select * from vitess_test where intval=:ival1", bindVars) _, _ = client.Execute("select * from vitess_test where intval=:ival2", bindVars) vend := framework.DebugVars() - if err := verifyIntValue(vend, "QueryCacheLength", 1); err != nil { - t.Error(err) - } - if err := verifyIntValue(vend, "QueryCacheSize", 1); err != nil { - t.Error(err) - } - if err := verifyIntValue(vend, "QueryCacheCapacity", 1); err != nil { - t.Error(err) - } + verifyIntValue(t, vend, "QueryCacheLength", 1) + verifyIntValue(t, vend, "QueryCacheSize", 1) + verifyIntValue(t, vend, "QueryCacheCapacity", 1) framework.Server.SetQueryPlanCacheCap(10) _, _ = client.Execute("select * from vitess_test where intval=:ival1", bindVars) vend = framework.DebugVars() - if err := verifyIntValue(vend, "QueryCacheLength", 2); err != nil { - t.Error(err) - } - if err := verifyIntValue(vend, "QueryCacheSize", 2); err != nil { - t.Error(err) - } - + verifyIntValue(t, vend, "QueryCacheLength", 2) + verifyIntValue(t, vend, "QueryCacheSize", 2) _, _ = client.Execute("select * from vitess_test where intval=1", bindVars) vend = framework.DebugVars() - if err := verifyIntValue(vend, "QueryCacheLength", 3); err != nil { - t.Error(err) - } - if err := verifyIntValue(vend, "QueryCacheSize", 3); err != nil { - t.Error(err) - } + verifyIntValue(t, vend, "QueryCacheLength", 3) + verifyIntValue(t, vend, "QueryCacheSize", 3) } func TestMaxResultSize(t *testing.T) { @@ -311,10 +222,7 @@ func TestMaxResultSize(t *testing.T) { if err == nil || !strings.HasPrefix(err.Error(), want) { t.Errorf("Error: %v, must start with %s", err, want) } - if err := verifyIntValue(framework.DebugVars(), "MaxResultSize", 2); err != nil { - t.Error(err) - } - + verifyIntValue(t, framework.DebugVars(), "MaxResultSize", 2) framework.Server.SetMaxResultSize(10) _, err = client.Execute(query, nil) if err != nil { @@ -337,10 +245,7 @@ func TestWarnResultSize(t *testing.T) { t.Errorf("Warnings.ResultsExceeded counter should have increased by 1, instead got %v", exceededCountDiff) } - if err := verifyIntValue(framework.DebugVars(), "WarnResultSize", 2); err != nil { - t.Error(err) - } - + verifyIntValue(t, framework.DebugVars(), "WarnResultSize", 2) framework.Server.SetWarnResultSize(10) _, _ = client.Execute(query, nil) newerWarningsResultsExceededCount := framework.FetchInt(framework.DebugVars(), "Warnings/ResultsExceeded") @@ -370,10 +275,6 @@ func TestQueryTimeout(t *testing.T) { t.Errorf("Error code: %v, want %v", code, vtrpcpb.Code_ABORTED) } vend := framework.DebugVars() - if err := verifyIntValue(vend, "QueryTimeout", int(100*time.Millisecond)); err != nil { - t.Error(err) - } - if err := compareIntDiff(vend, "Kills/Queries", vstart, 1); err != nil { - t.Error(err) - } + verifyIntValue(t, vend, "QueryTimeout", int(100*time.Millisecond)) + compareIntDiff(t, vend, "Kills/Queries", vstart, 1) } diff --git a/go/vt/vttablet/endtoend/framework/eventcatcher.go b/go/vt/vttablet/endtoend/framework/eventcatcher.go index 3579709b807..429a40ffb8c 100644 --- a/go/vt/vttablet/endtoend/framework/eventcatcher.go +++ b/go/vt/vttablet/endtoend/framework/eventcatcher.go @@ -44,12 +44,12 @@ func (tc *TxCatcher) Close() { // Next fetches the next captured transaction. // If the wait is longer than one second, it returns an error. -func (tc *TxCatcher) Next() (*tabletserver.TxConnection, error) { +func (tc *TxCatcher) Next() (*tabletserver.StatefulConnection, error) { event, err := tc.catcher.next() if err != nil { return nil, err } - return event.(*tabletserver.TxConnection), nil + return event.(*tabletserver.StatefulConnection), nil } // QueryCatcher allows you to capture and fetch queries that are being diff --git a/go/vt/vttablet/endtoend/misc_test.go b/go/vt/vttablet/endtoend/misc_test.go index a6f41805c85..6b21c05ee6c 100644 --- a/go/vt/vttablet/endtoend/misc_test.go +++ b/go/vt/vttablet/endtoend/misc_test.go @@ -48,12 +48,8 @@ func TestSimpleRead(t *testing.T) { return } vend := framework.DebugVars() - if err := compareIntDiff(vend, "Queries/TotalCount", vstart, 1); err != nil { - t.Error(err) - } - if err := compareIntDiff(vend, "Queries/Histograms/Select/Count", vstart, 1); err != nil { - t.Error(err) - } + compareIntDiff(t, vend, "Queries/TotalCount", vstart, 1) + compareIntDiff(t, vend, "Queries/Histograms/Select/Count", vstart, 1) } func TestBinary(t *testing.T) { @@ -188,9 +184,7 @@ func TestIntegrityError(t *testing.T) { if err == nil || !strings.HasPrefix(err.Error(), want) { t.Errorf("Error: %v, want prefix %s", err, want) } - if err := compareIntDiff(framework.DebugVars(), "Errors/ALREADY_EXISTS", vstart, 1); err != nil { - t.Error(err) - } + compareIntDiff(t, framework.DebugVars(), "Errors/ALREADY_EXISTS", vstart, 1) } func TestTrailingComment(t *testing.T) { @@ -295,10 +289,7 @@ func TestConsolidation(t *testing.T) { wg.Wait() vend := framework.DebugVars() - if err := compareIntDiff(vend, "Waits/Histograms/Consolidations/Count", vstart, 1); err != nil { - t.Logf("DebugVars Waits/Histograms/Consolidations/Count not incremented with sleep=%v", sleep) - continue - } + compareIntDiff(t, vend, "Waits/Histograms/Consolidations/Count", vstart, 1) t.Logf("DebugVars properly incremented with sleep=%v", sleep) return } @@ -501,15 +492,9 @@ func TestQueryStats(t *testing.T) { t.Errorf("stat: %+v, want %+v", stat, want) } vend := framework.DebugVars() - if err := compareIntDiff(vend, "QueryCounts/vitess_a.Select", vstart, 2); err != nil { - t.Error(err) - } - if err := compareIntDiff(vend, "QueryRowCounts/vitess_a.Select", vstart, 2); err != nil { - t.Error(err) - } - if err := compareIntDiff(vend, "QueryErrorCounts/vitess_a.Select", vstart, 1); err != nil { - t.Error(err) - } + compareIntDiff(t, vend, "QueryCounts/vitess_a.Select", vstart, 2) + compareIntDiff(t, vend, "QueryRowCounts/vitess_a.Select", vstart, 2) + compareIntDiff(t, vend, "QueryErrorCounts/vitess_a.Select", vstart, 1) // Ensure BeginExecute also updates the stats and strips comments. query = "select /* begin_execute */ 1 /* trailing comment */" diff --git a/go/vt/vttablet/endtoend/transaction_test.go b/go/vt/vttablet/endtoend/transaction_test.go index 715c2d07904..e588ea03bb2 100644 --- a/go/vt/vttablet/endtoend/transaction_test.go +++ b/go/vt/vttablet/endtoend/transaction_test.go @@ -18,11 +18,12 @@ package endtoend import ( "fmt" - "reflect" "strings" "testing" "time" + "vitess.io/vitess/go/test/utils" + "github.com/golang/protobuf/proto" "github.com/stretchr/testify/require" "golang.org/x/net/context" @@ -40,63 +41,28 @@ func TestCommit(t *testing.T) { client := framework.NewClient() defer client.Execute("delete from vitess_test where intval=4", nil) - catcher := framework.NewTxCatcher() - defer catcher.Close() vstart := framework.DebugVars() - query := "insert into vitess_test (intval, floatval, charval, binval) " + - "values(4, null, null, null)" + query := "insert into vitess_test (intval, floatval, charval, binval) values (4, null, null, null)" err := client.Begin(false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) + _, err = client.Execute(query, nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) + err = client.Commit() - if err != nil { - t.Error(err) - return - } - tx, err := catcher.Next() - if err != nil { - t.Error(err) - return - } - want := []string{"insert into vitess_test(intval, floatval, charval, binval) values (4, null, null, null)"} - if !reflect.DeepEqual(tx.Queries, want) { - t.Errorf("queries: %v, want %v", tx.Queries, want) - } - if !reflect.DeepEqual(tx.Conclusion, "commit") { - t.Errorf("conclusion: %s, want commit", tx.Conclusion) - } + require.NoError(t, err) qr, err := client.Execute("select * from vitess_test", nil) - if err != nil { - t.Error(err) - return - } - if qr.RowsAffected != 4 { - t.Errorf("rows affected: %d, want 4", qr.RowsAffected) - } + require.NoError(t, err) + require.Equal(t, uint64(4), qr.RowsAffected, "rows affected") _, err = client.Execute("delete from vitess_test where intval=4", nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) qr, err = client.Execute("select * from vitess_test", nil) - if err != nil { - t.Error(err) - return - } - if qr.RowsAffected != 3 { - t.Errorf("rows affected: %d, want 4", qr.RowsAffected) - } + require.NoError(t, err) + require.Equal(t, uint64(3), qr.RowsAffected, "rows affected") expectedDiffs := []struct { tag string @@ -128,53 +94,25 @@ func TestCommit(t *testing.T) { }} vend := framework.DebugVars() for _, expected := range expectedDiffs { - if err := compareIntDiff(vend, expected.tag, vstart, expected.diff); err != nil { - t.Error(err) - } + compareIntDiff(t, vend, expected.tag, vstart, expected.diff) } } func TestRollback(t *testing.T) { client := framework.NewClient() - catcher := framework.NewTxCatcher() - defer catcher.Close() vstart := framework.DebugVars() query := "insert into vitess_test values(4, null, null, null)" err := client.Begin(false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) _, err = client.Execute(query, nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) err = client.Rollback() - if err != nil { - t.Error(err) - return - } - tx, err := catcher.Next() - if err != nil { - t.Error(err) - return - } - want := []string{"insert into vitess_test values (4, null, null, null)"} - if !reflect.DeepEqual(tx.Queries, want) { - t.Errorf("queries: %v, want %v", tx.Queries, want) - } - if !reflect.DeepEqual(tx.Conclusion, "rollback") { - t.Errorf("conclusion: %s, want rollback", tx.Conclusion) - } + require.NoError(t, err) qr, err := client.Execute("select * from vitess_test", nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) if qr.RowsAffected != 3 { t.Errorf("rows affected: %d, want 3", qr.RowsAffected) } @@ -200,9 +138,7 @@ func TestRollback(t *testing.T) { }} vend := framework.DebugVars() for _, expected := range expectedDiffs { - if err := compareIntDiff(vend, expected.tag, vstart, expected.diff); err != nil { - t.Error(err) - } + compareIntDiff(t, vend, expected.tag, vstart, expected.diff) } } @@ -210,53 +146,23 @@ func TestAutoCommit(t *testing.T) { client := framework.NewClient() defer client.Execute("delete from vitess_test where intval=4", nil) - catcher := framework.NewTxCatcher() - defer catcher.Close() vstart := framework.DebugVars() - query := "insert into vitess_test (intval, floatval, charval, binval) " + - "values(4, null, null, null)" + query := "insert into vitess_test (intval, floatval, charval, binval) values (4, null, null, null)" _, err := client.Execute(query, nil) - if err != nil { - t.Error(err) - return - } - tx, err := catcher.Next() - if err != nil { - t.Error(err) - return - } - want := []string{"insert into vitess_test(intval, floatval, charval, binval) values (4, null, null, null)"} - // Sometimes, no queries will be returned by the querylog because reliability - // is not guaranteed. If so, just move on without verifying. The subsequent - // rowcount check will anyway verify that the insert succeeded. - if len(tx.Queries) != 0 && !reflect.DeepEqual(tx.Queries, want) { - t.Errorf("queries: %v, want %v", tx.Queries, want) - } - if !reflect.DeepEqual(tx.Conclusion, "commit") { - t.Errorf("conclusion: %s, want commit", tx.Conclusion) - } + require.NoError(t, err) qr, err := client.Execute("select * from vitess_test", nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) if qr.RowsAffected != 4 { t.Errorf("rows affected: %d, want 4", qr.RowsAffected) } _, err = client.Execute("delete from vitess_test where intval=4", nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) qr, err = client.Execute("select * from vitess_test", nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) if qr.RowsAffected != 3 { t.Errorf("rows affected: %d, want 4", qr.RowsAffected) } @@ -306,34 +212,21 @@ func TestTxPoolSize(t *testing.T) { client1 := framework.NewClient() err := client1.Begin(false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) defer client1.Rollback() - if err := verifyIntValue(framework.DebugVars(), "TransactionPoolAvailable", tabletenv.NewCurrentConfig().TxPool.Size-1); err != nil { - t.Error(err) - } + verifyIntValue(t, framework.DebugVars(), "TransactionPoolAvailable", tabletenv.NewCurrentConfig().TxPool.Size-1) defer framework.Server.SetTxPoolSize(framework.Server.TxPoolSize()) framework.Server.SetTxPoolSize(1) vend := framework.DebugVars() - if err := verifyIntValue(vend, "TransactionPoolAvailable", 0); err != nil { - t.Error(err) - } - if err := verifyIntValue(vend, "TransactionPoolCapacity", 1); err != nil { - t.Error(err) - } + verifyIntValue(t, vend, "TransactionPoolAvailable", 0) + verifyIntValue(t, vend, "TransactionPoolCapacity", 1) client2 := framework.NewClient() err = client2.Begin(false) - want := "connection limit exceeded" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("%v, must contain %s", err, want) - } - if err := compareIntDiff(framework.DebugVars(), "Errors/RESOURCE_EXHAUSTED", vstart, 1); err != nil { - t.Error(err) - } + require.Error(t, err) + require.Contains(t, err.Error(), "connection limit exceeded") + compareIntDiff(t, framework.DebugVars(), "Errors/RESOURCE_EXHAUSTED", vstart, 1) } func TestForUpdate(t *testing.T) { @@ -348,20 +241,11 @@ func TestForUpdate(t *testing.T) { // We should not get errors here err = client.Begin(false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) _, err = client.Execute(query, nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) err = client.Commit() - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) } } @@ -372,31 +256,18 @@ func TestPrepareRollback(t *testing.T) { query := "insert into vitess_test (intval, floatval, charval, binval) " + "values(4, null, null, null)" err := client.Begin(false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) _, err = client.Execute(query, nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) err = client.Prepare("aa") if err != nil { client.RollbackPrepared("aa", 0) - t.Error(err) - return + t.Fatalf(err.Error()) } err = client.RollbackPrepared("aa", 0) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) qr, err := client.Execute("select * from vitess_test", nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) if qr.RowsAffected != 3 { t.Errorf("rows affected: %d, want 3", qr.RowsAffected) } @@ -409,31 +280,18 @@ func TestPrepareCommit(t *testing.T) { query := "insert into vitess_test (intval, floatval, charval, binval) " + "values(4, null, null, null)" err := client.Begin(false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) _, err = client.Execute(query, nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) err = client.Prepare("aa") if err != nil { client.RollbackPrepared("aa", 0) - t.Error(err) - return + t.Fatal(err) } err = client.CommitPrepared("aa") - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) qr, err := client.Execute("select * from vitess_test", nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) if qr.RowsAffected != 4 { t.Errorf("rows affected: %d, want 4", qr.RowsAffected) } @@ -446,43 +304,24 @@ func TestPrepareReparentCommit(t *testing.T) { query := "insert into vitess_test (intval, floatval, charval, binval) " + "values(4, null, null, null)" err := client.Begin(false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) _, err = client.Execute(query, nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) err = client.Prepare("aa") if err != nil { client.RollbackPrepared("aa", 0) - t.Error(err) - return + t.Fatal(err) } // Rollback all transactions err = client.SetServingType(topodatapb.TabletType_REPLICA) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) // This should resurrect the prepared transaction. err = client.SetServingType(topodatapb.TabletType_MASTER) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) err = client.CommitPrepared("aa") - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) qr, err := client.Execute("select * from vitess_test", nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) if qr.RowsAffected != 4 { t.Errorf("rows affected: %d, want 4", qr.RowsAffected) } @@ -509,19 +348,14 @@ func TestMMCommitFlow(t *testing.T) { require.NoError(t, err) err = client.CreateTransaction("aa", []*querypb.Target{}) - want := "Duplicate entry" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("%v, must contain %s", err, want) - } + require.Error(t, err) + require.Contains(t, err.Error(), "Duplicate entry") err = client.StartCommit("aa") require.NoError(t, err) err = client.SetRollback("aa", 0) - want = "could not transition to ROLLBACK: aa (CallerID: dev)" - if err == nil || err.Error() != want { - t.Errorf("%v, must contain %s", err, want) - } + require.EqualError(t, err, "could not transition to ROLLBACK: aa (CallerID: dev)") info, err := client.ReadTransaction("aa") require.NoError(t, err) @@ -539,9 +373,7 @@ func TestMMCommitFlow(t *testing.T) { TabletType: topodatapb.TabletType_MASTER, }}, } - if !proto.Equal(info, wantInfo) { - t.Errorf("ReadTransaction: %#v, want %#v", info, wantInfo) - } + utils.MustMatch(t, wantInfo, info, "ReadTransaction") err = client.ConcludeTransaction("aa") require.NoError(t, err) @@ -661,21 +493,12 @@ func TestUnresolvedTracking(t *testing.T) { query := "insert into vitess_test (intval, floatval, charval, binval) " + "values(4, null, null, null)" err := client.Begin(false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) _, err = client.Execute(query, nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) err = client.Prepare("aa") defer client.RollbackPrepared("aa", 0) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) time.Sleep(10 * time.Second) vars := framework.DebugVars() if val := framework.FetchInt(vars, "Unresolved/Prepares"); val != 1 { @@ -694,57 +517,30 @@ func TestManualTwopcz(t *testing.T) { ctx := context.Background() conn, err := mysql.Connect(ctx, &connParams) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) defer conn.Close() // Successful prepare. err = client.Begin(false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) _, err = client.Execute("insert into vitess_test (intval, floatval, charval, binval) values(4, null, null, null)", nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) _, err = client.Execute("insert into vitess_test (intval, floatval, charval, binval) values(5, null, null, null)", nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) err = client.Prepare("dtidsuccess") defer client.RollbackPrepared("dtidsuccess", 0) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) // Failed transaction. err = client.Begin(false) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) _, err = client.Execute("insert into vitess_test (intval, floatval, charval, binval) values(6, null, null, null)", nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) _, err = client.Execute("insert into vitess_test (intval, floatval, charval, binval) values(7, null, null, null)", nil) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) err = client.Prepare("dtidfail") defer client.RollbackPrepared("dtidfail", 0) - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) conn.ExecuteFetch(fmt.Sprintf("update _vt.redo_state set state = %d where dtid = 'dtidfail'", tabletserver.RedoStateFailed), 10, false) conn.ExecuteFetch("commit", 10, false) @@ -758,10 +554,7 @@ func TestManualTwopcz(t *testing.T) { }}) defer client.ConcludeTransaction("distributed") - if err != nil { - t.Error(err) - return - } + require.NoError(t, err) fmt.Printf("%s/twopcz\n", framework.ServerAddress) fmt.Print("Sleeping for 30 seconds\n") time.Sleep(30 * time.Second) diff --git a/go/vt/vttablet/tabletserver/query_executor.go b/go/vt/vttablet/tabletserver/query_executor.go index 55ff6abf2d2..d637a52fd49 100644 --- a/go/vt/vttablet/tabletserver/query_executor.go +++ b/go/vt/vttablet/tabletserver/query_executor.go @@ -22,6 +22,8 @@ import ( "strings" "time" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" + "vitess.io/vitess/go/vt/vtgate/evalengine" "golang.org/x/net/context" @@ -113,11 +115,11 @@ func (qre *QueryExecutor) Execute() (reply *sqltypes.Result, err error) { if qre.transactionID != 0 { // Need upfront connection for DMLs and transactions - conn, err := qre.tsv.te.txPool.Get(qre.transactionID, "for query") + conn, err := qre.tsv.te.txPool.GetAndLock(qre.transactionID, "for query") if err != nil { return nil, err } - defer conn.Recycle() + defer conn.Unlock() return qre.txConnExec(conn) } @@ -145,50 +147,51 @@ func (qre *QueryExecutor) Execute() (reply *sqltypes.Result, err error) { return nil, vterrors.Errorf(vtrpcpb.Code_INTERNAL, "%s unexpected plan type", qre.plan.PlanID.String()) } -func (qre *QueryExecutor) execAutocommit(f func(conn *TxConnection) (*sqltypes.Result, error)) (reply *sqltypes.Result, err error) { +func (qre *QueryExecutor) execAutocommit(f func(conn tx.IStatefulConnection) (*sqltypes.Result, error)) (reply *sqltypes.Result, err error) { if qre.options == nil { qre.options = &querypb.ExecuteOptions{} } qre.options.TransactionIsolation = querypb.ExecuteOptions_AUTOCOMMIT - conn, _, err := qre.tsv.te.txPool.LocalBegin(qre.ctx, qre.options) + + conn, _, err := qre.tsv.te.txPool.Begin(qre.ctx, qre.options) + if err != nil { return nil, err } - defer qre.tsv.te.txPool.LocalConclude(qre.ctx, conn) + defer qre.tsv.te.txPool.RollbackAndRelease(qre.ctx, conn) return f(conn) } -func (qre *QueryExecutor) execAsTransaction(f func(conn *TxConnection) (*sqltypes.Result, error)) (reply *sqltypes.Result, err error) { - conn, beginSQL, err := qre.tsv.te.txPool.LocalBegin(qre.ctx, qre.options) +func (qre *QueryExecutor) execAsTransaction(f func(conn tx.IStatefulConnection) (*sqltypes.Result, error)) (*sqltypes.Result, error) { + conn, beginSQL, err := qre.tsv.te.txPool.Begin(qre.ctx, qre.options) if err != nil { return nil, err } - defer qre.tsv.te.txPool.LocalConclude(qre.ctx, conn) + defer qre.tsv.te.txPool.RollbackAndRelease(qre.ctx, conn) qre.logStats.AddRewrittenSQL(beginSQL, time.Now()) - reply, err = f(conn) + result, err := f(conn) if err != nil { // dbConn is nil, it means the transaction was aborted. // If so, we should not relog the rollback. // TODO(sougou): these txPool functions should take the logstats // and log any statements they issue. This needs to be done as // a separate refactor because it impacts lot of code. - if conn.dbConn != nil { + if conn.IsInTransaction() { defer qre.logStats.AddRewrittenSQL("rollback", time.Now()) - qre.tsv.te.txPool.LocalConclude(qre.ctx, conn) } return nil, err } defer qre.logStats.AddRewrittenSQL("commit", time.Now()) - if _, err := qre.tsv.te.txPool.LocalCommit(qre.ctx, conn); err != nil { + if _, err := qre.tsv.te.txPool.Commit(qre.ctx, conn); err != nil { return nil, err } - return reply, nil + return result, nil } -func (qre *QueryExecutor) txConnExec(conn *TxConnection) (*sqltypes.Result, error) { +func (qre *QueryExecutor) txConnExec(conn tx.IStatefulConnection) (*sqltypes.Result, error) { switch qre.plan.PlanID { case planbuilder.PlanInsert, planbuilder.PlanUpdate, planbuilder.PlanDelete: return qre.txFetch(conn, true) @@ -232,12 +235,12 @@ func (qre *QueryExecutor) Stream(callback func(*sqltypes.Result) error) error { // if we have a transaction id, let's use the txPool for this query var conn *connpool.DBConn if qre.transactionID != 0 { - txConn, err := qre.tsv.te.txPool.Get(qre.transactionID, "for streaming query") + txConn, err := qre.tsv.te.txPool.GetAndLock(qre.transactionID, "for streaming query") if err != nil { return err } - defer txConn.Recycle() - conn = txConn.dbConn + defer txConn.Unlock() + conn = txConn.UnderlyingdDBConn() } else { dbConn, err := qre.getStreamConn() if err != nil { @@ -361,7 +364,7 @@ func (qre *QueryExecutor) checkAccess(authorized *tableacl.ACLResult, tableName return nil } -func (qre *QueryExecutor) execDDL(conn *TxConnection) (*sqltypes.Result, error) { +func (qre *QueryExecutor) execDDL(conn tx.IStatefulConnection) (*sqltypes.Result, error) { defer func() { if err := qre.tsv.se.Reload(qre.ctx); err != nil { log.Errorf("failed to reload schema %v", err) @@ -372,13 +375,27 @@ func (qre *QueryExecutor) execDDL(conn *TxConnection) (*sqltypes.Result, error) if err != nil { return nil, err } - err = conn.BeginAgain(qre.ctx) + err = qre.BeginAgain(qre.ctx, conn) if err != nil { return nil, err } return result, nil } +// BeginAgain commits the existing transaction and begins a new one +func (*QueryExecutor) BeginAgain(ctx context.Context, dc tx.IStatefulConnection) error { + if dc.IsClosed() || dc.TxProperties().Autocommit { + return nil + } + if _, err := dc.Exec(ctx, "commit", 1, false); err != nil { + return err + } + if _, err := dc.Exec(ctx, "begin", 1, false); err != nil { + return err + } + return nil +} + func (qre *QueryExecutor) execNextval() (*sqltypes.Result, error) { inc, err := resolveNumber(qre.plan.NextCount, qre.bindVars) if err != nil { @@ -393,7 +410,7 @@ func (qre *QueryExecutor) execNextval() (*sqltypes.Result, error) { t.SequenceInfo.Lock() defer t.SequenceInfo.Unlock() if t.SequenceInfo.NextVal == 0 || t.SequenceInfo.NextVal+inc > t.SequenceInfo.LastVal { - _, err := qre.execAsTransaction(func(conn *TxConnection) (*sqltypes.Result, error) { + _, err := qre.execAsTransaction(func(conn tx.IStatefulConnection) (*sqltypes.Result, error) { query := fmt.Sprintf("select next_id, cache from %s where id = 0 for update", sqlparser.String(tableName)) qr, err := qre.execSQL(conn, query, false) if err != nil { @@ -429,7 +446,7 @@ func (qre *QueryExecutor) execNextval() (*sqltypes.Result, error) { newLast += cache } query = fmt.Sprintf("update %s set next_id = %d where id = 0", sqlparser.String(tableName), newLast) - conn.RecordQuery(query) + conn.TxProperties().RecordQuery(query) _, err = qre.execSQL(conn, query, false) if err != nil { return nil, err @@ -473,7 +490,7 @@ func (qre *QueryExecutor) execSelect() (*sqltypes.Result, error) { return qre.dbConnFetch(conn, qre.plan.FullQuery, qre.bindVars) } -func (qre *QueryExecutor) execDMLLimit(conn *TxConnection) (*sqltypes.Result, error) { +func (qre *QueryExecutor) execDMLLimit(conn tx.IStatefulConnection) (*sqltypes.Result, error) { maxrows := qre.tsv.qe.maxResultSize.Get() qre.bindVars["#maxLimit"] = sqltypes.Int64BindVariable(maxrows + 1) result, err := qre.txFetch(conn, true) @@ -482,7 +499,7 @@ func (qre *QueryExecutor) execDMLLimit(conn *TxConnection) (*sqltypes.Result, er } if err := qre.verifyRowCount(int64(result.RowsAffected), maxrows); err != nil { defer qre.logStats.AddRewrittenSQL("rollback", time.Now()) - qre.tsv.te.txPool.LocalConclude(qre.ctx, conn) + _ = qre.tsv.te.txPool.Rollback(qre.ctx, conn) return nil, err } return result, nil @@ -585,7 +602,7 @@ func (qre *QueryExecutor) qFetch(logStats *tabletenv.LogStats, parsedQuery *sqlp } // txFetch fetches from a TxConnection. -func (qre *QueryExecutor) txFetch(conn *TxConnection, record bool) (*sqltypes.Result, error) { +func (qre *QueryExecutor) txFetch(conn tx.IStatefulConnection, record bool) (*sqltypes.Result, error) { sql, _, err := qre.generateFinalSQL(qre.plan.FullQuery, qre.bindVars) if err != nil { return nil, err @@ -596,7 +613,7 @@ func (qre *QueryExecutor) txFetch(conn *TxConnection, record bool) (*sqltypes.Re } // Only record successful queries. if record { - conn.RecordQuery(sql) + conn.TxProperties().RecordQuery(sql) } return qr, nil } @@ -643,12 +660,12 @@ func (qre *QueryExecutor) getSelectLimit() int64 { return maxRows } -// poolConn is an abstraction for reusing code in execSQL. -type poolConn interface { +// executor is an abstraction for reusing code in execSQL. +type executor interface { Exec(ctx context.Context, query string, maxrows int, wantfields bool) (*sqltypes.Result, error) } -func (qre *QueryExecutor) execSQL(conn poolConn, sql string, wantfields bool) (*sqltypes.Result, error) { +func (qre *QueryExecutor) execSQL(conn executor, sql string, wantfields bool) (*sqltypes.Result, error) { span, ctx := trace.NewSpan(qre.ctx, "QueryExecutor.execSQL") defer span.Finish() diff --git a/go/vt/vttablet/tabletserver/query_executor_test.go b/go/vt/vttablet/tabletserver/query_executor_test.go index c58947bf23f..1b54e198a66 100644 --- a/go/vt/vttablet/tabletserver/query_executor_test.go +++ b/go/vt/vttablet/tabletserver/query_executor_test.go @@ -24,6 +24,8 @@ import ( "strings" "testing" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "golang.org/x/net/context" @@ -377,8 +379,8 @@ func TestQueryExecutorLimitFailure(t *testing.T) { logWant: "begin; delete from test_table limit 3; rollback", inTxWant: "delete from test_table limit 3", }} - for _, tcase := range testcases { - func() { + for i, tcase := range testcases { + t.Run(fmt.Sprintf("%d - %s", i, tcase.input), func(t *testing.T) { db := setUpQueryExecutorTest(t) defer db.Close() for _, dbr := range tcase.dbResponses { @@ -393,21 +395,21 @@ func TestQueryExecutorLimitFailure(t *testing.T) { // Test outside a transaction. qre := newTestQueryExecutor(ctx, tsv, tcase.input, 0) _, err := qre.Execute() - if err == nil || !strings.Contains(err.Error(), tcase.err) { - t.Errorf("Execute(%v): %v, must contain %v", tcase.input, err, tcase.err) - } + assert.Error(t, err) + assert.Contains(t, err.Error(), tcase.err) assert.Equal(t, tcase.logWant, qre.logStats.RewrittenSQL(), tcase.input) // Test inside a transaction. txid, err := tsv.Begin(ctx, &tsv.target, nil) + require.NoError(t, err) defer tsv.Commit(ctx, &tsv.target, txid) qre = newTestQueryExecutor(ctx, tsv, tcase.input, txid) _, err = qre.Execute() - if err == nil || !strings.Contains(err.Error(), tcase.err) { - t.Errorf("Execute(%v): %v, must contain %v", tcase.input, err, tcase.err) - } + assert.Error(t, err) + assert.Contains(t, err.Error(), tcase.err) + want := tcase.logWant if tcase.inTxWant != "" { want = tcase.inTxWant @@ -418,13 +420,12 @@ func TestQueryExecutorLimitFailure(t *testing.T) { return } // Ensure transaction was rolled back. - qre = newTestQueryExecutor(ctx, tsv, "update test_table set a=1", txid) - _, err = qre.Execute() - notxError := "ended at" - if err == nil || !strings.Contains(err.Error(), notxError) { - t.Errorf("Execute(%v): %v, must contain %v", tcase.input, err, notxError) - } - }() + conn, err := tsv.te.txPool.GetAndLock(txid, "") + require.NoError(t, err) + defer conn.Release(tx.TxClose) + + require.False(t, conn.IsInTransaction(), "connection is still in a transaction") + }) } } @@ -599,7 +600,7 @@ func TestQueryExecutorMessageStreamACL(t *testing.T) { db := setUpQueryExecutorTest(t) defer db.Close() - tsv := newTestTabletServer(context.Background(), enableStrictTableACL, db) + tsv := newTestTabletServer(ctx, enableStrictTableACL, db) defer tsv.StopService() plan, err := tsv.qe.GetMessageStreamPlan("msg") diff --git a/go/vt/vttablet/tabletserver/stateful_connection.go b/go/vt/vttablet/tabletserver/stateful_connection.go new file mode 100644 index 00000000000..b698cf392ea --- /dev/null +++ b/go/vt/vttablet/tabletserver/stateful_connection.go @@ -0,0 +1,162 @@ +/* +Copyright 2019 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tabletserver + +import ( + "fmt" + + "vitess.io/vitess/go/mysql" + "vitess.io/vitess/go/sqltypes" + "vitess.io/vitess/go/vt/vttablet/tabletserver/connpool" + + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" + + "golang.org/x/net/context" + + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/vterrors" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tabletenv" +) + +// StatefulConnection is used in the situations where we need a dedicated connection for a vtgate session. +// This is used for transactions and reserved connections. +// NOTE: After use, if must be returned either by doing a Unlock() or a Release(). +type StatefulConnection struct { + pool *StatefulConnectionPool + dbConn *connpool.DBConn + ConnID tx.ConnID + env tabletenv.Env + + txProps *tx.Properties +} + +// Close closes the underlying connection. When the connection is Unblocked, it will be Released +func (sc *StatefulConnection) Close() { + if sc.dbConn != nil { + sc.dbConn.Close() + } +} + +//IsClosed returns true when the connection is still operational +func (sc *StatefulConnection) IsClosed() bool { + return sc.dbConn == nil || sc.dbConn.IsClosed() +} + +//IsInTransaction returns true when the connection has tx state +func (sc *StatefulConnection) IsInTransaction() bool { + return sc.txProps != nil +} + +// Exec executes the statement in the dedicated connection +func (sc *StatefulConnection) Exec(ctx context.Context, query string, maxrows int, wantfields bool) (*sqltypes.Result, error) { + if sc.IsClosed() { + if sc.IsInTransaction() { + return nil, vterrors.Errorf(vtrpcpb.Code_ABORTED, "transaction was aborted: %v", sc.txProps.Conclusion) + } + return nil, vterrors.New(vtrpcpb.Code_ABORTED, "connection was aborted") + } + r, err := sc.dbConn.ExecOnce(ctx, query, maxrows, wantfields) + if err != nil { + if mysql.IsConnErr(err) { + select { + case <-ctx.Done(): + // If the context is done, the query was killed. + // So, don't trigger a mysql check. + default: + sc.env.CheckMySQL() + } + } + return nil, err + } + return r, nil +} + +func (sc *StatefulConnection) execWithRetry(ctx context.Context, query string, maxrows int, wantfields bool) error { + if sc.IsClosed() { + return nil + } + if _, err := sc.dbConn.Exec(ctx, query, maxrows, wantfields); err != nil { + return err + } + return nil +} + +// Unlock returns the connection to the pool. The connection remains active. +// This method is idempotent and can be called multiple times +func (sc *StatefulConnection) Unlock() { + if sc.dbConn == nil { + return + } + if sc.dbConn.IsClosed() { + sc.Releasef("unlocked closed connection") + } else { + sc.pool.markAsNotInUse(sc.ConnID) + } +} + +//Release is used when the connection will not be used ever again. +//The underlying dbConn is removed so that this connection cannot be used by mistake. +func (sc *StatefulConnection) Release(reason tx.ReleaseReason) { + sc.Releasef(reason.String()) +} + +//Releasef is used when the connection will not be used ever again. +//The underlying dbConn is removed so that this connection cannot be used by mistake. +func (sc *StatefulConnection) Releasef(reasonFormat string, a ...interface{}) { + if sc.dbConn == nil { + return + } + sc.pool.unregister(sc.ConnID, fmt.Sprintf(reasonFormat, a...)) + sc.dbConn.Recycle() + sc.dbConn = nil +} + +// String returns a printable version of the connection info. +func (sc *StatefulConnection) String() string { + return fmt.Sprintf( + "%v\t%s", + sc.ConnID, + sc.txProps.String(), + ) +} + +//TxProperties returns the transactional properties of the connection +func (sc *StatefulConnection) TxProperties() *tx.Properties { + return sc.txProps +} + +//ID returns the identifier for this connection +func (sc *StatefulConnection) ID() tx.ConnID { + return sc.ConnID +} + +//UnderlyingdDBConn returns the underlying database connection +func (sc *StatefulConnection) UnderlyingdDBConn() *connpool.DBConn { + return sc.dbConn +} + +//CleanTxState cleans out the current transaction state +func (sc *StatefulConnection) CleanTxState() { + sc.txProps = nil +} + +//Stats implements the tx.IStatefulConnection interface +func (sc *StatefulConnection) Stats() *tabletenv.Stats { + return sc.env.Stats() +} + +var _ tx.IStatefulConnection = (*StatefulConnection)(nil) diff --git a/go/vt/vttablet/tabletserver/stateful_connection_pool.go b/go/vt/vttablet/tabletserver/stateful_connection_pool.go new file mode 100644 index 00000000000..f2d2903ee68 --- /dev/null +++ b/go/vt/vttablet/tabletserver/stateful_connection_pool.go @@ -0,0 +1,194 @@ +/* +Copyright 2020 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tabletserver + +import ( + "time" + + "vitess.io/vitess/go/pools" + + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" + + "golang.org/x/net/context" + + "vitess.io/vitess/go/sync2" + "vitess.io/vitess/go/vt/dbconfigs" + "vitess.io/vitess/go/vt/log" + querypb "vitess.io/vitess/go/vt/proto/query" + "vitess.io/vitess/go/vt/vttablet/tabletserver/connpool" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tabletenv" +) + +//StatefulConnectionPool keeps track of currently and future active connections +//it's used whenever the session has some state that requires a dedicated connection +type StatefulConnectionPool struct { + // conns is the 'regular' pool. By default, connections + // are pulled from here for starting transactions. + conns *connpool.Pool + + // foundRowsPool is the alternate pool that creates + // connections with CLIENT_FOUND_ROWS flag set. A separate + // pool is needed because this option can only be set at + // connection time. + foundRowsPool *connpool.Pool + active *pools.Numbered + lastID sync2.AtomicInt64 + env tabletenv.Env +} + +//NewStatefulConnPool creates an ActivePool +func NewStatefulConnPool(env tabletenv.Env) *StatefulConnectionPool { + config := env.Config() + + return &StatefulConnectionPool{ + env: env, + conns: connpool.NewPool(env, "TransactionPool", config.TxPool), + foundRowsPool: connpool.NewPool(env, "FoundRowsPool", config.TxPool), + active: pools.NewNumbered(), + lastID: sync2.NewAtomicInt64(time.Now().UnixNano()), + } +} + +// Open makes the TxPool operational. This also starts the transaction killer +// that will kill long-running transactions. +func (sf *StatefulConnectionPool) Open(appParams, dbaParams, appDebugParams dbconfigs.Connector) { + log.Infof("Starting transaction id: %d", sf.lastID) + sf.conns.Open(appParams, dbaParams, appDebugParams) + foundRowsParam, _ := appParams.MysqlParams() + foundRowsParam.EnableClientFoundRows() + appParams = dbconfigs.New(foundRowsParam) + sf.foundRowsPool.Open(appParams, dbaParams, appDebugParams) +} + +// Close closes the TxPool. A closed pool can be reopened. +func (sf *StatefulConnectionPool) Close() { + for _, v := range sf.active.GetOutdated(time.Duration(0), "for closing") { + conn := v.(*StatefulConnection) + thing := "connection" + if conn.IsInTransaction() { + thing = "transaction" + } + log.Warningf("killing %s for shutdown: %s", thing, conn.String()) + sf.env.Stats().InternalErrors.Add("StrayTransactions", 1) + conn.Close() + conn.Releasef("pool closed") + } + sf.conns.Close() + sf.foundRowsPool.Close() +} + +// AdjustLastID adjusts the last transaction id to be at least +// as large as the input value. This will ensure that there are +// no dtid collisions with future transactions. +func (sf *StatefulConnectionPool) AdjustLastID(id int64) { + if current := sf.lastID.Get(); current < id { + log.Infof("Adjusting transaction id to: %d", id) + sf.lastID.Set(id) + } +} + +// GetOutdated returns a list of connections that are older than age. +// It does not return any connections that are in use. +func (sf *StatefulConnectionPool) GetOutdated(age time.Duration, purpose string) []*StatefulConnection { + return mapToTxConn(sf.active.GetOutdated(age, purpose)) +} + +func mapToTxConn(outdated []interface{}) []*StatefulConnection { + result := make([]*StatefulConnection, len(outdated)) + for i, el := range outdated { + result[i] = el.(*StatefulConnection) + } + return result +} + +// WaitForEmpty returns as soon as the pool becomes empty +func (sf *StatefulConnectionPool) WaitForEmpty() { + sf.active.WaitForEmpty() +} + +// GetAndLock locks the connection for use. It accepts a purpose as a string. +// If it cannot be found, it returns a "not found" error. If in use, +// it returns a "in use: purpose" error. +func (sf *StatefulConnectionPool) GetAndLock(id int64, reason string) (*StatefulConnection, error) { + conn, err := sf.active.Get(id, reason) + if err != nil { + return nil, err + } + return conn.(*StatefulConnection), nil +} + +//NewConn creates a new StatefulConnection. It will be created from either the normal pool or +//the found_rows pool, depending on the options provided +func (sf *StatefulConnectionPool) NewConn(ctx context.Context, options *querypb.ExecuteOptions) (*StatefulConnection, error) { + + var conn *connpool.DBConn + var err error + + if options.GetClientFoundRows() { + conn, err = sf.foundRowsPool.Get(ctx) + } else { + conn, err = sf.conns.Get(ctx) + } + if err != nil { + return nil, err + } + + connID := sf.lastID.Add(1) + sfConn := &StatefulConnection{ + dbConn: conn, + ConnID: connID, + pool: sf, + env: sf.env, + } + + err = sf.active.Register( + sfConn.ConnID, + sfConn, + options.GetWorkload() != querypb.ExecuteOptions_DBA, + ) + if err != nil { + sfConn.Release(tx.ConnInitFail) + return nil, err + } + + return sf.GetAndLock(sfConn.ConnID, "new connection") +} + +//ForAllTxProperties executes a function an every connection that has a not-nil TxProperties +func (sf *StatefulConnectionPool) ForAllTxProperties(f func(*tx.Properties)) { + for _, connection := range mapToTxConn(sf.active.GetAll()) { + props := connection.txProps + if props != nil { + f(props) + } + } +} + +// Unregister forgets the specified connection. If the connection is not present, it's ignored. +func (sf *StatefulConnectionPool) unregister(id tx.ConnID, reason string) { + sf.active.Unregister(id, reason) +} + +//markAsNotInUse marks the connection as not in use at the moment +func (sf *StatefulConnectionPool) markAsNotInUse(id tx.ConnID) { + sf.active.Put(id) +} + +// Capacity returns the pool capacity. +func (sf *StatefulConnectionPool) Capacity() int { + return int(sf.conns.Capacity()) +} diff --git a/go/vt/vttablet/tabletserver/stateful_connection_pool_test.go b/go/vt/vttablet/tabletserver/stateful_connection_pool_test.go new file mode 100644 index 00000000000..5ae132a04a8 --- /dev/null +++ b/go/vt/vttablet/tabletserver/stateful_connection_pool_test.go @@ -0,0 +1,138 @@ +/* +Copyright 2020 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tabletserver + +import ( + "context" + "testing" + + "github.com/stretchr/testify/require" + "gotest.tools/assert" + "vitess.io/vitess/go/mysql/fakesqldb" + "vitess.io/vitess/go/sqltypes" + querypb "vitess.io/vitess/go/vt/proto/query" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" +) + +var ctx = context.Background() + +func TestActivePoolClientRowsFound(t *testing.T) { + db := fakesqldb.New(t) + defer db.Close() + db.AddQuery("begin", &sqltypes.Result{}) + + pool := newActivePool() + pool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) + + startNormalSize := pool.conns.Available() + startFoundRowsSize := pool.foundRowsPool.Available() + + conn1, err := pool.NewConn(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) + assert.Equal(t, startNormalSize-1, pool.conns.Available(), "default pool not used") + + conn2, err := pool.NewConn(ctx, &querypb.ExecuteOptions{ClientFoundRows: true}) + require.NoError(t, err) + assert.Equal(t, startFoundRowsSize-1, pool.conns.Available(), "foundRows pool not used") + + conn1.Release(tx.TxClose) + assert.Equal(t, startNormalSize, pool.conns.Available(), "default pool not restored after release") + + conn2.Release(tx.TxClose) + assert.Equal(t, startFoundRowsSize, pool.conns.Available(), "default pool not restored after release") +} + +func TestActivePoolForAllTxProps(t *testing.T) { + db := fakesqldb.New(t) + defer db.Close() + pool := newActivePool() + pool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) + conn1, err := pool.NewConn(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) + conn1.txProps = &tx.Properties{} + + conn2, err := pool.NewConn(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) + // for the second connection, we are not going to set a tx state + + conn3, err := pool.NewConn(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) + conn3.txProps = &tx.Properties{} + + pool.ForAllTxProperties(func(p *tx.Properties) { + p.LogToFile = true + }) + + require.True(t, conn1.txProps.LogToFile, "connection missed") + require.Nil(t, conn2.txProps) + require.True(t, conn3.txProps.LogToFile, "connection missed") +} + +func TestActivePoolGetConnNonExistentTransaction(t *testing.T) { + db := fakesqldb.New(t) + defer db.Close() + pool := newActivePool() + pool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) + _, err := pool.GetAndLock(12345, "for query") + require.EqualError(t, err, "not found") +} + +func TestExecWithAbortedCtx(t *testing.T) { + ctx, cancel := context.WithCancel(ctx) + db := fakesqldb.New(t) + defer db.Close() + pool := newActivePool() + pool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) + conn, err := pool.NewConn(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) + cancel() + _, err = conn.Exec(ctx, "", 0, false) + require.Error(t, err) +} + +func TestExecWithDbconnClosed(t *testing.T) { + db := fakesqldb.New(t) + defer db.Close() + pool := newActivePool() + pool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) + conn, err := pool.NewConn(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) + conn.Close() + + _, err = conn.Exec(ctx, "", 0, false) + require.EqualError(t, err, "connection was aborted") +} + +func TestExecWithDbconnClosedHavingTx(t *testing.T) { + db := fakesqldb.New(t) + defer db.Close() + pool := newActivePool() + pool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) + conn, err := pool.NewConn(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) + conn.txProps = &tx.Properties{Conclusion: "foobar"} + conn.Close() + + _, err = conn.Exec(ctx, "", 0, false) + require.EqualError(t, err, "transaction was aborted: foobar") +} + +func newActivePool() *StatefulConnectionPool { + env := newEnv("ActivePoolTest") + + return NewStatefulConnPool(env) +} diff --git a/go/vt/vttablet/tabletserver/tabletserver.go b/go/vt/vttablet/tabletserver/tabletserver.go index be94d0f5d8e..6a7c9ea5d90 100644 --- a/go/vt/vttablet/tabletserver/tabletserver.go +++ b/go/vt/vttablet/tabletserver/tabletserver.go @@ -29,6 +29,8 @@ import ( "syscall" "time" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" + "golang.org/x/net/context" "vitess.io/vitess/go/acl" "vitess.io/vitess/go/history" @@ -168,6 +170,8 @@ type TabletServer struct { vstreamer *vstreamer.Engine messager *messager.Engine + txController tx.EngineStateMachine + // checkMySQLThrottler is used to throttle the number of // requests sent to CheckMySQL. checkMySQLThrottler *sync2.Semaphore @@ -226,6 +230,7 @@ func NewTabletServer(name string, config *tabletenv.TabletConfig, topoServer *to tsv.se = schema.NewEngine(tsv) tsv.qe = NewQueryEngine(tsv, tsv.se) tsv.te = NewTxEngine(tsv) + tsv.txController = tsv.te tsv.hw = heartbeat.NewWriter(tsv, alias) tsv.hr = heartbeat.NewReader(tsv) tsv.txThrottler = txthrottler.NewTxThrottler(tsv.config, topoServer) @@ -518,7 +523,7 @@ func (tsv *TabletServer) fullStart() (err error) { if err := tsv.qe.Open(); err != nil { return err } - if err := tsv.te.Init(); err != nil { + if err := tsv.txController.Init(); err != nil { return err } if err := tsv.hw.Init(tsv.target); err != nil { @@ -536,7 +541,7 @@ func (tsv *TabletServer) serveNewType() (err error) { // transactional requests are not allowed. So, we can // be sure that the tx pool won't change after the wait. if tsv.target.TabletType == topodatapb.TabletType_MASTER { - tsv.te.AcceptReadWrite() + tsv.txController.AcceptReadWrite() if err := tsv.txThrottler.Open(tsv.target.Keyspace, tsv.target.Shard); err != nil { return err } @@ -544,7 +549,7 @@ func (tsv *TabletServer) serveNewType() (err error) { tsv.hr.Close() tsv.hw.Open() } else { - tsv.te.AcceptReadOnly() + tsv.txController.AcceptReadOnly() tsv.messager.Close() tsv.hr.Open() tsv.hw.Close() @@ -600,7 +605,7 @@ func (tsv *TabletServer) waitForShutdown() { // will be allowed. They will enable the conclusion of outstanding // transactions. tsv.messager.Close() - tsv.te.StopGently() + tsv.txController.StopGently() tsv.qe.streamQList.TerminateAll() tsv.watcher.Close() tsv.requests.Wait() @@ -615,7 +620,7 @@ func (tsv *TabletServer) closeAll() { tsv.vstreamer.Close() tsv.hr.Close() tsv.hw.Close() - tsv.te.StopGently() + tsv.txController.StopGently() tsv.watcher.Close() tsv.qe.Close() tsv.se.Close() @@ -953,7 +958,7 @@ func (tsv *TabletServer) Execute(ctx context.Context, target *querypb.Target, sq trace.AnnotateSQL(span, sql) defer span.Finish() - allowOnShutdown := (transactionID != 0) + allowOnShutdown := transactionID != 0 err = tsv.execRequest( ctx, tsv.QueryTimeout.Get(), "Execute", sql, bindVariables, @@ -1845,12 +1850,12 @@ func (tsv *TabletServer) StreamPoolSize() int { // SetTxPoolSize changes the tx pool size to the specified value. // This function should only be used for testing. func (tsv *TabletServer) SetTxPoolSize(val int) { - tsv.te.txPool.conns.SetCapacity(val) + tsv.te.txPool.scp.conns.SetCapacity(val) } // TxPoolSize returns the tx pool size. func (tsv *TabletServer) TxPoolSize() int { - return int(tsv.te.txPool.conns.Capacity()) + return tsv.te.txPool.scp.Capacity() } // SetTxTimeout changes the transaction timeout to the specified value. diff --git a/go/vt/vttablet/tabletserver/tabletserver_test.go b/go/vt/vttablet/tabletserver/tabletserver_test.go index e1225aebea4..8f1ebdc2b33 100644 --- a/go/vt/vttablet/tabletserver/tabletserver_test.go +++ b/go/vt/vttablet/tabletserver/tabletserver_test.go @@ -29,6 +29,8 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + "github.com/golang/protobuf/proto" "github.com/stretchr/testify/require" "golang.org/x/net/context" @@ -514,16 +516,17 @@ func TestTabletServerMasterToReplica(t *testing.T) { target := querypb.Target{TabletType: topodatapb.TabletType_MASTER} txid1, err := tsv.Begin(ctx, &target, nil) require.NoError(t, err) - if _, err := tsv.Execute(ctx, &target, "update test_table set name = 2 where pk = 1", nil, txid1, nil); err != nil { - t.Error(err) - } - if err = tsv.Prepare(ctx, &target, txid1, "aa"); err != nil { - t.Error(err) - } + + _, err = tsv.Execute(ctx, &target, "update test_table set name = 2 where pk = 1", nil, txid1, nil) + require.NoError(t, err) + + err = tsv.Prepare(ctx, &target, txid1, "aa") + require.NoError(t, err) + txid2, err := tsv.Begin(ctx, &target, nil) require.NoError(t, err) // This makes txid2 busy - conn2, err := tsv.te.txPool.Get(txid2, "for query") + conn2, err := tsv.te.txPool.GetAndLock(txid2, "for query") require.NoError(t, err) ch := make(chan bool) go func() { @@ -538,12 +541,10 @@ func TestTabletServerMasterToReplica(t *testing.T) { t.Fatal("ch should not fire") case <-time.After(10 * time.Millisecond): } - if tsv.te.txPool.activePool.Size() != 1 { - t.Errorf("len(tsv.te.txPool.activePool.Size()): %d, want 1", len(tsv.te.preparedPool.conns)) - } + assert.Equal(t, int64(1), tsv.te.txPool.scp.active.Size(), "tsv.te.txPool.scp.active.Size(): ") // Concluding conn2 will allow the transition to go through. - tsv.te.txPool.LocalConclude(ctx, conn2) + tsv.te.txPool.RollbackAndRelease(ctx, conn2) <-ch } @@ -588,7 +589,7 @@ func TestTabletServerRedoLogIsKeptBetweenRestarts(t *testing.T) { if len(tsv.te.preparedPool.conns) != 1 { t.Errorf("len(tsv.te.preparedPool.conns): %d, want 1", len(tsv.te.preparedPool.conns)) } - got := tsv.te.preparedPool.conns["dtid0"].Queries + got := tsv.te.preparedPool.conns["dtid0"].TxProperties().Queries want := []string{"update test_table set name = 2 where pk = 1 limit 10001"} if !reflect.DeepEqual(got, want) { t.Errorf("Prepared queries: %v, want %v", got, want) @@ -598,7 +599,7 @@ func TestTabletServerRedoLogIsKeptBetweenRestarts(t *testing.T) { t.Errorf("len(tsv.te.preparedPool.conns): %d, want 0", v) } - tsv.te.txPool.lastID.Set(1) + tsv.te.txPool.scp.lastID.Set(1) // Ensure we continue past errors. db.AddQuery(tpc.readAllRedo, &sqltypes.Result{ Fields: []*querypb.Field{ @@ -628,7 +629,7 @@ func TestTabletServerRedoLogIsKeptBetweenRestarts(t *testing.T) { if len(tsv.te.preparedPool.conns) != 1 { t.Errorf("len(tsv.te.preparedPool.conns): %d, want 1", len(tsv.te.preparedPool.conns)) } - got = tsv.te.preparedPool.conns["a:b:10"].Queries + got = tsv.te.preparedPool.conns["a:b:10"].TxProperties().Queries want = []string{"update test_table set name = 2 where pk = 1 limit 10001"} if !reflect.DeepEqual(got, want) { t.Errorf("Prepared queries: %v, want %v", got, want) @@ -638,7 +639,7 @@ func TestTabletServerRedoLogIsKeptBetweenRestarts(t *testing.T) { t.Errorf("Failed dtids: %v, want %v", tsv.te.preparedPool.reserved, wantFailed) } // Verify last id got adjusted. - if v := tsv.te.txPool.lastID.Get(); v != 20 { + if v := tsv.te.txPool.scp.lastID.Get(); v != 20 { t.Errorf("tsv.te.txPool.lastID.Get(): %d, want 20", v) } turnOffTxEngine() @@ -2480,7 +2481,7 @@ func TestConfigChanges(t *testing.T) { if val := tsv.TxPoolSize(); val != newSize { t.Errorf("TxPoolSize: %d, want %d", val, newSize) } - if val := int(tsv.te.txPool.conns.Capacity()); val != newSize { + if val := int(tsv.te.txPool.scp.Capacity()); val != newSize { t.Errorf("tsv.te.txPool.pool.Capacity: %d, want %d", val, newSize) } @@ -2489,7 +2490,7 @@ func TestConfigChanges(t *testing.T) { t.Errorf("tsv.TxTimeout: %v, want %v", val, newDuration) } if val := tsv.te.txPool.Timeout(); val != newDuration { - t.Errorf("tsv.te.txPool.Timeout: %v, want %v", val, newDuration) + t.Errorf("tsv.te.Pool().Timeout: %v, want %v", val, newDuration) } tsv.SetQueryPlanCacheCap(newSize) diff --git a/go/vt/vttablet/tabletserver/twopc.go b/go/vt/vttablet/tabletserver/twopc.go index 4c365afb5d6..df03db170e0 100644 --- a/go/vt/vttablet/tabletserver/twopc.go +++ b/go/vt/vttablet/tabletserver/twopc.go @@ -20,6 +20,8 @@ import ( "fmt" "time" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" + "vitess.io/vitess/go/vt/vtgate/evalengine" "golang.org/x/net/context" @@ -209,7 +211,7 @@ func (tpc *TwoPC) Close() { } // SaveRedo saves the statements in the redo log using the supplied connection. -func (tpc *TwoPC) SaveRedo(ctx context.Context, conn *TxConnection, dtid string, queries []string) error { +func (tpc *TwoPC) SaveRedo(ctx context.Context, conn tx.IStatefulConnection, dtid string, queries []string) error { bindVars := map[string]*querypb.BindVariable{ "dtid": sqltypes.StringBindVariable(dtid), "state": sqltypes.Int64BindVariable(RedoStatePrepared), @@ -240,7 +242,7 @@ func (tpc *TwoPC) SaveRedo(ctx context.Context, conn *TxConnection, dtid string, } // UpdateRedo changes the state of the redo log for the dtid. -func (tpc *TwoPC) UpdateRedo(ctx context.Context, conn *TxConnection, dtid string, state int) error { +func (tpc *TwoPC) UpdateRedo(ctx context.Context, conn tx.IStatefulConnection, dtid string, state int) error { bindVars := map[string]*querypb.BindVariable{ "dtid": sqltypes.StringBindVariable(dtid), "state": sqltypes.Int64BindVariable(int64(state)), @@ -250,7 +252,7 @@ func (tpc *TwoPC) UpdateRedo(ctx context.Context, conn *TxConnection, dtid strin } // DeleteRedo deletes the redo log for the dtid. -func (tpc *TwoPC) DeleteRedo(ctx context.Context, conn *TxConnection, dtid string) error { +func (tpc *TwoPC) DeleteRedo(ctx context.Context, conn tx.IStatefulConnection, dtid string) error { bindVars := map[string]*querypb.BindVariable{ "dtid": sqltypes.StringBindVariable(dtid), } @@ -262,15 +264,8 @@ func (tpc *TwoPC) DeleteRedo(ctx context.Context, conn *TxConnection, dtid strin return err } -// PreparedTx represents a displayable version of a prepared transaction. -type PreparedTx struct { - Dtid string - Queries []string - Time time.Time -} - // ReadAllRedo returns all the prepared transactions from the redo logs. -func (tpc *TwoPC) ReadAllRedo(ctx context.Context) (prepared, failed []*PreparedTx, err error) { +func (tpc *TwoPC) ReadAllRedo(ctx context.Context) (prepared, failed []*tx.PreparedTx, err error) { conn, err := tpc.readPool.Get(ctx) if err != nil { return nil, nil, err @@ -282,7 +277,7 @@ func (tpc *TwoPC) ReadAllRedo(ctx context.Context) (prepared, failed []*Prepared return nil, nil, err } - var curTx *PreparedTx + var curTx *tx.PreparedTx for _, row := range qr.Rows { dtid := row[0].ToString() if curTx == nil || dtid != curTx.Dtid { @@ -290,7 +285,7 @@ func (tpc *TwoPC) ReadAllRedo(ctx context.Context) (prepared, failed []*Prepared // A failure in time parsing will show up as a very old time, // which is harmless. tm, _ := evalengine.ToInt64(row[2]) - curTx = &PreparedTx{ + curTx = &tx.PreparedTx{ Dtid: dtid, Time: time.Unix(0, tm), } @@ -336,7 +331,7 @@ func (tpc *TwoPC) CountUnresolvedRedo(ctx context.Context, unresolvedTime time.T } // CreateTransaction saves the metadata of a 2pc transaction as Prepared. -func (tpc *TwoPC) CreateTransaction(ctx context.Context, conn *TxConnection, dtid string, participants []*querypb.Target) error { +func (tpc *TwoPC) CreateTransaction(ctx context.Context, conn tx.IStatefulConnection, dtid string, participants []*querypb.Target) error { bindVars := map[string]*querypb.BindVariable{ "dtid": sqltypes.StringBindVariable(dtid), "state": sqltypes.Int64BindVariable(int64(DTStatePrepare)), @@ -369,7 +364,7 @@ func (tpc *TwoPC) CreateTransaction(ctx context.Context, conn *TxConnection, dti // Transition performs a transition from Prepare to the specified state. // If the transaction is not a in the Prepare state, an error is returned. -func (tpc *TwoPC) Transition(ctx context.Context, conn *TxConnection, dtid string, state querypb.TransactionState) error { +func (tpc *TwoPC) Transition(ctx context.Context, conn tx.IStatefulConnection, dtid string, state querypb.TransactionState) error { bindVars := map[string]*querypb.BindVariable{ "dtid": sqltypes.StringBindVariable(dtid), "state": sqltypes.Int64BindVariable(int64(state)), @@ -386,7 +381,7 @@ func (tpc *TwoPC) Transition(ctx context.Context, conn *TxConnection, dtid strin } // DeleteTransaction deletes the metadata for the specified transaction. -func (tpc *TwoPC) DeleteTransaction(ctx context.Context, conn *TxConnection, dtid string) error { +func (tpc *TwoPC) DeleteTransaction(ctx context.Context, conn tx.IStatefulConnection, dtid string) error { bindVars := map[string]*querypb.BindVariable{ "dtid": sqltypes.StringBindVariable(dtid), } @@ -474,17 +469,8 @@ func (tpc *TwoPC) ReadAbandoned(ctx context.Context, abandonTime time.Time) (map return txs, nil } -// DistributedTx is similar to querypb.TransactionMetadata, but -// is display friendly. -type DistributedTx struct { - Dtid string - State string - Created time.Time - Participants []querypb.Target -} - // ReadAllTransactions returns info about all distributed transactions. -func (tpc *TwoPC) ReadAllTransactions(ctx context.Context) ([]*DistributedTx, error) { +func (tpc *TwoPC) ReadAllTransactions(ctx context.Context) ([]*tx.DistributedTx, error) { conn, err := tpc.readPool.Get(ctx) if err != nil { return nil, err @@ -496,8 +482,8 @@ func (tpc *TwoPC) ReadAllTransactions(ctx context.Context) ([]*DistributedTx, er return nil, err } - var curTx *DistributedTx - var distributed []*DistributedTx + var curTx *tx.DistributedTx + var distributed []*tx.DistributedTx for _, row := range qr.Rows { dtid := row[0].ToString() if curTx == nil || dtid != curTx.Dtid { @@ -515,7 +501,7 @@ func (tpc *TwoPC) ReadAllTransactions(ctx context.Context) ([]*DistributedTx, er if protostate < querypb.TransactionState_UNKNOWN || protostate > querypb.TransactionState_ROLLBACK { log.Errorf("Unexpected state for dtid %s: %v.", dtid, protostate) } - curTx = &DistributedTx{ + curTx = &tx.DistributedTx{ Dtid: dtid, State: querypb.TransactionState(st).String(), Created: time.Unix(0, tm), @@ -530,7 +516,7 @@ func (tpc *TwoPC) ReadAllTransactions(ctx context.Context) ([]*DistributedTx, er return distributed, nil } -func (tpc *TwoPC) exec(ctx context.Context, conn *TxConnection, pq *sqlparser.ParsedQuery, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error) { +func (tpc *TwoPC) exec(ctx context.Context, conn tx.IStatefulConnection, pq *sqlparser.ParsedQuery, bindVars map[string]*querypb.BindVariable) (*sqltypes.Result, error) { q, err := pq.GenerateQuery(bindVars, nil) if err != nil { return nil, err diff --git a/go/vt/vttablet/tabletserver/twopc_test.go b/go/vt/vttablet/tabletserver/twopc_test.go index c64231be73b..6790773ab65 100644 --- a/go/vt/vttablet/tabletserver/twopc_test.go +++ b/go/vt/vttablet/tabletserver/twopc_test.go @@ -22,6 +22,8 @@ import ( "testing" "time" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" + "golang.org/x/net/context" "vitess.io/vitess/go/sqltypes" @@ -48,7 +50,7 @@ func TestReadAllRedo(t *testing.T) { if err != nil { t.Fatal(err) } - var want []*PreparedTx + var want []*tx.PreparedTx if !reflect.DeepEqual(prepared, want) { t.Errorf("ReadAllRedo: %s, want %s", jsonStr(prepared), jsonStr(want)) } @@ -74,7 +76,7 @@ func TestReadAllRedo(t *testing.T) { if err != nil { t.Fatal(err) } - want = []*PreparedTx{{ + want = []*tx.PreparedTx{{ Dtid: "dtid0", Queries: []string{"stmt01"}, Time: time.Unix(0, 1), @@ -109,7 +111,7 @@ func TestReadAllRedo(t *testing.T) { if err != nil { t.Fatal(err) } - want = []*PreparedTx{{ + want = []*tx.PreparedTx{{ Dtid: "dtid0", Queries: []string{"stmt01", "stmt02"}, Time: time.Unix(0, 1), @@ -149,7 +151,7 @@ func TestReadAllRedo(t *testing.T) { if err != nil { t.Fatal(err) } - want = []*PreparedTx{{ + want = []*tx.PreparedTx{{ Dtid: "dtid0", Queries: []string{"stmt01", "stmt02"}, Time: time.Unix(0, 1), @@ -208,7 +210,7 @@ func TestReadAllRedo(t *testing.T) { if err != nil { t.Fatal(err) } - want = []*PreparedTx{{ + want = []*tx.PreparedTx{{ Dtid: "dtid0", Queries: []string{"stmt01", "stmt02"}, Time: time.Unix(0, 1), @@ -220,7 +222,7 @@ func TestReadAllRedo(t *testing.T) { if !reflect.DeepEqual(prepared, want) { t.Errorf("ReadAllRedo: %s, want %s", jsonStr(prepared), jsonStr(want)) } - wantFailed := []*PreparedTx{{ + wantFailed := []*tx.PreparedTx{{ Dtid: "dtid1", Queries: []string{"stmt11"}, Time: time.Unix(0, 1), @@ -252,7 +254,7 @@ func TestReadAllTransactions(t *testing.T) { if err != nil { t.Fatal(err) } - var want []*DistributedTx + var want []*tx.DistributedTx if !reflect.DeepEqual(distributed, want) { t.Errorf("ReadAllTransactions: %s, want %s", jsonStr(distributed), jsonStr(want)) } @@ -277,7 +279,7 @@ func TestReadAllTransactions(t *testing.T) { if err != nil { t.Fatal(err) } - want = []*DistributedTx{{ + want = []*tx.DistributedTx{{ Dtid: "dtid0", State: "PREPARE", Created: time.Unix(0, 1), @@ -316,7 +318,7 @@ func TestReadAllTransactions(t *testing.T) { if err != nil { t.Fatal(err) } - want = []*DistributedTx{{ + want = []*tx.DistributedTx{{ Dtid: "dtid0", State: "PREPARE", Created: time.Unix(0, 1), @@ -364,7 +366,7 @@ func TestReadAllTransactions(t *testing.T) { if err != nil { t.Fatal(err) } - want = []*DistributedTx{{ + want = []*tx.DistributedTx{{ Dtid: "dtid0", State: "PREPARE", Created: time.Unix(0, 1), diff --git a/go/vt/vttablet/tabletserver/twopcz.go b/go/vt/vttablet/tabletserver/twopcz.go index 6f93d05e580..dd8768844c3 100644 --- a/go/vt/vttablet/tabletserver/twopcz.go +++ b/go/vt/vttablet/tabletserver/twopcz.go @@ -22,6 +22,8 @@ import ( "html/template" "net/http" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" + "vitess.io/vitess/go/acl" "vitess.io/vitess/go/vt/log" ) @@ -160,8 +162,8 @@ func twopczHandler(txe *TxExecutor, w http.ResponseWriter, r *http.Request) { if format == "json" { w.Header().Set("Content-Type", "application/json") js, err := json.Marshal(struct { - Distributed []*DistributedTx - Prepared, Failed []*PreparedTx + Distributed []*tx.DistributedTx + Prepared, Failed []*tx.PreparedTx }{ Distributed: distributed, Prepared: prepared, diff --git a/go/vt/vttablet/tabletserver/tx/api.go b/go/vt/vttablet/tabletserver/tx/api.go new file mode 100644 index 00000000000..5b98f97fba2 --- /dev/null +++ b/go/vt/vttablet/tabletserver/tx/api.go @@ -0,0 +1,170 @@ +/* +Copyright 2020 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tx + +import ( + "context" + "fmt" + "strings" + "time" + + "vitess.io/vitess/go/sqltypes" + querypb "vitess.io/vitess/go/vt/proto/query" + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/servenv" + "vitess.io/vitess/go/vt/vttablet/tabletserver/connpool" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tabletenv" +) + +type ( + // ConnID as type int64 + ConnID = int64 + + //DTID as type string + DTID = string + + //EngineStateMachine is used to control the state the transactional engine - + //whether new connections and/or transactions are allowed or not. + EngineStateMachine interface { + Init() error + AcceptReadWrite() error + AcceptReadOnly() error + StopGently() + } + + //IStatefulConnection is a connection where the user is trusted to clean things up after using the connection + IStatefulConnection interface { + // Executes a query on the connection + Exec(ctx context.Context, query string, maxrows int, wantfields bool) (*sqltypes.Result, error) + + // Release is used after we are done with the connection and will not use it again + Release(reason ReleaseReason) + + // Releasef is used after we are done with the connection and will not use it again + Releasef(format string, params ...interface{}) + + // Unlock marks the connection as not in use. The connection remains active. + Unlock() + + IsInTransaction() bool + + Close() + + IsClosed() bool + + String() string + + TxProperties() *Properties + + ID() ConnID + + UnderlyingdDBConn() *connpool.DBConn + + CleanTxState() + + Stats() *tabletenv.Stats + } + + // ReleaseReason as type int + ReleaseReason int + + //Properties contains all information that is related to the currently running + //transaction on the connection + Properties struct { + EffectiveCaller *vtrpcpb.CallerID + ImmediateCaller *querypb.VTGateCallerID + StartTime time.Time + EndTime time.Time + Queries []string + Autocommit bool + Conclusion string + LogToFile bool + + Stats *servenv.TimingsWrapper + } +) + +const ( + // TxClose - connection released on close. + TxClose ReleaseReason = iota + + // TxCommit - connection released on commit. + TxCommit + + // TxRollback - connection released on rollback. + TxRollback + + // TxKill - connection released on tx kill. + TxKill + + // ConnInitFail - connection released on failed to start tx. + ConnInitFail +) + +func (r ReleaseReason) String() string { + return txResolutions[r] +} + +//Name return the name of enum. +func (r ReleaseReason) Name() string { + return txNames[r] +} + +var txResolutions = map[ReleaseReason]string{ + TxClose: "closed", + TxCommit: "transaction committed", + TxRollback: "transaction rolled back", + TxKill: "kill", + ConnInitFail: "initFail", +} + +var txNames = map[ReleaseReason]string{ + TxClose: "close", + TxCommit: "commit", + TxRollback: "rollback", + TxKill: "kill", + ConnInitFail: "initFail", +} + +// RecordQuery records the query against this transaction. +func (p *Properties) RecordQuery(query string) { + if p == nil { + return + } + p.Queries = append(p.Queries, query) +} + +// InTransaction returns true as soon as this struct is not nil +func (p *Properties) InTransaction() bool { return p != nil } + +// String returns a printable version of the transaction +func (p *Properties) String() string { + if p == nil { + return "" + } + + return fmt.Sprintf( + "'%v'\t'%v'\t%v\t%v\t%.6f\t%v\t%v\t\n", + p.EffectiveCaller, + p.ImmediateCaller, + p.StartTime.Format(time.StampMicro), + p.EndTime.Format(time.StampMicro), + p.EndTime.Sub(p.StartTime).Seconds(), + p.Conclusion, + strings.Join(p.Queries, ";"), + ) +} diff --git a/go/vt/vttablet/tabletserver/tx/twopc.go b/go/vt/vttablet/tabletserver/tx/twopc.go new file mode 100644 index 00000000000..56cfbd1a51f --- /dev/null +++ b/go/vt/vttablet/tabletserver/tx/twopc.go @@ -0,0 +1,39 @@ +/* +Copyright 2020 The Vitess Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package tx + +import ( + "time" + + querypb "vitess.io/vitess/go/vt/proto/query" +) + +// DistributedTx is similar to querypb.TransactionMetadata, but +// is display friendly. +type DistributedTx struct { + Dtid string + State string + Created time.Time + Participants []querypb.Target +} + +// PreparedTx represents a displayable version of a prepared transaction. +type PreparedTx struct { + Dtid string + Queries []string + Time time.Time +} diff --git a/go/vt/vttablet/tabletserver/tx_engine.go b/go/vt/vttablet/tabletserver/tx_engine.go index 51ede88c3e8..c6580b199e0 100644 --- a/go/vt/vttablet/tabletserver/tx_engine.go +++ b/go/vt/vttablet/tabletserver/tx_engine.go @@ -21,6 +21,8 @@ import ( "sync" "time" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" + "golang.org/x/net/context" "vitess.io/vitess/go/timer" @@ -140,39 +142,6 @@ func NewTxEngine(env tabletenv.Env) *TxEngine { return te } -// Stop will stop accepting any new transactions. Transactions are immediately aborted. -func (te *TxEngine) Stop() error { - te.beginRequests.Wait() - te.stateLock.Lock() - - switch te.state { - case NotServing: - // Nothing to do. We are already stopped or stopping - te.stateLock.Unlock() - return nil - - case AcceptingReadAndWrite: - return te.transitionTo(NotServing) - - case AcceptingReadOnly: - // We are not master, so it's safe to kill all read-only transactions - te.close(true) - te.state = NotServing - te.stateLock.Unlock() - return nil - - case Transitioning: - te.nextState = NotServing - te.stateLock.Unlock() - te.blockUntilEndOfTransition() - return nil - - default: - te.stateLock.Unlock() - return te.unknownStateError() - } -} - // AcceptReadWrite will start accepting all transactions. // If transitioning from RO mode, transactions might need to be // rolled back before new transactions can be accepts. @@ -272,14 +241,28 @@ func (te *TxEngine) Begin(ctx context.Context, options *querypb.ExecuteOptions) te.stateLock.Unlock() defer te.beginRequests.Done() - return te.txPool.Begin(ctx, options) + conn, beginSQL, err := te.txPool.Begin(ctx, options) + if err != nil { + return 0, "", err + } + defer conn.Unlock() + return conn.ID(), beginSQL, err } // Commit commits the specified transaction. func (te *TxEngine) Commit(ctx context.Context, transactionID int64) (string, error) { span, ctx := trace.NewSpan(ctx, "TxEngine.Commit") defer span.Finish() - return te.txPool.Commit(ctx, transactionID) + conn, err := te.txPool.GetAndLock(transactionID, "for commit") + if err != nil { + return "", err + } + defer conn.Release(tx.TxCommit) + queries, err := te.txPool.Commit(ctx, conn) + if err != nil { + return "", err + } + return queries, nil } // Rollback rolls back the specified transaction. @@ -287,7 +270,12 @@ func (te *TxEngine) Rollback(ctx context.Context, transactionID int64) error { span, ctx := trace.NewSpan(ctx, "TxEngine.Rollback") defer span.Finish() - return te.txPool.Rollback(ctx, transactionID) + conn, err := te.txPool.GetAndLock(transactionID, "for rollback") + if err != nil { + return err + } + defer conn.Release(tx.TxRollback) + return te.txPool.Rollback(ctx, conn) } func (te *TxEngine) unknownStateError() error { @@ -442,45 +430,45 @@ func (te *TxEngine) prepareFromRedo() error { maxid := int64(0) outer: - for _, tx := range prepared { - txid, err := dtids.TransactionID(tx.Dtid) + for _, preparedTx := range prepared { + txid, err := dtids.TransactionID(preparedTx.Dtid) if err != nil { log.Errorf("Error extracting transaction ID from ditd: %v", err) } if txid > maxid { maxid = txid } - conn, _, err := te.txPool.LocalBegin(ctx, &querypb.ExecuteOptions{}) + conn, _, err := te.txPool.Begin(ctx, &querypb.ExecuteOptions{}) if err != nil { allErr.RecordError(err) continue } - for _, stmt := range tx.Queries { - conn.RecordQuery(stmt) + for _, stmt := range preparedTx.Queries { + conn.TxProperties().RecordQuery(stmt) _, err := conn.Exec(ctx, stmt, 1, false) if err != nil { allErr.RecordError(err) - te.txPool.LocalConclude(ctx, conn) + te.txPool.RollbackAndRelease(ctx, conn) continue outer } } // We should not use the external Prepare because // we don't want to write again to the redo log. - err = te.preparedPool.Put(conn, tx.Dtid) + err = te.preparedPool.Put(conn, preparedTx.Dtid) if err != nil { allErr.RecordError(err) continue } } - for _, tx := range failed { - txid, err := dtids.TransactionID(tx.Dtid) + for _, preparedTx := range failed { + txid, err := dtids.TransactionID(preparedTx.Dtid) if err != nil { log.Errorf("Error extracting transaction ID from ditd: %v", err) } if txid > maxid { maxid = txid } - te.preparedPool.SetFailed(tx.Dtid) + te.preparedPool.SetFailed(preparedTx.Dtid) } te.txPool.AdjustLastID(maxid) log.Infof("Prepared %d transactions, and registered %d failures.", len(prepared), len(failed)) @@ -492,10 +480,8 @@ outer: // This is used for transitioning from a master to a non-master // serving type. func (te *TxEngine) rollbackTransactions() { + te.rollbackPrepared() ctx := tabletenv.LocalContext() - for _, c := range te.preparedPool.FetchAll() { - te.txPool.LocalConclude(ctx, c) - } // The order of rollbacks is currently not material because // we don't allow new statements or commits during // this function. In case of any such change, this will @@ -505,8 +491,9 @@ func (te *TxEngine) rollbackTransactions() { func (te *TxEngine) rollbackPrepared() { ctx := tabletenv.LocalContext() - for _, c := range te.preparedPool.FetchAll() { - te.txPool.LocalConclude(ctx, c) + for _, conn := range te.preparedPool.FetchAll() { + te.txPool.Rollback(ctx, conn) + conn.Release(tx.TxRollback) } } diff --git a/go/vt/vttablet/tabletserver/tx_engine_test.go b/go/vt/vttablet/tabletserver/tx_engine_test.go index 85e80d34557..58d16cd81cb 100644 --- a/go/vt/vttablet/tabletserver/tx_engine_test.go +++ b/go/vt/vttablet/tabletserver/tx_engine_test.go @@ -23,6 +23,10 @@ import ( "testing" "time" + "github.com/stretchr/testify/assert" + + "github.com/stretchr/testify/require" + "vitess.io/vitess/go/mysql/fakesqldb" "vitess.io/vitess/go/sqltypes" @@ -53,14 +57,10 @@ func TestTxEngineClose(t *testing.T) { // Normal close with timeout wait. te.open() - c, beginSQL, err := te.txPool.LocalBegin(ctx, &querypb.ExecuteOptions{}) - if err != nil { - t.Fatal(err) - } - if beginSQL != "begin" { - t.Errorf("beginSQL: %q, want 'begin'", beginSQL) - } - c.Recycle() + c, beginSQL, err := te.txPool.Begin(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) + require.Equal(t, "begin", beginSQL) + c.Unlock() start = time.Now() te.close(false) if diff := time.Since(start); diff < 500*time.Millisecond { @@ -69,11 +69,11 @@ func TestTxEngineClose(t *testing.T) { // Immediate close. te.open() - c, _, err = te.txPool.LocalBegin(ctx, &querypb.ExecuteOptions{}) + c, _, err = te.txPool.Begin(ctx, &querypb.ExecuteOptions{}) if err != nil { t.Fatal(err) } - c.Recycle() + c.Unlock() start = time.Now() te.close(true) if diff := time.Since(start); diff > 500*time.Millisecond { @@ -83,11 +83,11 @@ func TestTxEngineClose(t *testing.T) { // Normal close with short grace period. te.shutdownGracePeriod = 250 * time.Millisecond te.open() - c, _, err = te.txPool.LocalBegin(ctx, &querypb.ExecuteOptions{}) + c, _, err = te.txPool.Begin(ctx, &querypb.ExecuteOptions{}) if err != nil { t.Fatal(err) } - c.Recycle() + c.Unlock() start = time.Now() te.close(false) if diff := time.Since(start); diff > 500*time.Millisecond { @@ -100,18 +100,16 @@ func TestTxEngineClose(t *testing.T) { // Normal close with short grace period, but pool gets empty early. te.shutdownGracePeriod = 250 * time.Millisecond te.open() - c, _, err = te.txPool.LocalBegin(ctx, &querypb.ExecuteOptions{}) + c, _, err = te.txPool.Begin(ctx, &querypb.ExecuteOptions{}) if err != nil { t.Fatal(err) } - c.Recycle() + c.Unlock() go func() { time.Sleep(100 * time.Millisecond) - _, err := te.txPool.Get(c.TransactionID, "return") - if err != nil { - t.Error(err) - } - te.txPool.LocalConclude(ctx, c) + _, err := te.txPool.GetAndLock(c.ID(), "return") + assert.NoError(t, err) + te.txPool.RollbackAndRelease(ctx, c) }() start = time.Now() te.close(false) @@ -124,13 +122,11 @@ func TestTxEngineClose(t *testing.T) { // Immediate close, but connection is in use. te.open() - c, _, err = te.txPool.LocalBegin(ctx, &querypb.ExecuteOptions{}) - if err != nil { - t.Fatal(err) - } + c, _, err = te.txPool.Begin(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) go func() { time.Sleep(100 * time.Millisecond) - te.txPool.LocalConclude(ctx, c) + te.txPool.RollbackAndRelease(ctx, c) }() start = time.Now() te.close(true) @@ -197,7 +193,8 @@ func changeState(te *TxEngine, state txEngineState) error { case AcceptingReadOnly: return te.AcceptReadOnly() case NotServing: - return te.Stop() + te.StopGently() + return nil default: return fmt.Errorf("don't know how to do that: %v", state) } @@ -409,28 +406,24 @@ func TestWithInnerTests(outerT *testing.T) { defer db.Close() te := setupTxEngine(db) - failIfError(t, + require.NoError(t, changeState(te, test.startState)) switch test.tx { case NoTx: // nothing to do case WriteAccepted: - failIfError(t, + require.NoError(t, startTransaction(te, true)) case ReadOnlyAccepted: - failIfError(t, + require.NoError(t, startTransaction(te, false)) case WriteRejected: err := startTransaction(te, true) - if err == nil { - t.Fatalf("expected an error to be returned when opening write transaction, but got nil") - } + require.Error(t, err) case ReadOnlyRejected: err := startTransaction(te, false) - if err == nil { - t.Fatalf("expected an error to be returned when opening read transaction, but got nil") - } + require.Error(t, err) default: t.Fatalf("don't know how to [%v]", test.tx) } @@ -441,7 +434,7 @@ func TestWithInnerTests(outerT *testing.T) { go func(s txEngineState) { defer wg.Done() - failIfError(t, + require.NoError(t, changeState(te, s)) }(newState) @@ -452,7 +445,7 @@ func TestWithInnerTests(outerT *testing.T) { // Let's wait for all transitions to wrap up wg.Wait() - failIfError(t, + require.NoError(t, test.stateAssertion(te.state)) }) } @@ -468,13 +461,6 @@ func setupTxEngine(db *fakesqldb.DB) *TxEngine { return te } -func failIfError(t *testing.T, err error) { - if err != nil { - t.Logf("%+v", err) - t.FailNow() - } -} - func assertEndStateIs(expected txEngineState) func(actual txEngineState) error { return func(actual txEngineState) error { if actual != expected { diff --git a/go/vt/vttablet/tabletserver/tx_executor.go b/go/vt/vttablet/tabletserver/tx_executor.go index 351de1dd805..3eb067f0992 100644 --- a/go/vt/vttablet/tabletserver/tx_executor.go +++ b/go/vt/vttablet/tabletserver/tx_executor.go @@ -19,6 +19,8 @@ package tabletserver import ( "time" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" + "golang.org/x/net/context" "vitess.io/vitess/go/trace" @@ -31,6 +33,7 @@ import ( ) // TxExecutor is used for executing a transactional request. +// TODO: merge this with tx_engine type TxExecutor struct { // TODO(sougou): Parameterize this. ctx context.Context @@ -49,40 +52,27 @@ func (txe *TxExecutor) Prepare(transactionID int64, dtid string) error { defer txe.te.env.Stats().QueryTimings.Record("PREPARE", time.Now()) txe.logStats.TransactionID = transactionID - conn, err := txe.te.txPool.Get(transactionID, "for prepare") + conn, err := txe.te.txPool.GetAndLock(transactionID, "for prepare") if err != nil { return err } // If no queries were executed, we just rollback. - if len(conn.Queries) == 0 { - txe.te.txPool.LocalConclude(txe.ctx, conn) + if len(conn.TxProperties().Queries) == 0 { + conn.Release(tx.TxRollback) return nil } err = txe.te.preparedPool.Put(conn, dtid) if err != nil { - txe.te.txPool.localRollback(txe.ctx, conn) + txe.te.txPool.RollbackAndRelease(txe.ctx, conn) return vterrors.Errorf(vtrpcpb.Code_RESOURCE_EXHAUSTED, "prepare failed for transaction %d: %v", transactionID, err) } - localConn, _, err := txe.te.txPool.LocalBegin(txe.ctx, &querypb.ExecuteOptions{}) - if err != nil { - return err - } - defer txe.te.txPool.LocalConclude(txe.ctx, localConn) + return txe.inTransaction(func(localConn tx.IStatefulConnection) error { + return txe.te.twoPC.SaveRedo(txe.ctx, localConn, dtid, conn.TxProperties().Queries) + }) - err = txe.te.twoPC.SaveRedo(txe.ctx, localConn, dtid, conn.Queries) - if err != nil { - return err - } - - _, err = txe.te.txPool.LocalCommit(txe.ctx, localConn) - if err != nil { - return err - } - - return nil } // CommitPrepared commits a prepared transaction. If the operation @@ -103,13 +93,13 @@ func (txe *TxExecutor) CommitPrepared(dtid string) error { // We have to use a context that will never give up, // even if the original context expires. ctx := trace.CopySpan(context.Background(), txe.ctx) - defer txe.te.txPool.LocalConclude(ctx, conn) + defer txe.te.txPool.RollbackAndRelease(ctx, conn) err = txe.te.twoPC.DeleteRedo(ctx, conn, dtid) if err != nil { txe.markFailed(ctx, dtid) return err } - _, err = txe.te.txPool.LocalCommit(ctx, conn) + _, err = txe.te.txPool.Commit(ctx, conn) if err != nil { txe.markFailed(ctx, dtid) return err @@ -128,19 +118,19 @@ func (txe *TxExecutor) CommitPrepared(dtid string) error { func (txe *TxExecutor) markFailed(ctx context.Context, dtid string) { txe.te.env.Stats().InternalErrors.Add("TwopcCommit", 1) txe.te.preparedPool.SetFailed(dtid) - conn, _, err := txe.te.txPool.LocalBegin(ctx, &querypb.ExecuteOptions{}) + conn, _, err := txe.te.txPool.Begin(ctx, &querypb.ExecuteOptions{}) if err != nil { log.Errorf("markFailed: Begin failed for dtid %s: %v", dtid, err) return } - defer txe.te.txPool.LocalConclude(ctx, conn) + defer txe.te.txPool.RollbackAndRelease(ctx, conn) if err = txe.te.twoPC.UpdateRedo(ctx, conn, dtid, RedoStateFailed); err != nil { log.Errorf("markFailed: UpdateRedo failed for dtid %s: %v", dtid, err) return } - if _, err = txe.te.txPool.LocalCommit(ctx, conn); err != nil { + if _, err = txe.te.txPool.Commit(ctx, conn); err != nil { log.Errorf("markFailed: Commit failed for dtid %s: %v", dtid, err) } } @@ -168,28 +158,17 @@ func (txe *TxExecutor) RollbackPrepared(dtid string, originalID int64) error { return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "2pc is not enabled") } defer txe.te.env.Stats().QueryTimings.Record("ROLLBACK_PREPARED", time.Now()) - conn, _, err := txe.te.txPool.LocalBegin(txe.ctx, &querypb.ExecuteOptions{}) - if err != nil { - goto returnConn - } - defer txe.te.txPool.LocalConclude(txe.ctx, conn) - - err = txe.te.twoPC.DeleteRedo(txe.ctx, conn, dtid) - if err != nil { - goto returnConn - } - - _, err = txe.te.txPool.LocalCommit(txe.ctx, conn) - -returnConn: - if preparedConn := txe.te.preparedPool.FetchForRollback(dtid); preparedConn != nil { - txe.te.txPool.LocalConclude(txe.ctx, preparedConn) - } - if originalID != 0 { - txe.te.txPool.Rollback(txe.ctx, originalID) - } - - return err + defer func() { + if preparedConn := txe.te.preparedPool.FetchForRollback(dtid); preparedConn != nil { + txe.te.txPool.RollbackAndRelease(txe.ctx, preparedConn) + } + if originalID != 0 { + txe.te.Rollback(txe.ctx, originalID) + } + }() + return txe.inTransaction(func(conn tx.IStatefulConnection) error { + return txe.te.twoPC.DeleteRedo(txe.ctx, conn, dtid) + }) } // CreateTransaction creates the metadata for a 2PC transaction. @@ -198,18 +177,9 @@ func (txe *TxExecutor) CreateTransaction(dtid string, participants []*querypb.Ta return vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "2pc is not enabled") } defer txe.te.env.Stats().QueryTimings.Record("CREATE_TRANSACTION", time.Now()) - conn, _, err := txe.te.txPool.LocalBegin(txe.ctx, &querypb.ExecuteOptions{}) - if err != nil { - return err - } - defer txe.te.txPool.LocalConclude(txe.ctx, conn) - - err = txe.te.twoPC.CreateTransaction(txe.ctx, conn, dtid, participants) - if err != nil { - return err - } - _, err = txe.te.txPool.LocalCommit(txe.ctx, conn) - return err + return txe.inTransaction(func(conn tx.IStatefulConnection) error { + return txe.te.twoPC.CreateTransaction(txe.ctx, conn, dtid, participants) + }) } // StartCommit atomically commits the transaction along with the @@ -221,17 +191,17 @@ func (txe *TxExecutor) StartCommit(transactionID int64, dtid string) error { defer txe.te.env.Stats().QueryTimings.Record("START_COMMIT", time.Now()) txe.logStats.TransactionID = transactionID - conn, err := txe.te.txPool.Get(transactionID, "for 2pc commit") + conn, err := txe.te.txPool.GetAndLock(transactionID, "for 2pc commit") if err != nil { return err } - defer txe.te.txPool.LocalConclude(txe.ctx, conn) + defer txe.te.txPool.RollbackAndRelease(txe.ctx, conn) err = txe.te.twoPC.Transition(txe.ctx, conn, dtid, querypb.TransactionState_COMMIT) if err != nil { return err } - _, err = txe.te.txPool.LocalCommit(txe.ctx, conn) + _, err = txe.te.txPool.Commit(txe.ctx, conn) return err } @@ -245,26 +215,12 @@ func (txe *TxExecutor) SetRollback(dtid string, transactionID int64) error { txe.logStats.TransactionID = transactionID if transactionID != 0 { - txe.te.txPool.Rollback(txe.ctx, transactionID) + txe.te.Rollback(txe.ctx, transactionID) } - conn, _, err := txe.te.txPool.LocalBegin(txe.ctx, &querypb.ExecuteOptions{}) - if err != nil { - return err - } - defer txe.te.txPool.LocalConclude(txe.ctx, conn) - - err = txe.te.twoPC.Transition(txe.ctx, conn, dtid, querypb.TransactionState_ROLLBACK) - if err != nil { - return err - } - - _, err = txe.te.txPool.LocalCommit(txe.ctx, conn) - if err != nil { - return err - } - - return nil + return txe.inTransaction(func(conn tx.IStatefulConnection) error { + return txe.te.twoPC.Transition(txe.ctx, conn, dtid, querypb.TransactionState_ROLLBACK) + }) } // ConcludeTransaction deletes the 2pc transaction metadata @@ -275,18 +231,9 @@ func (txe *TxExecutor) ConcludeTransaction(dtid string) error { } defer txe.te.env.Stats().QueryTimings.Record("RESOLVE", time.Now()) - conn, _, err := txe.te.txPool.LocalBegin(txe.ctx, &querypb.ExecuteOptions{}) - if err != nil { - return err - } - defer txe.te.txPool.LocalConclude(txe.ctx, conn) - - err = txe.te.twoPC.DeleteTransaction(txe.ctx, conn, dtid) - if err != nil { - return err - } - _, err = txe.te.txPool.LocalCommit(txe.ctx, conn) - return err + return txe.inTransaction(func(conn tx.IStatefulConnection) error { + return txe.te.twoPC.DeleteTransaction(txe.ctx, conn, dtid) + }) } // ReadTransaction returns the metadata for the sepcified dtid. @@ -298,7 +245,7 @@ func (txe *TxExecutor) ReadTransaction(dtid string) (*querypb.TransactionMetadat } // ReadTwopcInflight returns info about all in-flight 2pc transactions. -func (txe *TxExecutor) ReadTwopcInflight() (distributed []*DistributedTx, prepared, failed []*PreparedTx, err error) { +func (txe *TxExecutor) ReadTwopcInflight() (distributed []*tx.DistributedTx, prepared, failed []*tx.PreparedTx, err error) { if !txe.te.twopcEnabled { return nil, nil, nil, vterrors.Errorf(vtrpcpb.Code_INVALID_ARGUMENT, "2pc is not enabled") } @@ -312,3 +259,22 @@ func (txe *TxExecutor) ReadTwopcInflight() (distributed []*DistributedTx, prepar } return distributed, prepared, failed, nil } + +func (txe *TxExecutor) inTransaction(f func(tx.IStatefulConnection) error) error { + conn, _, err := txe.te.txPool.Begin(txe.ctx, &querypb.ExecuteOptions{}) + if err != nil { + return err + } + defer txe.te.txPool.RollbackAndRelease(txe.ctx, conn) + + err = f(conn) + if err != nil { + return err + } + + _, err = txe.te.txPool.Commit(txe.ctx, conn) + if err != nil { + return err + } + return nil +} diff --git a/go/vt/vttablet/tabletserver/tx_executor_test.go b/go/vt/vttablet/tabletserver/tx_executor_test.go index 2e5f775d396..b8c2c02aa8c 100644 --- a/go/vt/vttablet/tabletserver/tx_executor_test.go +++ b/go/vt/vttablet/tabletserver/tx_executor_test.go @@ -20,10 +20,11 @@ import ( "errors" "fmt" "reflect" - "strings" "testing" "time" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" + "github.com/golang/protobuf/proto" "github.com/stretchr/testify/require" "golang.org/x/net/context" @@ -46,9 +47,7 @@ func TestTxExecutorEmptyPrepare(t *testing.T) { err := txe.Prepare(txid, "aa") require.NoError(t, err) // Nothing should be prepared. - if len(txe.te.preparedPool.conns) != 0 { - t.Errorf("len(txe.te.preparedPool.conns): %d, want 0", len(txe.te.preparedPool.conns)) - } + require.Empty(t, txe.te.preparedPool.conns, "txe.te.preparedPool.conns") } func TestTxExecutorPrepare(t *testing.T) { @@ -73,10 +72,7 @@ func TestTxExecutorPrepareNotInTx(t *testing.T) { defer db.Close() defer tsv.StopService() err := txe.Prepare(0, "aa") - want := "transaction 0: not found" - if err == nil || err.Error() != want { - t.Errorf("Prepare err: %v, want %s", err, want) - } + require.EqualError(t, err, "transaction 0: not found") } func TestTxExecutorPreparePoolFail(t *testing.T) { @@ -89,10 +85,8 @@ func TestTxExecutorPreparePoolFail(t *testing.T) { require.NoError(t, err) defer txe.RollbackPrepared("aa", 0) err = txe.Prepare(txid2, "bb") - want := "prepared transactions exceeded limit" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("Prepare err: %v, must contain %s", err, want) - } + require.Error(t, err) + require.Contains(t, err.Error(), "prepared transactions exceeded limit") } func TestTxExecutorPrepareRedoBeginFail(t *testing.T) { @@ -103,10 +97,8 @@ func TestTxExecutorPrepareRedoBeginFail(t *testing.T) { db.AddRejectedQuery("begin", errors.New("begin fail")) err := txe.Prepare(txid, "aa") defer txe.RollbackPrepared("aa", 0) - want := "begin fail" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("Prepare err: %v, want %s", err, want) - } + require.Error(t, err) + require.Contains(t, err.Error(), "begin fail") } func TestTxExecutorPrepareRedoFail(t *testing.T) { @@ -116,10 +108,8 @@ func TestTxExecutorPrepareRedoFail(t *testing.T) { txid := newTxForPrep(tsv) err := txe.Prepare(txid, "bb") defer txe.RollbackPrepared("bb", 0) - want := "is not supported" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("Prepare err: %v, must contain %s", err, want) - } + require.Error(t, err) + require.Contains(t, err.Error(), "is not supported") } func TestTxExecutorPrepareRedoCommitFail(t *testing.T) { @@ -130,10 +120,8 @@ func TestTxExecutorPrepareRedoCommitFail(t *testing.T) { db.AddRejectedQuery("commit", errors.New("commit fail")) err := txe.Prepare(txid, "aa") defer txe.RollbackPrepared("aa", 0) - want := "commit fail" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("Prepare err: %v, want %s", err, want) - } + require.Error(t, err) + require.Contains(t, err.Error(), "commit fail") } func TestTxExecutorCommit(t *testing.T) { @@ -162,16 +150,12 @@ func TestTxExecutorCommitRedoFail(t *testing.T) { defer txe.RollbackPrepared("bb", 0) db.AddQuery("update _vt.redo_state set state = 'Failed' where dtid = 'bb'", &sqltypes.Result{}) err = txe.CommitPrepared("bb") - want := "is not supported" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("txe.CommitPrepared err: %v, must contain %s", err, want) - } + require.Error(t, err) + require.Contains(t, err.Error(), "is not supported") // A retry should fail differently. err = txe.CommitPrepared("bb") - want = "cannot commit dtid bb, state: failed" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("txe.CommitPrepared err: %v, must contain %s", err, want) - } + require.Error(t, err) + require.Contains(t, err.Error(), "cannot commit dtid bb, state: failed") } func TestTxExecutorCommitRedoCommitFail(t *testing.T) { @@ -184,10 +168,8 @@ func TestTxExecutorCommitRedoCommitFail(t *testing.T) { defer txe.RollbackPrepared("aa", 0) db.AddRejectedQuery("commit", errors.New("commit fail")) err = txe.CommitPrepared("aa") - want := "commit fail" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("Prepare err: %v, want %s", err, want) - } + require.Error(t, err) + require.Contains(t, err.Error(), "commit fail") } func TestTxExecutorRollbackBeginFail(t *testing.T) { @@ -199,10 +181,8 @@ func TestTxExecutorRollbackBeginFail(t *testing.T) { require.NoError(t, err) db.AddRejectedQuery("begin", errors.New("begin fail")) err = txe.RollbackPrepared("aa", txid) - want := "begin fail" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("Prepare err: %v, want %s", err, want) - } + require.Error(t, err) + require.Contains(t, err.Error(), "begin fail") } func TestTxExecutorRollbackRedoFail(t *testing.T) { @@ -215,10 +195,8 @@ func TestTxExecutorRollbackRedoFail(t *testing.T) { err := txe.Prepare(txid, "bb") require.NoError(t, err) err = txe.RollbackPrepared("bb", txid) - want := "is not supported" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("Prepare err: %v, must contain %s", err, want) - } + require.Error(t, err) + require.Contains(t, err.Error(), "is not supported") } func TestExecutorCreateTransaction(t *testing.T) { @@ -249,10 +227,8 @@ func TestExecutorStartCommit(t *testing.T) { db.AddQuery(commitTransition, &sqltypes.Result{}) txid = newTxForPrep(tsv) err = txe.StartCommit(txid, "aa") - want := "could not transition to COMMIT: aa" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("Prepare err: %v, want %s", err, want) - } + require.Error(t, err) + require.Contains(t, err.Error(), "could not transition to COMMIT: aa") } func TestExecutorSetRollback(t *testing.T) { @@ -269,10 +245,8 @@ func TestExecutorSetRollback(t *testing.T) { db.AddQuery(rollbackTransition, &sqltypes.Result{}) txid = newTxForPrep(tsv) err = txe.SetRollback("aa", txid) - want := "could not transition to ROLLBACK: aa" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("Prepare err: %v, want %s", err, want) - } + require.Error(t, err) + require.Contains(t, err.Error(), "could not transition to ROLLBACK: aa") } func TestExecutorConcludeTransaction(t *testing.T) { @@ -409,7 +383,7 @@ func TestExecutorReadAllTransactions(t *testing.T) { }) got, _, _, err := txe.ReadTwopcInflight() require.NoError(t, err) - want := []*DistributedTx{{ + want := []*tx.DistributedTx{{ Dtid: "dtid0", State: "PREPARE", Created: time.Unix(0, 1), @@ -514,15 +488,12 @@ func TestNoTwopc(t *testing.T) { want := "2pc is not enabled" for _, tc := range testcases { err := tc.fun() - if err == nil || err.Error() != want { - t.Errorf("%s: %v, want %s", tc.desc, err, want) - } + require.EqualError(t, err, want) } } func newTestTxExecutor(t *testing.T) (txe *TxExecutor, tsv *TabletServer, db *fakesqldb.DB) { db = setUpQueryExecutorTest(t) - ctx := context.Background() logStats := tabletenv.NewLogStats(ctx, "TestTxExecutor") tsv = newTestTabletServer(ctx, smallTxPool, db) db.AddQueryPattern("insert into _vt\\.redo_state\\(dtid, state, time_created\\) values \\('aa', 1,.*", &sqltypes.Result{}) @@ -540,7 +511,6 @@ func newTestTxExecutor(t *testing.T) (txe *TxExecutor, tsv *TabletServer, db *fa // newShortAgeExecutor is same as newTestTxExecutor, but shorter transaction abandon age. func newShortAgeExecutor(t *testing.T) (txe *TxExecutor, tsv *TabletServer, db *fakesqldb.DB) { db = setUpQueryExecutorTest(t) - ctx := context.Background() logStats := tabletenv.NewLogStats(ctx, "TestTxExecutor") tsv = newTestTabletServer(ctx, smallTxPool|shortTwopcAge, db) db.AddQueryPattern("insert into _vt\\.redo_state\\(dtid, state, time_created\\) values \\('aa', 1,.*", &sqltypes.Result{}) @@ -558,7 +528,6 @@ func newShortAgeExecutor(t *testing.T) (txe *TxExecutor, tsv *TabletServer, db * // newNoTwopcExecutor is same as newTestTxExecutor, but 2pc disabled. func newNoTwopcExecutor(t *testing.T) (txe *TxExecutor, tsv *TabletServer, db *fakesqldb.DB) { db = setUpQueryExecutorTest(t) - ctx := context.Background() logStats := tabletenv.NewLogStats(ctx, "TestTxExecutor") tsv = newTestTabletServer(ctx, noTwopc, db) return &TxExecutor{ @@ -572,7 +541,7 @@ func newNoTwopcExecutor(t *testing.T) (txe *TxExecutor, tsv *TabletServer, db *f func newTxForPrep(tsv *TabletServer) int64 { txid := newTransaction(tsv, nil) target := querypb.Target{TabletType: topodatapb.TabletType_MASTER} - _, err := tsv.Execute(context.Background(), &target, "update test_table set name = 2 where pk = 1", nil, txid, nil) + _, err := tsv.Execute(ctx, &target, "update test_table set name = 2 where pk = 1", nil, txid, nil) if err != nil { panic(err) } diff --git a/go/vt/vttablet/tabletserver/tx_pool.go b/go/vt/vttablet/tabletserver/tx_pool.go index ac03877ef48..27849d5da06 100644 --- a/go/vt/vttablet/tabletserver/tx_pool.go +++ b/go/vt/vttablet/tabletserver/tx_pool.go @@ -17,25 +17,24 @@ limitations under the License. package tabletserver import ( - "fmt" - "strings" "sync" "time" + "vitess.io/vitess/go/pools" + + "vitess.io/vitess/go/vt/servenv" + + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" + "golang.org/x/net/context" - "vitess.io/vitess/go/mysql" - "vitess.io/vitess/go/pools" - "vitess.io/vitess/go/sqltypes" "vitess.io/vitess/go/sync2" "vitess.io/vitess/go/timer" "vitess.io/vitess/go/trace" "vitess.io/vitess/go/vt/callerid" "vitess.io/vitess/go/vt/dbconfigs" "vitess.io/vitess/go/vt/log" - "vitess.io/vitess/go/vt/servenv" "vitess.io/vitess/go/vt/vterrors" - "vitess.io/vitess/go/vt/vttablet/tabletserver/connpool" "vitess.io/vitess/go/vt/vttablet/tabletserver/tabletenv" "vitess.io/vitess/go/vt/vttablet/tabletserver/txlimiter" @@ -43,68 +42,45 @@ import ( vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" ) -// These consts identify how a transaction was resolved. -const ( - TxClose = "close" - TxCommit = "commit" - TxRollback = "rollback" - TxKill = "kill" -) - const txLogInterval = 1 * time.Minute -type queries struct { - setIsolationLevel string - openTransaction string +var txIsolations = map[querypb.ExecuteOptions_TransactionIsolation]queries{ + querypb.ExecuteOptions_DEFAULT: {setIsolationLevel: "", openTransaction: "begin"}, + querypb.ExecuteOptions_REPEATABLE_READ: {setIsolationLevel: "REPEATABLE READ", openTransaction: "begin"}, + querypb.ExecuteOptions_READ_COMMITTED: {setIsolationLevel: "READ COMMITTED", openTransaction: "begin"}, + querypb.ExecuteOptions_READ_UNCOMMITTED: {setIsolationLevel: "READ UNCOMMITTED", openTransaction: "begin"}, + querypb.ExecuteOptions_SERIALIZABLE: {setIsolationLevel: "SERIALIZABLE", openTransaction: "begin"}, + querypb.ExecuteOptions_CONSISTENT_SNAPSHOT_READ_ONLY: {setIsolationLevel: "REPEATABLE READ", openTransaction: "start transaction with consistent snapshot, read only"}, } -var ( - txIsolations = map[querypb.ExecuteOptions_TransactionIsolation]queries{ - querypb.ExecuteOptions_DEFAULT: {setIsolationLevel: "", openTransaction: "begin"}, - querypb.ExecuteOptions_REPEATABLE_READ: {setIsolationLevel: "REPEATABLE READ", openTransaction: "begin"}, - querypb.ExecuteOptions_READ_COMMITTED: {setIsolationLevel: "READ COMMITTED", openTransaction: "begin"}, - querypb.ExecuteOptions_READ_UNCOMMITTED: {setIsolationLevel: "READ UNCOMMITTED", openTransaction: "begin"}, - querypb.ExecuteOptions_SERIALIZABLE: {setIsolationLevel: "SERIALIZABLE", openTransaction: "begin"}, - querypb.ExecuteOptions_CONSISTENT_SNAPSHOT_READ_ONLY: {setIsolationLevel: "REPEATABLE READ", openTransaction: "start transaction with consistent snapshot, read only"}, +type ( + // TxPool does a lot of the transactional operations on StatefulConnections. It does not, with two exceptions, + // concern itself with a connections life cycle. The two exceptions are Begin, which creates a new StatefulConnection, + // and RollbackAndRelease, which does a Release after doing the rollback. + TxPool struct { + env tabletenv.Env + scp *StatefulConnectionPool + transactionTimeout sync2.AtomicDuration + ticks *timer.Timer + limiter txlimiter.TxLimiter + + logMu sync.Mutex + lastLog time.Time + txStats *servenv.TimingsWrapper + } + queries struct { + setIsolationLevel string + openTransaction string } ) -// TxPool is the transaction pool for the query service. -type TxPool struct { - env tabletenv.Env - - // conns is the 'regular' pool. By default, connections - // are pulled from here for starting transactions. - conns *connpool.Pool - - // foundRowsPool is the alternate pool that creates - // connections with CLIENT_FOUND_ROWS flag set. A separate - // pool is needed because this option can only be set at - // connection time. - foundRowsPool *connpool.Pool - activePool *pools.Numbered - lastID sync2.AtomicInt64 - transactionTimeout sync2.AtomicDuration - ticks *timer.Timer - limiter txlimiter.TxLimiter - - txStats *servenv.TimingsWrapper - - // Tracking culprits that cause tx pool full errors. - logMu sync.Mutex - lastLog time.Time -} - // NewTxPool creates a new TxPool. It's not operational until it's Open'd. func NewTxPool(env tabletenv.Env, limiter txlimiter.TxLimiter) *TxPool { config := env.Config() transactionTimeout := time.Duration(config.Oltp.TxTimeoutSeconds * 1e9) axp := &TxPool{ env: env, - conns: connpool.NewPool(env, "TransactionPool", config.TxPool), - foundRowsPool: connpool.NewPool(env, "FoundRowsPool", config.TxPool), - activePool: pools.NewNumbered(), - lastID: sync2.NewAtomicInt64(time.Now().UnixNano()), + scp: NewStatefulConnPool(env), transactionTimeout: sync2.NewAtomicDuration(transactionTimeout), ticks: timer.NewTimer(transactionTimeout / 10), limiter: limiter, @@ -119,237 +95,172 @@ func NewTxPool(env tabletenv.Env, limiter txlimiter.TxLimiter) *TxPool { // Open makes the TxPool operational. This also starts the transaction killer // that will kill long-running transactions. func (tp *TxPool) Open(appParams, dbaParams, appDebugParams dbconfigs.Connector) { - log.Infof("Starting transaction id: %d", tp.lastID) - tp.conns.Open(appParams, dbaParams, appDebugParams) - foundRowsParam, _ := appParams.MysqlParams() - foundRowsParam.EnableClientFoundRows() - appParams = dbconfigs.New(foundRowsParam) - tp.foundRowsPool.Open(appParams, dbaParams, appDebugParams) + tp.scp.Open(appParams, dbaParams, appDebugParams) tp.ticks.Start(func() { tp.transactionKiller() }) } // Close closes the TxPool. A closed pool can be reopened. func (tp *TxPool) Close() { tp.ticks.Stop() - for _, v := range tp.activePool.GetOutdated(time.Duration(0), "for closing") { - conn := v.(*TxConnection) - log.Warningf("killing transaction for shutdown: %s", conn.Format()) - tp.env.Stats().InternalErrors.Add("StrayTransactions", 1) - conn.Close() - conn.conclude(TxClose, "pool closed") - } - tp.conns.Close() - tp.foundRowsPool.Close() + tp.scp.Close() } // AdjustLastID adjusts the last transaction id to be at least // as large as the input value. This will ensure that there are // no dtid collisions with future transactions. func (tp *TxPool) AdjustLastID(id int64) { - if current := tp.lastID.Get(); current < id { - log.Infof("Adjusting transaction id to: %d", id) - tp.lastID.Set(id) - } + tp.scp.AdjustLastID(id) } // RollbackNonBusy rolls back all transactions that are not in use. // Transactions can be in use for situations like executing statements // or in prepared state. func (tp *TxPool) RollbackNonBusy(ctx context.Context) { - for _, v := range tp.activePool.GetOutdated(time.Duration(0), "for transition") { - tp.LocalConclude(ctx, v.(*TxConnection)) + for _, v := range tp.scp.GetOutdated(time.Duration(0), "for transition") { + tp.RollbackAndRelease(ctx, v) } } func (tp *TxPool) transactionKiller() { defer tp.env.LogError() - for _, v := range tp.activePool.GetOutdated(tp.Timeout(), "for tx killer rollback") { - conn := v.(*TxConnection) - log.Warningf("killing transaction (exceeded timeout: %v): %s", tp.Timeout(), conn.Format()) + for _, conn := range tp.scp.GetOutdated(tp.Timeout(), "for tx killer rollback") { + log.Warningf("killing transaction (exceeded timeout: %v): %s", tp.Timeout(), conn.String()) tp.env.Stats().KillCounters.Add("Transactions", 1) conn.Close() - conn.conclude(TxKill, fmt.Sprintf("exceeded timeout: %v", tp.Timeout())) + conn.Releasef("exceeded timeout: %v", tp.Timeout()) } } // WaitForEmpty waits until all active transactions are completed. func (tp *TxPool) WaitForEmpty() { - tp.activePool.WaitForEmpty() + tp.scp.WaitForEmpty() } -// Begin begins a transaction, and returns the associated transaction id and -// the statements (if any) executed to initiate the transaction. In autocommit -// mode the statement will be "". -// -// Subsequent statements can access the connection through the transaction id. -func (tp *TxPool) Begin(ctx context.Context, options *querypb.ExecuteOptions) (int64, string, error) { - span, ctx := trace.NewSpan(ctx, "TxPool.Begin") - defer span.Finish() - var conn *connpool.DBConn - var err error - immediateCaller := callerid.ImmediateCallerIDFromContext(ctx) - effectiveCaller := callerid.EffectiveCallerIDFromContext(ctx) - - if !tp.limiter.Get(immediateCaller, effectiveCaller) { - return 0, "", vterrors.Errorf(vtrpcpb.Code_RESOURCE_EXHAUSTED, "per-user transaction pool connection limit exceeded") +//NewTxProps creates a new TxProperties struct +func (tp *TxPool) NewTxProps(immediateCaller *querypb.VTGateCallerID, effectiveCaller *vtrpcpb.CallerID, autocommit bool) *tx.Properties { + return &tx.Properties{ + StartTime: time.Now(), + EffectiveCaller: effectiveCaller, + ImmediateCaller: immediateCaller, + Autocommit: autocommit, + Stats: tp.txStats, } +} - var beginSucceeded bool - defer func() { - if beginSucceeded { - return - } - - if conn != nil { - conn.Recycle() - } - tp.limiter.Release(immediateCaller, effectiveCaller) - }() - - if options.GetClientFoundRows() { - conn, err = tp.foundRowsPool.Get(ctx) - } else { - conn, err = tp.conns.Get(ctx) - } +// GetAndLock fetches the connection associated to the transactionID and blocks it from concurrent use +// You must call Unlock on TxConnection once done. +func (tp *TxPool) GetAndLock(connID tx.ConnID, reason string) (tx.IStatefulConnection, error) { + conn, err := tp.scp.GetAndLock(connID, reason) if err != nil { - switch err { - case connpool.ErrConnPoolClosed: - return 0, "", err - case pools.ErrCtxTimeout: - tp.LogActive() - return 0, "", vterrors.Errorf(vtrpcpb.Code_RESOURCE_EXHAUSTED, "transaction pool aborting request due to already expired context") - case pools.ErrTimeout: - tp.LogActive() - return 0, "", vterrors.Errorf(vtrpcpb.Code_RESOURCE_EXHAUSTED, "transaction pool connection limit exceeded") - } - return 0, "", err + return nil, vterrors.Errorf(vtrpcpb.Code_ABORTED, "transaction %d: %v", connID, err) } - - autocommitTransaction := false - beginQueries := "" - if queries, ok := txIsolations[options.GetTransactionIsolation()]; ok { - if queries.setIsolationLevel != "" { - if _, err := conn.Exec(ctx, "set transaction isolation level "+queries.setIsolationLevel, 1, false); err != nil { - return 0, "", err - } - - beginQueries = queries.setIsolationLevel + "; " - } - - if _, err := conn.Exec(ctx, queries.openTransaction, 1, false); err != nil { - return 0, "", err - } - beginQueries = beginQueries + queries.openTransaction - } else if options.GetTransactionIsolation() == querypb.ExecuteOptions_AUTOCOMMIT { - autocommitTransaction = true - } else { - return 0, "", fmt.Errorf("don't know how to open a transaction of this type: %v", options.GetTransactionIsolation()) - } - - beginSucceeded = true - transactionID := tp.lastID.Add(1) - tp.activePool.Register( - transactionID, - newTxConnection( - conn, - transactionID, - tp, - immediateCaller, - effectiveCaller, - autocommitTransaction, - ), - options.GetWorkload() != querypb.ExecuteOptions_DBA, - ) - return transactionID, beginQueries, nil + return conn, nil } -// Commit commits the specified transaction. -func (tp *TxPool) Commit(ctx context.Context, transactionID int64) (string, error) { +// Commit commits the transaction on the connection. +func (tp *TxPool) Commit(ctx context.Context, txConn tx.IStatefulConnection) (string, error) { + if !txConn.IsInTransaction() { + return "", vterrors.New(vtrpcpb.Code_INTERNAL, "not in a transaction") + } span, ctx := trace.NewSpan(ctx, "TxPool.Commit") defer span.Finish() - conn, err := tp.Get(transactionID, "for commit") - if err != nil { - return "", err + defer tp.txComplete(txConn, tx.TxCommit) + if txConn.TxProperties().Autocommit { + return "", nil } - return tp.LocalCommit(ctx, conn) -} -// Rollback rolls back the specified transaction. -func (tp *TxPool) Rollback(ctx context.Context, transactionID int64) error { - span, ctx := trace.NewSpan(ctx, "TxPool.Rollback") - defer span.Finish() - - conn, err := tp.Get(transactionID, "for rollback") - if err != nil { - return err + if _, err := txConn.Exec(ctx, "commit", 1, false); err != nil { + txConn.Close() + return "", err } - return tp.localRollback(ctx, conn) + return "commit", nil } -// Get fetches the connection associated to the transactionID. -// You must call Recycle on TxConnection once done. -func (tp *TxPool) Get(transactionID int64, reason string) (*TxConnection, error) { - v, err := tp.activePool.Get(transactionID, reason) - if err != nil { - return nil, vterrors.Errorf(vtrpcpb.Code_ABORTED, "transaction %d: %v", transactionID, err) +// RollbackAndRelease rolls back the transaction on the specified connection, and releases the connection when done +func (tp *TxPool) RollbackAndRelease(ctx context.Context, txConn tx.IStatefulConnection) { + defer txConn.Release(tx.TxRollback) + rollbackError := tp.Rollback(ctx, txConn) + if rollbackError != nil { + log.Errorf("tried to rollback, but failed with: %v", rollbackError.Error()) } - return v.(*TxConnection), nil } -// LocalBegin is equivalent to Begin->Get. -// It's used for executing transactions within a request. It's safe -// to always call LocalConclude at the end. -func (tp *TxPool) LocalBegin(ctx context.Context, options *querypb.ExecuteOptions) (*TxConnection, string, error) { - span, ctx := trace.NewSpan(ctx, "TxPool.LocalBegin") +// Rollback rolls back the transaction on the specified connection. +func (tp *TxPool) Rollback(ctx context.Context, txConn tx.IStatefulConnection) error { + span, ctx := trace.NewSpan(ctx, "TxPool.Rollback") defer span.Finish() - - transactionID, beginSQL, err := tp.Begin(ctx, options) - if err != nil { - return nil, "", err + if txConn.IsClosed() || !txConn.IsInTransaction() { + return nil } - conn, err := tp.Get(transactionID, "for local query") - return conn, beginSQL, err + if txConn.TxProperties().Autocommit { + tp.txComplete(txConn, tx.TxCommit) + return nil + } + defer tp.txComplete(txConn, tx.TxRollback) + if _, err := txConn.Exec(ctx, "rollback", 1, false); err != nil { + txConn.Close() + return err + } + return nil } -// LocalCommit is the commit function for LocalBegin. -func (tp *TxPool) LocalCommit(ctx context.Context, conn *TxConnection) (string, error) { - span, ctx := trace.NewSpan(ctx, "TxPool.LocalCommit") +// Begin begins a transaction, and returns the associated connection and +// the statements (if any) executed to initiate the transaction. In autocommit +// mode the statement will be "". +// The connection returned is locked for the callee and its responsibility is to unlock the connection. +func (tp *TxPool) Begin(ctx context.Context, options *querypb.ExecuteOptions) (tx.IStatefulConnection, string, error) { + span, ctx := trace.NewSpan(ctx, "TxPool.Begin") defer span.Finish() - defer conn.conclude(TxCommit, "transaction committed") + beginQueries := "" - if conn.Autocommit { - return "", nil - } + immediateCaller := callerid.ImmediateCallerIDFromContext(ctx) + effectiveCaller := callerid.EffectiveCallerIDFromContext(ctx) - if _, err := conn.Exec(ctx, "commit", 1, false); err != nil { - conn.Close() - return "", err + if !tp.limiter.Get(immediateCaller, effectiveCaller) { + return nil, "", vterrors.Errorf(vtrpcpb.Code_RESOURCE_EXHAUSTED, "per-user transaction pool connection limit exceeded") } - return "commit", nil -} -// LocalConclude concludes a transaction started by LocalBegin. -// If the transaction was not previously concluded, it's rolled back. -func (tp *TxPool) LocalConclude(ctx context.Context, conn *TxConnection) { - if conn.dbConn == nil { - return + conn, err := tp.scp.NewConn(ctx, options) + if err != nil { + switch err { + case pools.ErrCtxTimeout: + tp.LogActive() + err = vterrors.Errorf(vtrpcpb.Code_RESOURCE_EXHAUSTED, "transaction pool aborting request due to already expired context") + case pools.ErrTimeout: + tp.LogActive() + err = vterrors.Errorf(vtrpcpb.Code_RESOURCE_EXHAUSTED, "transaction pool connection limit exceeded") + } + return nil, "", err } - span, ctx := trace.NewSpan(ctx, "TxPool.LocalConclude") - defer span.Finish() - _ = tp.localRollback(ctx, conn) -} - -func (tp *TxPool) localRollback(ctx context.Context, conn *TxConnection) error { - if conn.Autocommit { - conn.conclude(TxCommit, "returned to pool") + err = func() error { + autocommitTransaction := false + if queries, ok := txIsolations[options.GetTransactionIsolation()]; ok { + if queries.setIsolationLevel != "" { + txQuery := "set transaction isolation level " + queries.setIsolationLevel + if err := conn.execWithRetry(ctx, txQuery, 1, false); err != nil { + return vterrors.Wrap(err, txQuery) + } + beginQueries = queries.setIsolationLevel + "; " + } + if err := conn.execWithRetry(ctx, queries.openTransaction, 1, false); err != nil { + return vterrors.Wrap(err, queries.openTransaction) + } + beginQueries = beginQueries + queries.openTransaction + } else if options.GetTransactionIsolation() == querypb.ExecuteOptions_AUTOCOMMIT { + autocommitTransaction = true + } else { + return vterrors.Errorf(vtrpcpb.Code_INTERNAL, "don't know how to open a transaction of this type: %v", options.GetTransactionIsolation()) + } + conn.txProps = tp.NewTxProps(immediateCaller, effectiveCaller, autocommitTransaction) return nil - } - defer conn.conclude(TxRollback, "transaction rolled back") - if _, err := conn.Exec(ctx, "rollback", 1, false); err != nil { + }() + if err != nil { conn.Close() - return err + conn.Release(tx.ConnInitFail) + return nil, "", err } - return nil + + return conn, beginQueries, nil } // LogActive causes all existing transactions to be logged when they complete. @@ -361,10 +272,9 @@ func (tp *TxPool) LogActive() { return } tp.lastLog = time.Now() - conns := tp.activePool.GetAll() - for _, c := range conns { - c.(*TxConnection).LogToFile.Set(1) - } + tp.scp.ForAllTxProperties(func(props *tx.Properties) { + props.LogToFile = true + }) } // Timeout returns the transaction timeout. @@ -378,140 +288,29 @@ func (tp *TxPool) SetTimeout(timeout time.Duration) { tp.ticks.SetInterval(timeout / 10) } -// TxConnection is meant for executing transactions. It can return itself to -// the tx pool correctly. It also does not retry statements if there -// are failures. -type TxConnection struct { - dbConn *connpool.DBConn - TransactionID int64 - pool *TxPool - StartTime time.Time - EndTime time.Time - Queries []string - Conclusion string - LogToFile sync2.AtomicInt32 - ImmediateCallerID *querypb.VTGateCallerID - EffectiveCallerID *vtrpcpb.CallerID - Autocommit bool +func (tp *TxPool) txComplete(conn tx.IStatefulConnection, reason tx.ReleaseReason) { + tp.log(conn, reason) + tp.limiter.Release(conn.TxProperties().ImmediateCaller, conn.TxProperties().EffectiveCaller) + conn.CleanTxState() } -func newTxConnection(conn *connpool.DBConn, transactionID int64, pool *TxPool, immediate *querypb.VTGateCallerID, effective *vtrpcpb.CallerID, autocommit bool) *TxConnection { - return &TxConnection{ - dbConn: conn, - TransactionID: transactionID, - pool: pool, - StartTime: time.Now(), - ImmediateCallerID: immediate, - EffectiveCallerID: effective, - Autocommit: autocommit, +func (tp *TxPool) log(txc tx.IStatefulConnection, reason tx.ReleaseReason) { + if txc.TxProperties() == nil { + return //Nothing to log as no transaction exists on this connection. } -} + txc.TxProperties().Conclusion = reason.Name() + txc.TxProperties().EndTime = time.Now() -// Close closes the connection. -func (txc *TxConnection) Close() { - if txc.dbConn != nil { - txc.dbConn.Close() - } -} - -// Exec executes the statement for the current transaction. -func (txc *TxConnection) Exec(ctx context.Context, query string, maxrows int, wantfields bool) (*sqltypes.Result, error) { - if txc.dbConn == nil { - return nil, vterrors.Errorf(vtrpcpb.Code_ABORTED, "transaction was aborted: %v", txc.Conclusion) - } - r, err := txc.dbConn.ExecOnce(ctx, query, maxrows, wantfields) - if err != nil { - if mysql.IsConnErr(err) { - select { - case <-ctx.Done(): - // If the context is done, the query was killed. - // So, don't trigger a mysql check. - default: - txc.pool.env.CheckMySQL() - } - } - return nil, err - } - return r, nil -} - -// BeginAgain commits the existing transaction and begins a new one -func (txc *TxConnection) BeginAgain(ctx context.Context) error { - if txc.dbConn == nil || txc.Autocommit { - return nil - } - if _, err := txc.dbConn.Exec(ctx, "commit", 1, false); err != nil { - return err - } - if _, err := txc.dbConn.Exec(ctx, "begin", 1, false); err != nil { - return err - } - return nil -} - -// Recycle returns the connection to the pool. The transaction remains -// active. -func (txc *TxConnection) Recycle() { - if txc.dbConn == nil { - return - } - if txc.dbConn.IsClosed() { - txc.conclude(TxClose, "closed") - } else { - txc.pool.activePool.Put(txc.TransactionID) - } -} - -// RecordQuery records the query against this transaction. -func (txc *TxConnection) RecordQuery(query string) { - txc.Queries = append(txc.Queries, query) -} - -func (txc *TxConnection) conclude(conclusion, reason string) { - if txc.dbConn == nil { - return - } - txc.pool.activePool.Unregister(txc.TransactionID, reason) - txc.dbConn.Recycle() - txc.dbConn = nil - txc.pool.limiter.Release(txc.ImmediateCallerID, txc.EffectiveCallerID) - txc.log(conclusion) -} - -func (txc *TxConnection) log(conclusion string) { - txc.Conclusion = conclusion - txc.EndTime = time.Now() - - username := callerid.GetPrincipal(txc.EffectiveCallerID) + username := callerid.GetPrincipal(txc.TxProperties().EffectiveCaller) if username == "" { - username = callerid.GetUsername(txc.ImmediateCallerID) + username = callerid.GetUsername(txc.TxProperties().ImmediateCaller) } - duration := txc.EndTime.Sub(txc.StartTime) - txc.pool.env.Stats().UserTransactionCount.Add([]string{username, conclusion}, 1) - txc.pool.env.Stats().UserTransactionTimesNs.Add([]string{username, conclusion}, int64(duration)) - txc.pool.txStats.Add(conclusion, duration) - if txc.LogToFile.Get() != 0 { - log.Infof("Logged transaction: %s", txc.Format()) + duration := txc.TxProperties().EndTime.Sub(txc.TxProperties().StartTime) + txc.Stats().UserTransactionCount.Add([]string{username, reason.Name()}, 1) + txc.Stats().UserTransactionTimesNs.Add([]string{username, reason.Name()}, int64(duration)) + txc.TxProperties().Stats.Add(reason.Name(), duration) + if txc.TxProperties().LogToFile { + log.Infof("Logged transaction: %s", txc.String()) } tabletenv.TxLogger.Send(txc) } - -// EventTime returns the time the event was created. -func (txc *TxConnection) EventTime() time.Time { - return txc.EndTime -} - -// Format returns a printable version of the connection info. -func (txc *TxConnection) Format() string { - return fmt.Sprintf( - "%v\t'%v'\t'%v'\t%v\t%v\t%.6f\t%v\t%v\t\n", - txc.TransactionID, - callerid.GetPrincipal(txc.EffectiveCallerID), - callerid.GetUsername(txc.ImmediateCallerID), - txc.StartTime.Format(time.StampMicro), - txc.EndTime.Format(time.StampMicro), - txc.EndTime.Sub(txc.StartTime).Seconds(), - txc.Conclusion, - strings.Join(txc.Queries, ";"), - ) -} diff --git a/go/vt/vttablet/tabletserver/tx_pool_test.go b/go/vt/vttablet/tabletserver/tx_pool_test.go index df49e45f26e..26e24eb6268 100644 --- a/go/vt/vttablet/tabletserver/tx_pool_test.go +++ b/go/vt/vttablet/tabletserver/tx_pool_test.go @@ -17,330 +17,155 @@ limitations under the License. package tabletserver import ( + "context" "fmt" - "strings" "testing" "time" - "vitess.io/vitess/go/vt/vttablet/tabletserver/txlimiter" + vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" + "vitess.io/vitess/go/vt/vterrors" - "golang.org/x/net/context" + "gotest.tools/assert" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" + + "github.com/stretchr/testify/require" + + "vitess.io/vitess/go/vt/vttablet/tabletserver/txlimiter" - "vitess.io/vitess/go/mysql" "vitess.io/vitess/go/mysql/fakesqldb" "vitess.io/vitess/go/sqltypes" - "vitess.io/vitess/go/vt/vterrors" "vitess.io/vitess/go/vt/vttablet/tabletserver/tabletenv" - "regexp" - querypb "vitess.io/vitess/go/vt/proto/query" - vtrpcpb "vitess.io/vitess/go/vt/proto/vtrpc" ) func TestTxPoolExecuteCommit(t *testing.T) { - sql := "update test_column set x=1 where 1!=1" - db := fakesqldb.New(t) - defer db.Close() - db.AddQuery(sql, &sqltypes.Result{}) - db.AddQuery("begin", &sqltypes.Result{}) - db.AddQuery("commit", &sqltypes.Result{}) + db, txPool, closer := setup(t) + defer closer() - txPool := newTxPool() - txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - defer txPool.Close() - ctx := context.Background() - transactionID, beginSQL, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) - if err != nil { - t.Fatal(err) - } - if beginSQL != "begin" { - t.Errorf("beginSQL got %q want 'begin'", beginSQL) - } - txConn, err := txPool.Get(transactionID, "for query") - if err != nil { - t.Fatal(err) - } - txConn.RecordQuery(sql) - _, _ = txConn.Exec(ctx, sql, 1, true) - txConn.Recycle() + sql := "select 'this is a query'" + // begin a transaction and then return the connection + conn, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) - commitSQL, err := txPool.Commit(ctx, transactionID) - if err != nil { - t.Fatal(err) - } - if commitSQL != "commit" { - t.Errorf("commitSQL got %q want 'commit'", commitSQL) - } -} + id := conn.ID() + conn.Unlock() -func TestTxPoolExecuteRollback(t *testing.T) { - sql := "alter table test_table add test_column int" - db := fakesqldb.New(t) - defer db.Close() - db.AddQuery(sql, &sqltypes.Result{}) - db.AddQuery("begin", &sqltypes.Result{}) - db.AddQuery("rollback", &sqltypes.Result{}) + // get the connection and execute a query on it + conn2, err := txPool.GetAndLock(id, "") + require.NoError(t, err) + _, _ = conn2.Exec(ctx, sql, 1, true) + conn2.Unlock() - txPool := newTxPool() - txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - defer txPool.Close() - ctx := context.Background() - transactionID, beginSQL, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) - if err != nil { - t.Fatal(err) - } - if beginSQL != "begin" { - t.Errorf("beginSQL got %q want 'begin'", beginSQL) - } - txConn, err := txPool.Get(transactionID, "for query") - if err != nil { - t.Fatal(err) - } - defer txPool.Rollback(ctx, transactionID) - txConn.RecordQuery(sql) - _, err = txConn.Exec(ctx, sql, 1, true) - txConn.Recycle() - if err != nil { - t.Fatalf("got error: %v", err) - } -} + // get the connection again and now commit it + conn3, err := txPool.GetAndLock(id, "") + require.NoError(t, err) -func TestTxPoolRollbackNonBusy(t *testing.T) { - db := fakesqldb.New(t) - defer db.Close() - db.AddQuery("begin", &sqltypes.Result{}) - db.AddQuery("rollback", &sqltypes.Result{}) + _, err = txPool.Commit(ctx, conn3) + require.NoError(t, err) - txPool := newTxPool() - txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - defer txPool.Close() - ctx := context.Background() - txid1, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) - if err != nil { - t.Fatal(err) - } - _, _, err = txPool.Begin(ctx, &querypb.ExecuteOptions{}) - if err != nil { - t.Fatal(err) - } - conn1, err := txPool.Get(txid1, "for query") - if err != nil { - t.Fatal(err) - } - // This should rollback only txid2. - txPool.RollbackNonBusy(ctx) - if sz := txPool.activePool.Size(); sz != 1 { - t.Errorf("txPool.activePool.Size(): %d, want 1", sz) - } - conn1.Recycle() - // This should rollback txid1. - txPool.RollbackNonBusy(ctx) - if sz := txPool.activePool.Size(); sz != 0 { - t.Errorf("txPool.activePool.Size(): %d, want 0", sz) - } + // try committing again. this should fail + _, err = txPool.Commit(ctx, conn) + require.EqualError(t, err, "not in a transaction") + + // wrap everything up and assert + assert.Equal(t, "begin;"+sql+";commit", db.QueryLog()) + conn3.Release(tx.TxCommit) } -func TestTxPoolTransactionKillerEnforceTimeoutEnabled(t *testing.T) { - sqlWithTimeout := "alter table test_table add test_column int" - sqlWithoutTimeout := "alter table test_table add test_column_no_timeout int" - db := fakesqldb.New(t) - defer db.Close() - db.AddQuery(sqlWithTimeout, &sqltypes.Result{}) - db.AddQuery(sqlWithoutTimeout, &sqltypes.Result{}) - db.AddQuery("begin", &sqltypes.Result{}) - db.AddQuery("rollback", &sqltypes.Result{}) +func TestTxPoolExecuteRollback(t *testing.T) { + db, txPool, closer := setup(t) + defer closer() - txPool := newTxPool() - // make sure transaction killer will run frequent enough - txPool.SetTimeout(1 * time.Millisecond) - txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - defer txPool.Close() - ctx := context.Background() - killCount := txPool.env.Stats().KillCounters.Counts()["Transactions"] + conn, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) + defer conn.Release(tx.TxRollback) - txWithoutTimeout, err := addQuery(ctx, sqlWithoutTimeout, txPool, querypb.ExecuteOptions_DBA) - if err != nil { - t.Fatal(err) - } + err = txPool.Rollback(ctx, conn) + require.NoError(t, err) - if _, err := addQuery(ctx, sqlWithTimeout, txPool, querypb.ExecuteOptions_UNSPECIFIED); err != nil { - t.Fatal(err) - } + // try rolling back again, this should be no-op. + err = txPool.Rollback(ctx, conn) + require.NoError(t, err, "not in a transaction") - var ( - killCountDiff int64 - expectedKills = int64(1) - timeoutCh = time.After(5 * time.Second) - ) - - // transaction killer should kill the query the second query - for { - killCountDiff = txPool.env.Stats().KillCounters.Counts()["Transactions"] - killCount - if killCountDiff >= expectedKills { - break - } + assert.Equal(t, "begin;rollback", db.QueryLog()) +} - select { - case <-timeoutCh: - txPool.Rollback(ctx, txWithoutTimeout) - t.Fatal("waited too long for timed transaction to be killed by transaction killer") - default: - } - } +func TestTxPoolExecuteRollbackOnClosedConn(t *testing.T) { + db, txPool, closer := setup(t) + defer closer() - if killCountDiff > expectedKills { - t.Fatalf("expected only %v query to be killed, but got %v killed", expectedKills, killCountDiff) - } + conn, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) + defer conn.Release(tx.TxRollback) - txPool.Rollback(ctx, txWithoutTimeout) - txPool.WaitForEmpty() + conn.Close() - if got, expected := db.GetQueryCalledNum("begin"), 2; got != expected { - t.Fatalf("'begin' called: got=%v, expected=%v", got, expected) - } - if got, expected := db.GetQueryCalledNum(sqlWithoutTimeout), 1; got != expected { - t.Fatalf("'%v' called: got=%v, expected=%v", sqlWithoutTimeout, got, expected) - } - if got, expected := db.GetQueryCalledNum(sqlWithTimeout), 1; got != expected { - t.Fatalf("'%v' called: got=%v, expected=%v", sqlWithTimeout, got, expected) - } - if got, expected := db.GetQueryCalledNum("rollback"), 1; got != expected { - t.Fatalf("'rollback' called: got=%v, expected=%v", got, expected) - } + // rollback should not be logged. + err = txPool.Rollback(ctx, conn) + require.NoError(t, err) -} -func addQuery(ctx context.Context, sql string, txPool *TxPool, workload querypb.ExecuteOptions_Workload) (int64, error) { - transactionID, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{Workload: workload}) - if err != nil { - return 0, err - } - txConn, err := txPool.Get(transactionID, "for query") - if err != nil { - return 0, err - } - txConn.Exec(ctx, sql, 1, false) - txConn.Recycle() - return transactionID, nil + assert.Equal(t, "begin", db.QueryLog()) } -func TestTxPoolClientRowsFound(t *testing.T) { - db := fakesqldb.New(t) - defer db.Close() - db.AddQuery("begin", &sqltypes.Result{}) - txPool := newTxPool() - txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - ctx := context.Background() +func TestTxPoolRollbackNonBusy(t *testing.T) { + db, txPool, closer := setup(t) + defer closer() - startNormalSize := txPool.conns.Available() - startFoundRowsSize := txPool.foundRowsPool.Available() + // start two transactions, and mark one of them as unused + conn1, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) + conn2, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) + conn2.Unlock() // this marks conn2 as NonBusy - // Start a 'normal' transaction. It should take a connection - // for the normal 'conns' pool. - id1, beginSQL, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) - if err != nil { - t.Fatal(err) - } - if beginSQL != "begin" { - t.Errorf("beginSQL got %q want 'begin'", beginSQL) - } - if got, want := txPool.conns.Available(), startNormalSize-1; got != want { - t.Errorf("Normal pool size: %d, want %d", got, want) - } - if got, want := txPool.foundRowsPool.Available(), startFoundRowsSize; got != want { - t.Errorf("foundRows pool size: %d, want %d", got, want) - } + // This should rollback only txid2. + txPool.RollbackNonBusy(ctx) - // Start a 'foundRows' transaction. It should take a connection - // from the foundRows pool. - id2, beginSQL, err := txPool.Begin(ctx, &querypb.ExecuteOptions{ClientFoundRows: true}) - if err != nil { - t.Fatal(err) - } - if beginSQL != "begin" { - t.Errorf("beginSQL got %q want 'begin'", beginSQL) - } - if got, want := txPool.conns.Available(), startNormalSize-1; got != want { - t.Errorf("Normal pool size: %d, want %d", got, want) - } - if got, want := txPool.foundRowsPool.Available(), startFoundRowsSize-1; got != want { - t.Errorf("foundRows pool size: %d, want %d", got, want) - } + // committing tx1 should not be an issue + _, err = txPool.Commit(ctx, conn1) + require.NoError(t, err) - // Rollback the first transaction. The conn should be returned to - // the conns pool. - txPool.Rollback(ctx, id1) - if got, want := txPool.conns.Available(), startNormalSize; got != want { - t.Errorf("Normal pool size: %d, want %d", got, want) - } - if got, want := txPool.foundRowsPool.Available(), startFoundRowsSize-1; got != want { - t.Errorf("foundRows pool size: %d, want %d", got, want) - } + // Trying to get back to conn2 should not work since the transaction has been rolled back + _, err = txPool.GetAndLock(conn2.ID(), "") + require.Error(t, err) - // Rollback the second transaction. The conn should be returned to - // the foundRows pool. - txPool.Rollback(ctx, id2) - if got, want := txPool.conns.Available(), startNormalSize; got != want { - t.Errorf("Normal pool size: %d, want %d", got, want) - } - if got, want := txPool.foundRowsPool.Available(), startFoundRowsSize; got != want { - t.Errorf("foundRows pool size: %d, want %d", got, want) - } + conn1.Release(tx.TxCommit) + assert.Equal(t, "begin;begin;rollback;commit", db.QueryLog()) } func TestTxPoolTransactionIsolation(t *testing.T) { - db := fakesqldb.New(t) - defer db.Close() - db.AddQuery("begin", &sqltypes.Result{}) - txPool := newTxPool() - txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - ctx := context.Background() + db, txPool, closer := setup(t) + defer closer() - // Start a transaction with default. It should not change isolation. - _, beginSQL, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) - if err != nil { - t.Fatal(err) - } - if beginSQL != "begin" { - t.Errorf("beginSQL got %q want 'begin'", beginSQL) - } + c2, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{TransactionIsolation: querypb.ExecuteOptions_READ_COMMITTED}) + require.NoError(t, err) + c2.Release(tx.TxClose) - db.AddQuery("set transaction isolation level READ COMMITTED", &sqltypes.Result{}) - _, beginSQL, err = txPool.Begin(ctx, &querypb.ExecuteOptions{TransactionIsolation: querypb.ExecuteOptions_READ_COMMITTED}) - if err != nil { - t.Fatal(err) - } - wantBeginSQL := "READ COMMITTED; begin" - if beginSQL != wantBeginSQL { - t.Errorf("beginSQL got %q want %q", beginSQL, wantBeginSQL) - } + assert.Equal(t, "set transaction isolation level read committed;begin", db.QueryLog()) } func TestTxPoolAutocommit(t *testing.T) { - db := fakesqldb.New(t) - defer db.Close() - txPool := newTxPool() - txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - ctx := context.Background() + db, txPool, closer := setup(t) + defer closer() // Start a transaction with autocommit. This will ensure that the executor does not send begin/commit statements // to mysql. // This test is meaningful because if txPool.Begin were to send a BEGIN statement to the connection, it will fatal // because is not in the list of expected queries (i.e db.AddQuery hasn't been called). - txid, beginSQL, err := txPool.Begin(ctx, &querypb.ExecuteOptions{TransactionIsolation: querypb.ExecuteOptions_AUTOCOMMIT}) - if err != nil { - t.Fatal(err) - } - if beginSQL != "" { - t.Errorf("beginSQL got %q want ''", beginSQL) - } - commitSQL, err := txPool.Commit(ctx, txid) - if err != nil { - t.Fatal(err) - } - if commitSQL != "" { - t.Errorf("commitSQL got %q want ''", commitSQL) - } + conn1, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{TransactionIsolation: querypb.ExecuteOptions_AUTOCOMMIT}) + require.NoError(t, err) + + // run a query to see it in the query log + query := "select 3" + conn1.Exec(ctx, query, 1, false) + + _, err = txPool.Commit(ctx, conn1) + require.NoError(t, err) + conn1.Release(tx.TxCommit) + + // finally, we should only see the query, no begin/commit + assert.Equal(t, query, db.QueryLog()) } // TestTxPoolBeginWithPoolConnectionError_TransientErrno2006 tests the case @@ -348,214 +173,131 @@ func TestTxPoolAutocommit(t *testing.T) { // db connection. DBConn.Exec() is going to reconnect and retry automatically // due to this connection error and the BEGIN will succeed. func TestTxPoolBeginWithPoolConnectionError_Errno2006_Transient(t *testing.T) { - db, txPool, err := primeTxPoolWithConnection(t) - if err != nil { - t.Fatal(err) - } - defer db.Close() - defer txPool.Close() - - // Close the connection on the server side. - db.CloseAllConnections() - if err := db.WaitForClose(2 * time.Second); err != nil { - t.Fatal(err) - } - - ctx := context.Background() - txConn, _, err := txPool.LocalBegin(ctx, &querypb.ExecuteOptions{}) - if err != nil { - t.Fatalf("Begin should have succeeded after the retry in DBConn.Exec(): %v", err) - } - txPool.LocalConclude(ctx, txConn) -} - -// TestTxPoolBeginWithPoolConnectionError_Errno2006_Permanent tests the case -// where a transient errno 2006 is followed by permanent connection rejections. -// For example, if all open connections are killed and new connections are -// rejected. -func TestTxPoolBeginWithPoolConnectionError_Errno2006_Permanent(t *testing.T) { - db, txPool, err := primeTxPoolWithConnection(t) - if err != nil { - t.Fatal(err) - } + db, txPool := primeTxPoolWithConnection(t) defer db.Close() defer txPool.Close() // Close the connection on the server side. db.CloseAllConnections() - if err := db.WaitForClose(2 * time.Second); err != nil { - t.Fatal(err) - } - // Prevent new connections as well. - db.EnableConnFail() - - // This Begin will error with 2006. - // After that, vttablet will automatically try to reconnect and this fail. - // DBConn.Exec() will return the reconnect error as final error and not the - // initial connection error. - _, _, err = txPool.LocalBegin(context.Background(), &querypb.ExecuteOptions{}) - if err == nil || !strings.Contains(err.Error(), "(errno 2013)") { - t.Fatalf("Begin did not return the reconnect error: %v", err) - } - sqlErr, ok := err.(*mysql.SQLError) - if !ok { - t.Fatalf("Unexpected error type: %T, want %T", err, &mysql.SQLError{}) - } - if got, want := sqlErr.Number(), mysql.CRServerLost; got != want { - t.Errorf("Unexpected error code: %d, want %d", got, want) - } -} + err := db.WaitForClose(2 * time.Second) + require.NoError(t, err) -func TestTxPoolBeginWithPoolConnectionError_Errno2013(t *testing.T) { - db, txPool, err := primeTxPoolWithConnection(t) - if err != nil { - t.Fatal(err) - } - // No db.Close() needed. We close it below. - defer txPool.Close() - - // Close the connection *after* the server received the query. - // This will provoke a MySQL client error with errno 2013. - db.EnableShouldClose() - - // 2013 is not retryable. DBConn.Exec() fails after the first attempt. - _, _, err = txPool.Begin(context.Background(), &querypb.ExecuteOptions{}) - if err == nil || !strings.Contains(err.Error(), "(errno 2013)") { - t.Fatalf("Begin must return connection error with MySQL errno 2013: %v", err) - } - if got, want := vterrors.Code(err), vtrpcpb.Code_UNKNOWN; got != want { - t.Errorf("wrong error code for Begin error: got = %v, want = %v", got, want) - } + txConn, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err, "Begin should have succeeded after the retry in DBConn.Exec()") + txConn.Release(tx.TxCommit) } // primeTxPoolWithConnection is a helper function. It reconstructs the // scenario where future transactions are going to reuse an open db connection. -func primeTxPoolWithConnection(t *testing.T) (*fakesqldb.DB, *TxPool, error) { +func primeTxPoolWithConnection(t *testing.T) (*fakesqldb.DB, *TxPool) { + t.Helper() db := fakesqldb.New(t) txPool := newTxPool() // Set the capacity to 1 to ensure that the db connection is reused. - txPool.conns.SetCapacity(1) + txPool.scp.conns.SetCapacity(1) txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) // Run a query to trigger a database connection. That connection will be // reused by subsequent transactions. db.AddQuery("begin", &sqltypes.Result{}) db.AddQuery("rollback", &sqltypes.Result{}) - ctx := context.Background() - txConn, _, err := txPool.LocalBegin(ctx, &querypb.ExecuteOptions{}) - if err != nil { - return nil, nil, err - } - txPool.LocalConclude(ctx, txConn) + txConn, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) + txConn.Release(tx.TxCommit) - return db, txPool, nil + return db, txPool } func TestTxPoolBeginWithError(t *testing.T) { - db := fakesqldb.New(t) - defer db.Close() + db, txPool, closer := setup(t) + defer closer() db.AddRejectedQuery("begin", errRejected) - txPool := newTxPool() - txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - defer txPool.Close() - ctx := context.Background() _, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) - want := "error: rejected" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("Begin: %v, want %s", err, want) - } - if got, want := vterrors.Code(err), vtrpcpb.Code_UNKNOWN; got != want { - t.Errorf("wrong error code for Begin error: got = %v, want = %v", got, want) - } + require.Error(t, err) + require.Contains(t, err.Error(), "error: rejected") + require.Equal(t, vtrpcpb.Code_UNKNOWN, vterrors.Code(err), "wrong error code for Begin error") } func TestTxPoolCancelledContextError(t *testing.T) { - db := fakesqldb.New(t) - defer db.Close() - db.AddRejectedQuery("begin", errRejected) - txPool := newTxPool() - txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - defer txPool.Close() - ctx, cancel := context.WithCancel(context.Background()) + // given + db, txPool, closer := setup(t) + defer closer() + ctx, cancel := context.WithCancel(ctx) cancel() + + // when _, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) - want := "transaction pool aborting request due to already expired context" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("Unexpected error: %v, want %s", err, want) - } - if got, want := vterrors.Code(err), vtrpcpb.Code_RESOURCE_EXHAUSTED; got != want { - t.Errorf("wrong error code error: got = %v, want = %v", got, want) - } + + // then + require.Error(t, err) + require.Contains(t, err.Error(), "transaction pool aborting request due to already expired context") + require.Equal(t, vtrpcpb.Code_RESOURCE_EXHAUSTED, vterrors.Code(err)) + require.Empty(t, db.QueryLog()) +} + +func TestTxPoolWaitTimeoutError(t *testing.T) { + env := newEnv("TabletServerTest") + env.Config().TxPool.Size = 1 + env.Config().TxPool.MaxWaiters = 0 + env.Config().TxPool.TimeoutSeconds = 1 + // given + db, txPool, closer := setupWithEnv(t, env) + defer closer() + + // lock the only connection in the pool. + conn, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) + defer conn.Unlock() + + // try locking one more connection. + _, _, err = txPool.Begin(ctx, &querypb.ExecuteOptions{}) + + // then + require.Error(t, err) + require.Contains(t, err.Error(), "transaction pool connection limit exceeded") + require.Equal(t, vtrpcpb.Code_RESOURCE_EXHAUSTED, vterrors.Code(err)) + require.Equal(t, "begin", db.QueryLog()) + require.True(t, conn.TxProperties().LogToFile) } -func TestTxPoolRollbackFail(t *testing.T) { +func TestTxPoolRollbackFailIsPassedThrough(t *testing.T) { sql := "alter table test_table add test_column int" - db := fakesqldb.New(t) - defer db.Close() - db.AddQuery(sql, &sqltypes.Result{}) - db.AddQuery("begin", &sqltypes.Result{}) + db, txPool, closer := setup(t) + defer closer() db.AddRejectedQuery("rollback", errRejected) - txPool := newTxPool() - txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - defer txPool.Close() - ctx := context.Background() - transactionID, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) - if err != nil { - t.Fatal(err) - } - txConn, err := txPool.Get(transactionID, "for query") - if err != nil { - t.Fatal(err) - } - txConn.RecordQuery(sql) - _, err = txConn.Exec(ctx, sql, 1, true) - txConn.Recycle() - if err != nil { - t.Fatalf("got error: %v", err) - } - err = txPool.Rollback(ctx, transactionID) - want := "error: rejected" - if err == nil || !strings.Contains(err.Error(), want) { - t.Errorf("Begin: %v, want %s", err, want) - } -} + conn1, _, err := txPool.Begin(ctx, &querypb.ExecuteOptions{}) + require.NoError(t, err) -func TestTxPoolGetConnNonExistentTransaction(t *testing.T) { - db := fakesqldb.New(t) - defer db.Close() - txPool := newTxPool() - txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - defer txPool.Close() - _, err := txPool.Get(12345, "for query") - want := "transaction 12345: not found" - if err == nil || err.Error() != want { - t.Errorf("Get: %v, want %s", err, want) - } + _, err = conn1.Exec(ctx, sql, 1, true) + require.NoError(t, err) + + // rollback is refused by the underlying db and the error is passed on + err = txPool.Rollback(ctx, conn1) + require.Error(t, err) + require.Contains(t, err.Error(), "error: rejected") + + conn1.Unlock() } func TestTxPoolGetConnRecentlyRemovedTransaction(t *testing.T) { - db := fakesqldb.New(t) + db, txPool, _ := setup(t) defer db.Close() - ctx := context.Background() - db.AddQuery("begin", &sqltypes.Result{}) - db.AddQuery("commit", &sqltypes.Result{}) - db.AddQuery("rollback", &sqltypes.Result{}) - txPool := newTxPool() - txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - id, _, _ := txPool.Begin(ctx, &querypb.ExecuteOptions{}) + conn1, _, _ := txPool.Begin(ctx, &querypb.ExecuteOptions{}) + id := conn1.ID() + conn1.Unlock() txPool.Close() assertErrorMatch := func(id int64, reason string) { - conn, err := txPool.Get(id, "for query") - if err == nil { - conn.Recycle() - t.Fatalf("expected error, got nil") + conn, err := txPool.GetAndLock(id, "for query") + if err == nil { // + conn.Releasef("fail") + t.Errorf("expected to get an error") + return } + want := fmt.Sprintf("transaction %v: ended at .* \\(%v\\)", id, reason) - if m, _ := regexp.MatchString(want, err.Error()); !m { - t.Errorf("Get: %v, want match %s", err, want) - } + require.Regexp(t, want, err.Error()) } assertErrorMatch(id, "pool closed") @@ -563,149 +305,89 @@ func TestTxPoolGetConnRecentlyRemovedTransaction(t *testing.T) { txPool = newTxPool() txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - id, _, _ = txPool.Begin(ctx, &querypb.ExecuteOptions{}) - if _, err := txPool.Commit(ctx, id); err != nil { - t.Fatalf("got error: %v", err) - } - - assertErrorMatch(id, "transaction committed") + conn1, _, _ = txPool.Begin(ctx, &querypb.ExecuteOptions{}) + id = conn1.ID() + _, err := txPool.Commit(ctx, conn1) + require.NoError(t, err) - id, _, _ = txPool.Begin(ctx, &querypb.ExecuteOptions{}) - if err := txPool.Rollback(ctx, id); err != nil { - t.Fatalf("got error: %v", err) - } + conn1.Releasef("transaction committed") - assertErrorMatch(id, "transaction rolled back") + assertErrorMatch(id, "transaction committed") - txPool.Close() txPool = newTxPool() txPool.SetTimeout(1 * time.Millisecond) txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) defer txPool.Close() - id, _, _ = txPool.Begin(ctx, &querypb.ExecuteOptions{}) + conn1, _, _ = txPool.Begin(ctx, &querypb.ExecuteOptions{}) + conn1.Unlock() + id = conn1.ID() time.Sleep(20 * time.Millisecond) assertErrorMatch(id, "exceeded timeout: 1ms") +} - txPool.SetTimeout(1 * time.Hour) - id, _, _ = txPool.Begin(ctx, &querypb.ExecuteOptions{}) - txc, err := txPool.Get(id, "for close") - if err != nil { - t.Fatalf("got error: %v", err) - } - - txc.Close() - txc.Recycle() +func TestTxPoolCloseKillsStrayTransactions(t *testing.T) { + _, txPool, closer := setup(t) + defer closer() - assertErrorMatch(id, "closed") -} + startingStray := txPool.env.Stats().InternalErrors.Counts()["StrayTransactions"] -func TestTxPoolExecFailDueToConnFail_Errno2006(t *testing.T) { - db := fakesqldb.New(t) - defer db.Close() - db.AddQuery("begin", &sqltypes.Result{}) + // Start stray transaction. + conn, _, err := txPool.Begin(context.Background(), &querypb.ExecuteOptions{}) + require.NoError(t, err) + conn.Unlock() - txPool := newTxPool() - txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - defer txPool.Close() - ctx := context.Background() + // Close kills stray transaction. + txPool.Close() + require.Equal(t, int64(1), txPool.env.Stats().InternalErrors.Counts()["StrayTransactions"]-startingStray) + require.Equal(t, 0, txPool.scp.Capacity()) +} - // Start the transaction. - txConn, _, err := txPool.LocalBegin(ctx, &querypb.ExecuteOptions{}) - if err != nil { - t.Fatal(err) - } +func newTxPool() *TxPool { + return newTxPoolWithEnv(newEnv("TabletServerTest")) +} - // Close the connection on the server side. Future queries will fail. - db.CloseAllConnections() - if err := db.WaitForClose(2 * time.Second); err != nil { - t.Fatal(err) - } +func newTxPoolWithEnv(env tabletenv.Env) *TxPool { + limiter := &txlimiter.TxAllowAll{} + return NewTxPool(env, limiter) +} - // Query is going to fail with connection error because the connection was closed. - sql := "alter table test_table add test_column int" - _, err = txConn.Exec(ctx, sql, 1, true) - txConn.Recycle() - if err == nil || !strings.Contains(err.Error(), "(errno 2006)") { - t.Fatalf("Exec must return connection error with MySQL errno 2006: %v", err) - } - sqlErr, ok := err.(*mysql.SQLError) - if !ok { - t.Fatalf("Unexpected error type: %T, want %T", err, &mysql.SQLError{}) - } - if num := sqlErr.Number(); num != mysql.CRServerGone { - t.Errorf("Unexpected error code: %d, want %d", num, mysql.CRServerGone) - } +func newEnv(exporterName string) tabletenv.Env { + config := tabletenv.NewDefaultConfig() + config.TxPool.Size = 300 + config.Oltp.TxTimeoutSeconds = 30 + config.TxPool.TimeoutSeconds = 40 + config.TxPool.MaxWaiters = 500000 + config.OltpReadPool.IdleTimeoutSeconds = 30 + config.OlapReadPool.IdleTimeoutSeconds = 30 + config.TxPool.IdleTimeoutSeconds = 30 + env := tabletenv.NewEnv(config, exporterName) + return env } -func TestTxPoolExecFailDueToConnFail_Errno2013(t *testing.T) { +func setup(t *testing.T) (*fakesqldb.DB, *TxPool, func()) { db := fakesqldb.New(t) - // No db.Close() needed. We close it below. - db.AddQuery("begin", &sqltypes.Result{}) + db.AddQueryPattern(".*", &sqltypes.Result{}) txPool := newTxPool() txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - defer txPool.Close() - ctx := context.Background() - // Start the transaction. - txConn, _, err := txPool.LocalBegin(ctx, &querypb.ExecuteOptions{}) - if err != nil { - t.Fatal(err) - } - - // Close the connection *after* the server received the query. - // This will provoke a MySQL client error with errno 2013. - db.EnableShouldClose() - - // Query is going to fail with connection error because the connection was closed. - sql := "alter table test_table add test_column int" - db.AddQuery(sql, &sqltypes.Result{}) - _, err = txConn.Exec(ctx, sql, 1, true) - txConn.Recycle() - if err == nil || !strings.Contains(err.Error(), "(errno 2013)") { - t.Fatalf("Exec must return connection error with MySQL errno 2013: %v", err) - } - if got, want := vterrors.Code(err), vtrpcpb.Code_UNKNOWN; got != want { - t.Errorf("wrong error code for Exec error: got = %v, want = %v", got, want) + return db, txPool, func() { + txPool.Close() + db.Close() } } -func TestTxPoolCloseKillsStrayTransactions(t *testing.T) { +func setupWithEnv(t *testing.T, env tabletenv.Env) (*fakesqldb.DB, *TxPool, func()) { db := fakesqldb.New(t) - defer db.Close() - db.AddQuery("begin", &sqltypes.Result{}) + db.AddQueryPattern(".*", &sqltypes.Result{}) - txPool := newTxPool() + txPool := newTxPoolWithEnv(env) txPool.Open(db.ConnParams(), db.ConnParams(), db.ConnParams()) - startingStray := txPool.env.Stats().InternalErrors.Counts()["StrayTransactions"] - - // Start stray transaction. - _, _, err := txPool.Begin(context.Background(), &querypb.ExecuteOptions{}) - if err != nil { - t.Fatal(err) - } - // Close kills stray transaction. - txPool.Close() - if got, want := txPool.env.Stats().InternalErrors.Counts()["StrayTransactions"]-startingStray, int64(1); got != want { - t.Fatalf("internal error count for stray transactions not increased: got = %v, want = %v", got, want) - } - if got, want := txPool.conns.Capacity(), int64(0); got != want { - t.Fatalf("resource pool was not closed. capacity: got = %v, want = %v", got, want) + return db, txPool, func() { + txPool.Close() + db.Close() } } - -func newTxPool() *TxPool { - config := tabletenv.NewDefaultConfig() - config.TxPool.Size = 300 - config.Oltp.TxTimeoutSeconds = 30 - config.TxPool.TimeoutSeconds = 40 - config.TxPool.MaxWaiters = 500000 - config.OltpReadPool.IdleTimeoutSeconds = 30 - config.OlapReadPool.IdleTimeoutSeconds = 30 - config.TxPool.IdleTimeoutSeconds = 30 - limiter := &txlimiter.TxAllowAll{} - return NewTxPool(tabletenv.NewEnv(config, "TabletServerTest"), limiter) -} diff --git a/go/vt/vttablet/tabletserver/tx_prep_pool.go b/go/vt/vttablet/tabletserver/tx_prep_pool.go index 0ec7d51c3c0..1be341eef18 100644 --- a/go/vt/vttablet/tabletserver/tx_prep_pool.go +++ b/go/vt/vttablet/tabletserver/tx_prep_pool.go @@ -20,6 +20,8 @@ import ( "errors" "fmt" "sync" + + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" ) var ( @@ -32,7 +34,7 @@ var ( // is done by TxPool. type TxPreparedPool struct { mu sync.Mutex - conns map[string]*TxConnection + conns map[string]tx.IStatefulConnection reserved map[string]error capacity int } @@ -44,7 +46,7 @@ func NewTxPreparedPool(capacity int) *TxPreparedPool { capacity = 0 } return &TxPreparedPool{ - conns: make(map[string]*TxConnection, capacity), + conns: make(map[string]tx.IStatefulConnection, capacity), reserved: make(map[string]error), capacity: capacity, } @@ -52,7 +54,7 @@ func NewTxPreparedPool(capacity int) *TxPreparedPool { // Put adds the connection to the pool. It returns an error // if the pool is full or on duplicate key. -func (pp *TxPreparedPool) Put(c *TxConnection, dtid string) error { +func (pp *TxPreparedPool) Put(c tx.IStatefulConnection, dtid string) error { pp.mu.Lock() defer pp.mu.Unlock() if _, ok := pp.reserved[dtid]; ok { @@ -73,7 +75,7 @@ func (pp *TxPreparedPool) Put(c *TxConnection, dtid string) error { // is in the reserved list, it means that an operator is trying // to resolve a previously failed commit. So, it removes the entry // and returns nil. -func (pp *TxPreparedPool) FetchForRollback(dtid string) *TxConnection { +func (pp *TxPreparedPool) FetchForRollback(dtid string) tx.IStatefulConnection { pp.mu.Lock() defer pp.mu.Unlock() if _, ok := pp.reserved[dtid]; ok { @@ -92,7 +94,7 @@ func (pp *TxPreparedPool) FetchForRollback(dtid string) *TxConnection { // reserved list by calling Forget. If the commit failed, SetFailed // must be called. This will inform future retries that the previous // commit failed. -func (pp *TxPreparedPool) FetchForCommit(dtid string) (*TxConnection, error) { +func (pp *TxPreparedPool) FetchForCommit(dtid string) (tx.IStatefulConnection, error) { pp.mu.Lock() defer pp.mu.Unlock() if err, ok := pp.reserved[dtid]; ok { @@ -123,14 +125,14 @@ func (pp *TxPreparedPool) Forget(dtid string) { // FetchAll removes all connections and returns them as a list. // It also forgets all reserved dtids. -func (pp *TxPreparedPool) FetchAll() []*TxConnection { +func (pp *TxPreparedPool) FetchAll() []tx.IStatefulConnection { pp.mu.Lock() defer pp.mu.Unlock() - conns := make([]*TxConnection, 0, len(pp.conns)) + conns := make([]tx.IStatefulConnection, 0, len(pp.conns)) for _, c := range pp.conns { conns = append(conns, c) } - pp.conns = make(map[string]*TxConnection, pp.capacity) + pp.conns = make(map[string]tx.IStatefulConnection, pp.capacity) pp.reserved = make(map[string]error) return conns } diff --git a/go/vt/vttablet/tabletserver/tx_prep_pool_test.go b/go/vt/vttablet/tabletserver/tx_prep_pool_test.go index 7d6724643b7..a1cf50edb56 100644 --- a/go/vt/vttablet/tabletserver/tx_prep_pool_test.go +++ b/go/vt/vttablet/tabletserver/tx_prep_pool_test.go @@ -61,7 +61,7 @@ func TestPrepPut(t *testing.T) { func TestPrepFetchForRollback(t *testing.T) { pp := NewTxPreparedPool(2) - conn := &TxConnection{} + conn := &StatefulConnection{} pp.Put(conn, "aa") got := pp.FetchForRollback("bb") if got != nil { @@ -79,7 +79,7 @@ func TestPrepFetchForRollback(t *testing.T) { func TestPrepFetchForCommit(t *testing.T) { pp := NewTxPreparedPool(2) - conn := &TxConnection{} + conn := &StatefulConnection{} got, err := pp.FetchForCommit("aa") require.NoError(t, err) if got != nil { @@ -118,8 +118,8 @@ func TestPrepFetchForCommit(t *testing.T) { func TestPrepFetchAll(t *testing.T) { pp := NewTxPreparedPool(2) - conn1 := &TxConnection{} - conn2 := &TxConnection{} + conn1 := &StatefulConnection{} + conn2 := &StatefulConnection{} pp.Put(conn1, "aa") pp.Put(conn2, "bb") got := pp.FetchAll() diff --git a/go/vt/vttablet/tabletserver/txlogz.go b/go/vt/vttablet/tabletserver/txlogz.go index f393941add4..bd9f3076ff6 100644 --- a/go/vt/vttablet/tabletserver/txlogz.go +++ b/go/vt/vttablet/tabletserver/txlogz.go @@ -111,16 +111,16 @@ func txlogzHandler(w http.ResponseWriter, req *http.Request) { for i := 0; i < limit; i++ { select { case out := <-ch: - txc, ok := out.(*TxConnection) + txc, ok := out.(*StatefulConnection) if !ok { - err := fmt.Errorf("unexpected value in %s: %#v (expecting value of type %T)", tabletenv.TxLogger.Name(), out, &TxConnection{}) + err := fmt.Errorf("unexpected value in %s: %#v (expecting value of type %T)", tabletenv.TxLogger.Name(), out, &StatefulConnection{}) io.WriteString(w, ``) io.WriteString(w, err.Error()) io.WriteString(w, "") log.Error(err) continue } - duration := txc.EndTime.Sub(txc.StartTime).Seconds() + duration := txc.txProps.EndTime.Sub(txc.txProps.StartTime).Seconds() var level string if duration < 0.1 { level = "low" @@ -130,7 +130,7 @@ func txlogzHandler(w http.ResponseWriter, req *http.Request) { level = "high" } tmplData := struct { - *TxConnection + *StatefulConnection Duration float64 ColorLevel string }{txc, duration, level} diff --git a/go/vt/vttablet/tabletserver/txlogz_test.go b/go/vt/vttablet/tabletserver/txlogz_test.go index 2f3564c9dd8..bde1325254d 100644 --- a/go/vt/vttablet/tabletserver/txlogz_test.go +++ b/go/vt/vttablet/tabletserver/txlogz_test.go @@ -23,9 +23,11 @@ import ( "testing" "time" - "vitess.io/vitess/go/streamlog" - "vitess.io/vitess/go/sync2" + "vitess.io/vitess/go/vt/vttablet/tabletserver/tx" + "vitess.io/vitess/go/vt/callerid" + + "vitess.io/vitess/go/streamlog" "vitess.io/vitess/go/vt/vttablet/tabletserver/tabletenv" ) @@ -51,26 +53,27 @@ func testHandler(req *http.Request, t *testing.T) { if !strings.Contains(response.Body.String(), "error") { t.Fatalf("should show an error page since transaction log format is invalid.") } - txConn := &TxConnection{ - TransactionID: 123456, - StartTime: time.Now(), - Queries: []string{"select * from test"}, - Conclusion: "unknown", - LogToFile: sync2.AtomicInt32{}, - EffectiveCallerID: callerid.NewEffectiveCallerID("effective-caller", "component", "subcomponent"), - ImmediateCallerID: callerid.NewImmediateCallerID("immediate-caller"), + txConn := &StatefulConnection{ + ConnID: 123456, + txProps: &tx.Properties{ + EffectiveCaller: callerid.NewEffectiveCallerID("effective-caller", "component", "subcomponent"), + ImmediateCaller: callerid.NewImmediateCallerID("immediate-caller"), + StartTime: time.Now(), + Conclusion: "unknown", + Queries: []string{"select * from test"}, + }, } - txConn.EndTime = txConn.StartTime + txConn.txProps.EndTime = txConn.txProps.StartTime response = httptest.NewRecorder() tabletenv.TxLogger.Send(txConn) txlogzHandler(response, req) testNotRedacted(t, response) - txConn.EndTime = txConn.StartTime.Add(time.Duration(2) * time.Second) + txConn.txProps.EndTime = txConn.txProps.StartTime.Add(time.Duration(2) * time.Second) response = httptest.NewRecorder() tabletenv.TxLogger.Send(txConn) txlogzHandler(response, req) testNotRedacted(t, response) - txConn.EndTime = txConn.StartTime.Add(time.Duration(500) * time.Millisecond) + txConn.txProps.EndTime = txConn.txProps.StartTime.Add(time.Duration(500) * time.Millisecond) response = httptest.NewRecorder() tabletenv.TxLogger.Send(txConn) txlogzHandler(response, req)