Merge #155884

155884: rowenc, flowinfra: enable a couple test for secondary tenants r=yuzefovich a=yuzefovich

**rowenc: adjust TestInitIndexFetchSpec to work with test tenants**

With test tenants we have extra 2 bytes (the tenant prefix) included
into the "key prefix length" of the spec, so we need to make an
adjustment to the test. Also add a couple of log scopes in this package.

Fixes: #109390

**flowinfra: enable TestDistSQLReadsFillGatewayID for secondary tenants**

This required only a minor adjustment to the request filter.

Fixes: #109392.
Epic: CRDB-48945

Co-authored-by: Yahor Yuzefovich <[email protected]>

Author: craig[bot]

pkg/sql/flowinfra/cluster_test.go

@@ -506,6 +506,10 @@ func TestLimitedBufferingDeadlock(t *testing.T) {
 // Test that DistSQL reads fill the BatchRequest.Header.GatewayNodeID field with
 // the ID of the gateway (as opposed to the ID of the node that created the
 // batch). Important to lease follow-the-workload transfers.
+//
+// Note that in single tenant and shared-process modes, GatewayNodeID field is
+// set on the kvclient side (i.e. in the SQL pod) whereas in external-process
+// mode it's set on the kvserver side (i.e. in the storage pod).
 func TestDistSQLReadsFillGatewayID(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
@@ -522,19 +526,18 @@ func TestDistSQLReadsFillGatewayID(t *testing.T) {
 		base.TestClusterArgs{
 			ReplicationMode: base.ReplicationManual,
 			ServerArgs: base.TestServerArgs{
-				UseDatabase:       "test",
-				DefaultTestTenant: base.TestIsForStuffThatShouldWorkWithSecondaryTenantsButDoesntYet(109392),
+				UseDatabase: "test",
 				Knobs: base.TestingKnobs{Store: &kvserver.StoreTestingKnobs{
 					EvalKnobs: kvserverbase.BatchEvalTestingKnobs{
 						TestingEvalFilter: func(filterArgs kvserverbase.FilterArgs) *kvpb.Error {
 							scanReq, ok := filterArgs.Req.(*kvpb.ScanRequest)
 							if !ok {
 								return nil
 							}
-							if !strings.HasPrefix(
-								scanReq.Key.String(),
-								fmt.Sprintf("/Table/%d/1", tableID.Load()),
-							) {
+							pkPrefix := fmt.Sprintf("/Table/%d/1", tableID.Load())
+							tenantPKPrefix := fmt.Sprintf("/Tenant/%d%s", serverutils.TestTenantID().InternalValue, pkPrefix)
+							if !strings.HasPrefix(scanReq.Key.String(), pkPrefix) &&
+								!strings.HasPrefix(scanReq.Key.String(), tenantPKPrefix) {
 								return nil
 							}
 
@@ -552,6 +555,13 @@ func TestDistSQLReadsFillGatewayID(t *testing.T) {
 		})
 	defer tc.Stopper().Stop(context.Background())
 
+	if tc.DefaultTenantDeploymentMode().IsExternal() {
+		tc.GrantTenantCapabilities(
+			context.Background(), t, serverutils.TestTenantID(),
+			map[tenantcapabilitiespb.ID]string{tenantcapabilitiespb.CanAdminRelocateRange: "true"},
+		)
+	}
+
 	db := tc.ServerConn(0)
 	sqlutils.CreateTable(t, db, "t",
 		"num INT PRIMARY KEY",

pkg/sql/rowenc/index_encoding_test.go

@@ -40,6 +40,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/util/encoding"
 	"github.com/cockroachdb/cockroach/pkg/util/json"
 	"github.com/cockroachdb/cockroach/pkg/util/leaktest"
+	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/randutil"
 	"github.com/cockroachdb/cockroach/pkg/util/trigram"
 	"github.com/cockroachdb/cockroach/pkg/util/vector"
@@ -1076,7 +1077,6 @@ func TestEncodeTrigramInvertedIndexSpans(t *testing.T) {
 
 	runTest := func(indexedValue, value string, searchType trigramSearchType,
 		expectContainsKeys, expected, expectUnique bool) {
-		t.Logf("test case: %s %s %v %t %t %t", indexedValue, value, searchType, expectContainsKeys, expected, expectUnique)
 		keys, err := EncodeInvertedIndexTableKeys(tree.NewDString(indexedValue), nil, descpb.LatestIndexDescriptorVersion)
 		require.NoError(t, err)
 
@@ -1285,6 +1285,7 @@ func TestDecodeKeyVals(t *testing.T) {
 // write leaf keys, so that's what's tested here.
 func TestVectorEncoding(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	ctx := context.Background()
 	srv, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
@@ -1376,6 +1377,7 @@ func TestVectorEncoding(t *testing.T) {
 
 func TestVectorCompositeEncoding(t *testing.T) {
 	defer leaktest.AfterTest(t)()
+	defer log.Scope(t).Close(t)
 
 	ctx := context.Background()
 	srv, sqlDB, kvDB := serverutils.StartServer(t, base.TestServerArgs{})

pkg/sql/rowenc/index_fetch_test.go

@@ -28,9 +28,7 @@ func TestInitIndexFetchSpec(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	srv, db, kvDB := serverutils.StartServer(t, base.TestServerArgs{
-		DefaultTestTenant: base.TestIsForStuffThatShouldWorkWithSecondaryTenantsButDoesntYet(109390),
-	})
+	srv, db, kvDB := serverutils.StartServer(t, base.TestServerArgs{})
 	defer srv.Stopper().Stop(context.Background())
 	codec := srv.ApplicationLayer().Codec()
 
@@ -76,6 +74,13 @@ func TestInitIndexFetchSpec(t *testing.T) {
 				if err := rowenc.InitIndexFetchSpec(&spec, codec, table, index, fetchColumnIDs); err != nil {
 					d.Fatalf(t, "%+v", err)
 				}
+				if srv.StartedDefaultTestTenant() {
+					// If we started a test tenant, then all keys will have the
+					// tenant prefix (of 2 bytes) prepended to them. In order to
+					// not create the duplicate expectation file, we'll adjust
+					// the spec here to ignore those 2 bytes.
+					spec.KeyPrefixLength -= uint32(len(codec.TenantPrefix()))
+				}
 				res, err := json.MarshalIndent(&spec, "", "  ")
 				if err != nil {
 					d.Fatalf(t, "%+v", err)