Merge #151773 #152509 #152518

151773: sql: add ltree value encoding tests r=paulniziolek a=paulniziolek

#### sql: add ltree value encoding tests

This was forgotten when we first implemented the ltree datum.

Epic: None
Fixes: None

Release note: None

152509: crosscluster/physical: always destroy reader tenant on cutover r=jeffswenson a=msbutler

Previously, a reader tenant would only get automatically destroyed if a user specified an explicit cutover time. This UX is confusing. Now, the reader tenant will always get destroyed on cutover.

Fixes #149684

Release note (ops change): the PCR reader tenant is always destroyed on cutover

152518: sqlstatsccl: bump test pool size r=kyle-a-wong a=rickystewart

This test has hit an OOM.

Epic: none
Release note: None

Co-authored-by: Paul Niziolek <[email protected]>
Co-authored-by: Michael Butler <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>

Author: 4 people

pkg/ccl/testccl/sqlstatsccl/BUILD.bazel

@@ -2,12 +2,12 @@ load("@io_bazel_rules_go//go:def.bzl", "go_test")
 
 go_test(
     name = "sqlstatsccl_test",
-    # Explicitly indicate timeout as some of the tests (ie TestSQLStatsRegions) might take up to 3 min to run.
-    timeout = "moderate",
+    size = "medium",
     srcs = [
         "main_test.go",
         "sql_stats_test.go",
     ],
+    exec_properties = {"test.Pool": "large"},
     deps = [
         "//pkg/base",
         "//pkg/ccl",

pkg/crosscluster/physical/standby_read_ts_poller_job_test.go

@@ -10,7 +10,6 @@ import (
 	"fmt"
 	"net/url"
 	"testing"
-	"time"
 
 	apd "github.com/cockroachdb/apd/v3"
 	"github.com/cockroachdb/cockroach/pkg/crosscluster/replicationtestutils"
@@ -217,16 +216,9 @@ INSERT INTO a VALUES (1);
 `)
 
 		waitForPollerJobToStartDest(t, c, ingestionJobID)
-		if cutoverToLatest {
-			observeValueInReaderTenant(t, c.ReaderTenantSQL)
-			c.Cutover(ctx, producerJobID, ingestionJobID, time.Time{}, false)
-			jobutils.WaitForJobToSucceed(t, c.DestSysSQL, jobspb.JobID(ingestionJobID))
-			observeValueInReaderTenant(t, c.ReaderTenantSQL)
-		} else {
-			c.Cutover(ctx, producerJobID, ingestionJobID, c.SrcCluster.Server(0).Clock().Now().GoTime(), false)
-			waitToRemoveTenant(t, c.DestSysSQL, readerTenantName)
-			jobutils.WaitForJobToSucceed(t, c.DestSysSQL, jobspb.JobID(ingestionJobID))
-		}
+		c.Cutover(ctx, producerJobID, ingestionJobID, c.SrcCluster.Server(0).Clock().Now().GoTime(), false)
+		waitToRemoveTenant(t, c.DestSysSQL, readerTenantName)
+		jobutils.WaitForJobToSucceed(t, c.DestSysSQL, jobspb.JobID(ingestionJobID))
 	})
 }
 

pkg/crosscluster/physical/stream_ingestion_job.go

@@ -456,12 +456,11 @@ func maybeRevertToCutoverTimestamp(
 	// existed in the record at the point of the update rather the
 	// value that may be in the job record before the update.
 	var (
-		shouldRevertToCutover   bool
-		cutoverTimestamp        hlc.Timestamp
-		originalSpanToRevert    roachpb.Span
-		remainingSpansToRevert  roachpb.Spans
-		replicatedTimeAtCutover hlc.Timestamp
-		readerTenantID          roachpb.TenantID
+		shouldRevertToCutover  bool
+		cutoverTimestamp       hlc.Timestamp
+		originalSpanToRevert   roachpb.Span
+		remainingSpansToRevert roachpb.Spans
+		readerTenantID         roachpb.TenantID
 	)
 	if err := ingestionJob.NoTxn().Update(ctx,
 		func(txn isql.Txn, md jobs.JobMetadata, ju *jobs.JobUpdater) error {
@@ -478,7 +477,6 @@ func maybeRevertToCutoverTimestamp(
 			}
 
 			cutoverTimestamp = streamIngestionProgress.CutoverTime
-			replicatedTimeAtCutover = streamIngestionProgress.ReplicatedTimeAtCutover
 			readerTenantID = streamIngestionDetails.ReadTenantID
 			originalSpanToRevert = streamIngestionDetails.Span
 			remainingSpansToRevert = streamIngestionProgress.RemainingCutoverSpans
@@ -500,9 +498,7 @@ func maybeRevertToCutoverTimestamp(
 	if !shouldRevertToCutover {
 		return cutoverTimestamp, false, nil
 	}
-	// Identical cutoverTimestamp and replicatedTimeAtCutover implies that
-	// CUTOVER TO LATEST command was run. Destroy reader tenant if not CUTOVER TO LATEST.
-	if !cutoverTimestamp.Equal(replicatedTimeAtCutover) && readerTenantID.IsSet() {
+	if readerTenantID.IsSet() {
 		if err := stopTenant(ctx, p.ExecCfg(), readerTenantID); err != nil {
 			return cutoverTimestamp, false, errors.Wrapf(err, "failed to stop reader tenant")
 		}

pkg/sql/randgen/datum.go

@@ -316,25 +316,7 @@ func RandDatumWithNullChance(
 	case types.OidFamily:
 		return tree.NewDOidWithType(oid.Oid(rng.Uint32()), typ)
 	case types.LTreeFamily:
-		length := rng.Intn(10)
-		if length == 1 && rng.Intn(4) == 0 {
-			return tree.NewDLTree(ltree.Empty)
-		}
-		labels := make([]string, length)
-		for i := range labels {
-			// Labels cannot be empty.
-			labelLen := rng.Intn(9) + 1
-			p := make([]byte, labelLen)
-			for j := range p {
-				p[j] = charSet[rng.Intn(len(charSet))]
-			}
-			labels[i] = string(p)
-		}
-		l, err := tree.ParseDLTree(strings.Join(labels, ltree.PathSeparator))
-		if err != nil {
-			return nil
-		}
-		return l
+		return tree.NewDLTree(ltree.RandLTree(rng))
 	case types.UnknownFamily:
 		return tree.DNull
 	case types.ArrayFamily:

pkg/util/encoding/encoding.go

@@ -3644,6 +3644,13 @@ func PrettyPrintValueEncoded(b []byte) ([]byte, string, error) {
 		var s string
 		b, s, err = PrettyPrintTupleValueEncoded(b)
 		return b, s, err
+	case LTree:
+		var l ltree.T
+		b, l, err = DecodeLTreeValue(b)
+		if err != nil {
+			return b, "", err
+		}
+		return b, l.String(), nil
 	default:
 		return b, "", errors.Errorf("unknown type %s", typ)
 	}

pkg/util/encoding/encoding_test.go

@@ -1725,6 +1725,10 @@ func (rd randData) ipAddr() ipaddr.IPAddr {
 	return ipaddr.RandIPAddr(rd.Rand)
 }
 
+func (rd randData) ltree() ltree.T {
+	return ltree.RandLTree(rd.Rand)
+}
+
 func BenchmarkEncodeUint32(b *testing.B) {
 	rng, _ := randutil.NewTestRand()
 
@@ -2251,6 +2255,25 @@ func TestValueEncodeDecodeDuration(t *testing.T) {
 	}
 }
 
+func TestValueEncodeDecodeLTree(t *testing.T) {
+	rng, seed := randutil.NewTestRand()
+	rd := randData{rng}
+	tests := make([]ltree.T, 1000)
+	for i := range tests {
+		tests[i] = rd.ltree()
+	}
+	for _, test := range tests {
+		buf := EncodeLTreeValue(nil, NoColumnID, test)
+		_, x, err := DecodeLTreeValue(buf)
+		if err != nil {
+			t.Fatal(err)
+		}
+		if x.Compare(test) != 0 {
+			t.Errorf("seed %d: expected %s got %s", seed, test.String(), x.String())
+		}
+	}
+}
+
 func BenchmarkEncodeNonsortingVarint(b *testing.B) {
 	bytes := make([]byte, 0, b.N*MaxNonsortingVarintLen)
 	rng, _ := randutil.NewTestRand()
@@ -2456,6 +2479,9 @@ func randValueEncode(rd randData, buf []byte, colID uint32, typ Type) ([]byte, i
 	case IPAddr:
 		x := rd.ipAddr()
 		return EncodeIPAddrValue(buf, colID, x), x, true
+	case LTree:
+		x := rd.ltree()
+		return EncodeLTreeValue(buf, colID, x), x, true
 	default:
 		return buf, nil, false
 	}
@@ -2611,6 +2637,8 @@ func TestValueEncodingRand(t *testing.T) {
 			buf, decoded, err = DecodeBitArrayValue(buf)
 		case IPAddr:
 			buf, decoded, err = DecodeIPAddrValue(buf)
+		case LTree:
+			buf, decoded, err = DecodeLTreeValue(buf)
 		default:
 			err = errors.Errorf("unknown type %s", typ)
 		}
@@ -2641,6 +2669,12 @@ func TestValueEncodingRand(t *testing.T) {
 			if bitarray.Compare(d, val) != 0 {
 				t.Fatalf("seed %d: %s got %v expected %v", seed, typ, decoded, value)
 			}
+		case LTree:
+			d := decoded.(ltree.T)
+			val := value.(ltree.T)
+			if d.Compare(val) != 0 {
+				t.Fatalf("seed %d: %s got %v expected %v", seed, typ, decoded, value)
+			}
 		default:
 			if decoded != value {
 				t.Fatalf("seed %d: %s got %v expected %v", seed, typ, decoded, value)

pkg/util/ltree/ltree.go

@@ -7,6 +7,7 @@ package ltree
 
 import (
 	"bytes"
+	"math/rand"
 	"strings"
 
 	"github.com/cockroachdb/cockroach/pkg/sql/pgwire/pgcode"
@@ -136,6 +137,25 @@ func (lt T) Copy() T {
 	return T{path: copiedLabels}
 }
 
+func RandLTree(rng *rand.Rand) T {
+	charSet := "-0123456789ABCDEFGHIJKLMNOPQRSTUVWXYZ_abcdefghijklmnopqrstuvwxyz"
+	length := rng.Intn(10)
+	if length == 0 {
+		return Empty
+	}
+	labels := make([]string, length)
+	for i := range labels {
+		// Labels cannot be empty.
+		labelLen := rng.Intn(9) + 1
+		p := make([]byte, labelLen)
+		for j := range p {
+			p[j] = charSet[rng.Intn(len(charSet))]
+		}
+		labels[i] = string(p)
+	}
+	return T{path: labels}
+}
+
 // validateLabel checks if a label is valid and returns an error if it is not,
 // otherwise, it returns nil.
 // A label is valid if it: