Enable failures in tools/e2e_test_race.sh and fix races (#13654)

Signed-off-by: Florent Poinsard <[email protected]>

go/vt/vttablet/endtoend/framework/client.go

@@ -19,6 +19,7 @@ package framework
 import (
 	"context"
 	"errors"
+	"sync"
 	"time"
 
 	"vitess.io/vitess/go/sqltypes"
@@ -38,6 +39,7 @@ type QueryClient struct {
 	target              *querypb.Target
 	server              *tabletserver.TabletServer
 	transactionID       int64
+	reservedIDMu        sync.Mutex
 	reservedID          int64
 	sessionStateChanges string
 }
@@ -112,6 +114,8 @@ func (client *QueryClient) Commit() error {
 func (client *QueryClient) Rollback() error {
 	defer func() { client.transactionID = 0 }()
 	rID, err := client.server.Rollback(client.ctx, client.target, client.transactionID)
+	client.reservedIDMu.Lock()
+	defer client.reservedIDMu.Unlock()
 	client.reservedID = rID
 	if err != nil {
 		return err
@@ -291,6 +295,8 @@ func (client *QueryClient) MessageAck(name string, ids []string) (int64, error)
 
 // ReserveExecute performs a ReserveExecute.
 func (client *QueryClient) ReserveExecute(query string, preQueries []string, bindvars map[string]*querypb.BindVariable) (*sqltypes.Result, error) {
+	client.reservedIDMu.Lock()
+	defer client.reservedIDMu.Unlock()
 	if client.reservedID != 0 {
 		return nil, errors.New("already reserved a connection")
 	}

go/vt/vttablet/endtoend/transaction_test.go

@@ -321,7 +321,7 @@ func TestShutdownGracePeriod(t *testing.T) {
 	err := client.Begin(false)
 	require.NoError(t, err)
 	go func() {
-		_, err = client.Execute("select sleep(10) from dual", nil)
+		_, err := client.Execute("select sleep(10) from dual", nil)
 		assert.Error(t, err)
 	}()
 
@@ -346,7 +346,7 @@ func TestShutdownGracePeriod(t *testing.T) {
 	err = client.Begin(false)
 	require.NoError(t, err)
 	go func() {
-		_, err = client.Execute("select sleep(11) from dual", nil)
+		_, err := client.Execute("select sleep(11) from dual", nil)
 		assert.Error(t, err)
 	}()
 
@@ -373,7 +373,7 @@ func TestShutdownGracePeriodWithStreamExecute(t *testing.T) {
 	err := client.Begin(false)
 	require.NoError(t, err)
 	go func() {
-		_, err = client.StreamExecute("select sleep(10) from dual", nil)
+		_, err := client.StreamExecute("select sleep(10) from dual", nil)
 		assert.Error(t, err)
 	}()
 
@@ -398,7 +398,7 @@ func TestShutdownGracePeriodWithStreamExecute(t *testing.T) {
 	err = client.Begin(false)
 	require.NoError(t, err)
 	go func() {
-		_, err = client.StreamExecute("select sleep(11) from dual", nil)
+		_, err := client.StreamExecute("select sleep(11) from dual", nil)
 		assert.Error(t, err)
 	}()
 
@@ -425,7 +425,7 @@ func TestShutdownGracePeriodWithReserveExecute(t *testing.T) {
 	err := client.Begin(false)
 	require.NoError(t, err)
 	go func() {
-		_, err = client.ReserveExecute("select sleep(10) from dual", nil, nil)
+		_, err := client.ReserveExecute("select sleep(10) from dual", nil, nil)
 		assert.Error(t, err)
 	}()
 
@@ -450,7 +450,7 @@ func TestShutdownGracePeriodWithReserveExecute(t *testing.T) {
 	err = client.Begin(false)
 	require.NoError(t, err)
 	go func() {
-		_, err = client.ReserveExecute("select sleep(11) from dual", nil, nil)
+		_, err := client.ReserveExecute("select sleep(11) from dual", nil, nil)
 		assert.Error(t, err)
 	}()
 

go/vt/vttablet/tabletserver/throttle/client.go

@@ -52,7 +52,8 @@ type Client struct {
 	checkType ThrottleCheckType
 	flags     CheckFlags
 
-	lastSuccessfulThrottle int64
+	lastSuccessfulThrottleMu sync.Mutex
+	lastSuccessfulThrottle   int64
 }
 
 // NewProductionClient creates a client suitable for foreground/production jobs, which have normal priority.
@@ -96,6 +97,8 @@ func (c *Client) ThrottleCheckOK(ctx context.Context, overrideAppName throttlera
 		// no throttler
 		return true
 	}
+	c.lastSuccessfulThrottleMu.Lock()
+	defer c.lastSuccessfulThrottleMu.Unlock()
 	if c.lastSuccessfulThrottle >= atomic.LoadInt64(&throttleTicks) {
 		// if last check was OK just very recently there is no need to check again
 		return true

tools/e2e_test_race.sh

@@ -38,9 +38,11 @@ packages_with_tests=$(echo "$packages_with_tests" |  grep -vE "go/test/endtoend"
 # endtoend tests should be in a directory called endtoend
 all_e2e_tests=$(echo "$packages_with_tests" | cut -d" " -f1)
 
+set -exo pipefail
+
 # Run all endtoend tests.
 echo "$all_e2e_tests" | xargs go test $VT_GO_PARALLEL -race 2>&1 | tee $temp_log_file
-if [ ${PIPESTATUS[0]} -ne 0 ]; then
+if [ ${PIPESTATUS[1]} -ne 0 ]; then
   if grep "WARNING: DATA RACE" -q $temp_log_file; then
     echo
     echo "ERROR: go test -race found a data race. See log above."