Fix setup order to avoid races (#13871)

Signed-off-by: Dirkjan Bussink <d.bussink@gmail.com>
vitessio · Aug 29, 2023 · 7450cc0 · 7450cc0
1 parent e5b35fc
commit 7450cc0
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 40 deletions.
diff --git a/go/cmd/vttestserver/vttestserver_test.go b/go/cmd/vttestserver/vttestserver_test.go
@@ -114,7 +114,10 @@ func TestPersistentMode(t *testing.T) {
 	// reboot the persistent cluster
 	cluster.TearDown()
 	cluster, err = startPersistentCluster(dir)
-	defer cluster.TearDown()
+	defer func() {
+		cluster.PersistentMode = false // Cleanup the tmpdir as we're done
+		cluster.TearDown()
+	}()
 	assert.NoError(t, err)
 
 	// rerun our sanity checks to make sure vschema is persisted correctly

diff --git a/go/test/utils/noleak.go b/go/test/utils/noleak.go
@@ -18,16 +18,10 @@ package utils
 
 import (
 	"context"
-	"os"
-	"os/exec"
-	"strconv"
-	"strings"
 	"testing"
 	"time"
 
 	"go.uber.org/goleak"
-
-	"vitess.io/vitess/go/vt/log"
 )
 
 // LeakCheckContext returns a Context that will be automatically cancelled at the end
@@ -73,9 +67,6 @@ func ensureNoLeaks() error {
 	if err := ensureNoGoroutines(); err != nil {
 		return err
 	}
-	if err := ensureNoOpenSockets(); err != nil {
-		return err
-	}
 	return nil
 }
 
@@ -103,18 +94,3 @@ func ensureNoGoroutines() error {
 	}
 	return err
 }
-
-func ensureNoOpenSockets() error {
-	cmd := exec.Command("lsof", "-a", "-p", strconv.Itoa(os.Getpid()), "-i", "-P", "-V")
-	cmd.Stderr = nil
-	lsof, err := cmd.Output()
-	if err == nil {
-		log.Errorf("found open sockets:\n%s", lsof)
-	} else {
-		if strings.Contains(string(lsof), "no Internet files located") {
-			return nil
-		}
-		log.Errorf("failed to run `lsof`: %v (%q)", err, lsof)
-	}
-	return err
-}
diff --git a/go/vt/vtgate/executor_framework_test.go b/go/vt/vtgate/executor_framework_test.go
@@ -132,8 +132,16 @@ func createExecutorEnv(t testing.TB) (executor *Executor, sbc1, sbc2, sbclookup
 	ctx, cancel = context.WithCancel(context.Background())
 	cell := "aa"
 	hc := discovery.NewFakeHealthCheck(make(chan *discovery.TabletHealth))
+
 	s := createSandbox(KsTestSharded)
 	s.VSchema = executorVSchema
+	sb := createSandbox(KsTestUnsharded)
+	sb.VSchema = unshardedVSchema
+	// Use the 'X' in the name to ensure it's not alphabetically first.
+	// Otherwise, it would become the default keyspace for the dual table.
+	bad := createSandbox("TestXBadSharding")
+	bad.VSchema = badVSchema
+
 	serv := newSandboxForCells(ctx, []string{cell})
 	serv.topoServer.CreateKeyspace(ctx, KsTestSharded, &topodatapb.Keyspace{SidecarDbName: sidecar.DefaultName})
 	// Force a new cache to use for lookups of the sidecar database identifier
@@ -152,6 +160,7 @@ func createExecutorEnv(t testing.TB) (executor *Executor, sbc1, sbc2, sbclookup
 	if !created {
 		log.Fatal("Failed to [re]create a sidecar database identifier cache!")
 	}
+
 	resolver := newTestResolver(ctx, hc, serv, cell)
 	sbc1 = hc.AddTestTablet(cell, "-20", 1, "TestExecutor", "-20", topodatapb.TabletType_PRIMARY, true, 1, nil)
 	sbc2 = hc.AddTestTablet(cell, "40-60", 1, "TestExecutor", "40-60", topodatapb.TabletType_PRIMARY, true, 1, nil)
@@ -164,16 +173,9 @@ func createExecutorEnv(t testing.TB) (executor *Executor, sbc1, sbc2, sbclookup
 	_ = hc.AddTestTablet(cell, "e0-", 1, "TestExecutor", "e0-", topodatapb.TabletType_PRIMARY, true, 1, nil)
 	// Below is needed so that SendAnyWherePlan doesn't fail
 
-	sb := createSandbox(KsTestUnsharded)
 	sbclookup = hc.AddTestTablet(cell, "0", 1, KsTestUnsharded, "0", topodatapb.TabletType_PRIMARY, true, 1, nil)
 	_ = hc.AddTestTablet(cell, "2", 3, KsTestUnsharded, "0", topodatapb.TabletType_REPLICA, true, 1, nil)
 
-	// Ues the 'X' in the name to ensure it's not alphabetically first.
-	// Otherwise, it would become the default keyspace for the dual table.
-	bad := createSandbox("TestXBadSharding")
-	bad.VSchema = badVSchema
-
-	sb.VSchema = unshardedVSchema
 	queryLogger := streamlog.New[*logstats.LogStats]("VTGate", queryLogBufferSize)
 	plans := cache.NewDefaultCacheImpl(cache.DefaultConfig)
 	executor = NewExecutor(ctx, serv, cell, resolver, false, false, testBufferSize, plans, nil, false, querypb.ExecuteOptions_Gen4, queryLogger)
@@ -194,16 +196,17 @@ func createCustomExecutor(t testing.TB, vschema string) (executor *Executor, sbc
 	ctx, cancel = context.WithCancel(context.Background())
 	cell := "aa"
 	hc := discovery.NewFakeHealthCheck(nil)
+
 	s := createSandbox(KsTestSharded)
 	s.VSchema = vschema
+	sb := createSandbox(KsTestUnsharded)
+	sb.VSchema = unshardedVSchema
+
 	serv := newSandboxForCells(ctx, []string{cell})
 	resolver := newTestResolver(ctx, hc, serv, cell)
-	sbc1 = hc.AddTestTablet(cell, "-20", 1, "TestExecutor", "-20", topodatapb.TabletType_PRIMARY, true, 1, nil)
-	sbc2 = hc.AddTestTablet(cell, "40-60", 1, "TestExecutor", "40-60", topodatapb.TabletType_PRIMARY, true, 1, nil)
-
-	sb := createSandbox(KsTestUnsharded)
+	sbc1 = hc.AddTestTablet(cell, "-20", 1, KsTestSharded, "-20", topodatapb.TabletType_PRIMARY, true, 1, nil)
+	sbc2 = hc.AddTestTablet(cell, "40-60", 1, KsTestSharded, "40-60", topodatapb.TabletType_PRIMARY, true, 1, nil)
 	sbclookup = hc.AddTestTablet(cell, "0", 1, KsTestUnsharded, "0", topodatapb.TabletType_PRIMARY, true, 1, nil)
-	sb.VSchema = unshardedVSchema
 
 	queryLogger := streamlog.New[*logstats.LogStats]("VTGate", queryLogBufferSize)
 	plans := cache.NewDefaultCacheImpl(cache.DefaultConfig)
@@ -223,8 +226,12 @@ func createCustomExecutorSetValues(t testing.TB, vschema string, values []*sqlty
 	ctx, cancel = context.WithCancel(context.Background())
 	cell := "aa"
 	hc := discovery.NewFakeHealthCheck(nil)
+
 	s := createSandbox(KsTestSharded)
 	s.VSchema = vschema
+	sb := createSandbox(KsTestUnsharded)
+	sb.VSchema = unshardedVSchema
+
 	serv := newSandboxForCells(ctx, []string{cell})
 	resolver := newTestResolver(ctx, hc, serv, cell)
 	shards := []string{"-20", "20-40", "40-60", "60-80", "80-a0", "a0-c0", "c0-e0", "e0-"}
@@ -236,10 +243,7 @@ func createCustomExecutorSetValues(t testing.TB, vschema string, values []*sqlty
 		}
 		sbcs = append(sbcs, sbc)
 	}
-
-	sb := createSandbox(KsTestUnsharded)
 	sbclookup = hc.AddTestTablet(cell, "0", 1, KsTestUnsharded, "0", topodatapb.TabletType_PRIMARY, true, 1, nil)
-	sb.VSchema = unshardedVSchema
 
 	queryLogger := streamlog.New[*logstats.LogStats]("VTGate", queryLogBufferSize)
 	plans := cache.NewDefaultCacheImpl(cache.DefaultConfig)

diff --git a/go/vt/vtgate/schema/tracker.go b/go/vt/vtgate/schema/tracker.go
@@ -151,6 +151,10 @@ func (t *Tracker) Start() {
 		for {
 			select {
 			case th := <-t.ch:
+				if th == nil {
+					// channel closed
+					return
+				}
 				ksUpdater := t.getKeyspaceUpdateController(th)
 				ksUpdater.add(th)
 			case <-ctx.Done():