Merge #94010 #94345 #94458

94010: sql/importer: unskip TestCSVImportCanBeResumed r=rafiss a=stevendanna This unskips the test and adds a memory limit. The test completed 1500+ runs under stress. I believe #93782 resolved the main source of flakiness here. Fixes #91828 Release note: None 94345: storage: include range key stats in ScanStats r=erikgrinaker a=jbowens Include the new, low-level Pebble range key iterator stats (introduced in cockroachdb/pebble#1871) in roachpb.ScanStats. Informs #77580. Close #93326. Epic: None Release note: None 94458: logictest: fix flakes from mixed version testing r=ZhouXing19 a=rafiss fixes #92637 The main fix was to wait for each node to be reachable before beginning the upgrade process. This also includes a version bump that has better logging to make it easier to debug. Release note: None Co-authored-by: Steven Danna <[email protected]> Co-authored-by: Jackson Owens <[email protected]> Co-authored-by: Rafi Shamim <[email protected]>
cockroachdb · Jan 3, 2023 · 291a43b · 291a43b
4 parents be9e8fb + d522dab + e001bf9 + b4f40e4
commit 291a43b
Show file tree

Hide file tree

Showing 11 changed files with 39 additions and 26 deletions.
diff --git a/DEPS.bzl b/DEPS.bzl
@@ -1402,10 +1402,10 @@ def go_deps():
         name = "com_github_cockroachdb_cockroach_go_v2",
         build_file_proto_mode = "disable_global",
         importpath = "github.com/cockroachdb/cockroach-go/v2",
-        sha256 = "54ce3a52f7971a1f5b69a766a45688d77fd9118f815ac5839a4f2477df498b06",
-        strip_prefix = "github.com/cockroachdb/cockroach-go/[email protected].19",
+        sha256 = "c754ddca015e733515210f650389dd25f9efb3acc0a8309a6161474f5a1ac5eb",
+        strip_prefix = "github.com/cockroachdb/cockroach-go/[email protected].20",
         urls = [
-            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/cockroach-go/v2/com_github_cockroachdb_cockroach_go_v2-v2.2.19.zip",
+            "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/cockroach-go/v2/com_github_cockroachdb_cockroach_go_v2-v2.2.20.zip",
         ],
     )
     go_repository(

diff --git a/build/bazelutil/distdir_files.bzl b/build/bazelutil/distdir_files.bzl
@@ -185,7 +185,7 @@ DISTDIR_FILES = {
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/apd/v3/com_github_cockroachdb_apd_v3-v3.1.2.zip": "dde4e1e0861ab1276363eb60a6f1ac6b9f70e1a5baea1e7c7d3bd2a0b9cffad5",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/circuitbreaker/com_github_cockroachdb_circuitbreaker-v2.2.2-0.20190114160014-a614b14ccf63+incompatible.zip": "52fdb5ba6a60e9a2f1db42d5b3c4c13cc5bb3947d5ce7f1bba9b0a14de71813a",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/cmux/com_github_cockroachdb_cmux-v0.0.0-20170110192607-30d10be49292.zip": "88f6f9cf33eb535658540b46f6222f029398e590a3ff9cc873d7d561ac6debf0",
-    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/cockroach-go/v2/com_github_cockroachdb_cockroach_go_v2-v2.2.19.zip": "54ce3a52f7971a1f5b69a766a45688d77fd9118f815ac5839a4f2477df498b06",
+    "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/cockroach-go/v2/com_github_cockroachdb_cockroach_go_v2-v2.2.20.zip": "c754ddca015e733515210f650389dd25f9efb3acc0a8309a6161474f5a1ac5eb",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/crlfmt/com_github_cockroachdb_crlfmt-v0.0.0-20221214225007-b2fc5c302548.zip": "fedc01bdd6d964da0425d5eaac8efadc951e78e13f102292cc0774197f09ab63",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/datadriven/com_github_cockroachdb_datadriven-v1.0.2.zip": "1818b828715b773ea9eaf415fa3cc176c411e18f645ec85440b14abaf1f387c4",
     "https://storage.googleapis.com/cockroach-godeps/gomod/github.com/cockroachdb/errors/com_github_cockroachdb_errors-v1.9.0.zip": "ff3814544271799c80da14dadfe408efc4f66e02cbdf17b73e81614ed9f7ae43",

diff --git a/go.mod b/go.mod
@@ -104,7 +104,7 @@ require (
 	github.com/cockroachdb/apd/v3 v3.1.2
 	github.com/cockroachdb/circuitbreaker v2.2.2-0.20190114160014-a614b14ccf63+incompatible
 	github.com/cockroachdb/cmux v0.0.0-20170110192607-30d10be49292
-	github.com/cockroachdb/cockroach-go/v2 v2.2.19
+	github.com/cockroachdb/cockroach-go/v2 v2.2.20
 	github.com/cockroachdb/crlfmt v0.0.0-20221214225007-b2fc5c302548
 	github.com/cockroachdb/datadriven v1.0.2
 	github.com/cockroachdb/errors v1.9.0

diff --git a/go.sum b/go.sum
@@ -452,8 +452,8 @@ github.com/cockroachdb/circuitbreaker v2.2.2-0.20190114160014-a614b14ccf63+incom
 github.com/cockroachdb/circuitbreaker v2.2.2-0.20190114160014-a614b14ccf63+incompatible/go.mod h1:v3T8+rm/HmCL0D1BwDcGaHHAQDuFPW7EsnYs2nBRqUo=
 github.com/cockroachdb/cmux v0.0.0-20170110192607-30d10be49292 h1:dzj1/xcivGjNPwwifh/dWTczkwcuqsXXFHY1X/TZMtw=
 github.com/cockroachdb/cmux v0.0.0-20170110192607-30d10be49292/go.mod h1:qRiX68mZX1lGBkTWyp3CLcenw9I94W2dLeRvMzcn9N4=
-github.com/cockroachdb/cockroach-go/v2 v2.2.19 h1:YIHyz17jZumBeXPuoZKq/0nrITsqDoDD8/KQt3/xiyc=
-github.com/cockroachdb/cockroach-go/v2 v2.2.19/go.mod h1:mzlIDDBALQfEjv/7DU12fb2AfQ/MUYTlychcMpWp9QI=
+github.com/cockroachdb/cockroach-go/v2 v2.2.20 h1:TLSzwdTdIwgsbdApHzaxunhSMrmbGf5YY6oxtaP2kvw=
+github.com/cockroachdb/cockroach-go/v2 v2.2.20/go.mod h1:73vQi5H/H7kE8SgOt+XA6729Tubvj5hxKIEgbQQhp4c=
 github.com/cockroachdb/crlfmt v0.0.0-20221214225007-b2fc5c302548 h1:i0bnjanlWAvM50wHMT7EFyxlt5HQusznWrkwl+HBIsU=
 github.com/cockroachdb/crlfmt v0.0.0-20221214225007-b2fc5c302548/go.mod h1:qtkxNlt5i3rrdirfJE/bQeW/IeLajKexErv7jEIV+Uc=
 github.com/cockroachdb/datadriven v0.0.0-20190809214429-80d97fb3cbaa/go.mod h1:zn76sxSg3SzpJ0PPJaLDCu+Bu0Lg3sKTORVIj19EIF8=

diff --git a/pkg/roachpb/api.go b/pkg/roachpb/api.go
@@ -1781,14 +1781,18 @@ func humanizePointCount(n uint64) redact.SafeString {
 func (s *ScanStats) SafeFormat(w redact.SafePrinter, _ rune) {
 	w.Printf("scan stats: stepped %d times (%d internal); seeked %d times (%d internal); "+
 		"block-bytes: (total %s, cached %s); "+
-		"points: (count %s, key-bytes %s, value-bytes %s, tombstoned: %s)",
+		"points: (count %s, key-bytes %s, value-bytes %s, tombstoned: %s) "+
+		"ranges: (count %s), (contained-points %s, skipped-points %s)",
 		s.NumInterfaceSteps, s.NumInternalSteps, s.NumInterfaceSeeks, s.NumInternalSeeks,
 		humanizeutil.IBytes(int64(s.BlockBytes)),
 		humanizeutil.IBytes(int64(s.BlockBytesInCache)),
 		humanizePointCount(s.PointCount),
 		humanizeutil.IBytes(int64(s.KeyBytes)),
 		humanizeutil.IBytes(int64(s.ValueBytes)),
-		humanizePointCount(s.PointsCoveredByRangeTombstones))
+		humanizePointCount(s.PointsCoveredByRangeTombstones),
+		humanizePointCount(s.RangeKeyCount),
+		humanizePointCount(s.RangeKeyContainedPoints),
+		humanizePointCount(s.RangeKeySkippedPoints))
 }
 
 // String implements fmt.Stringer.

diff --git a/pkg/roachpb/api.proto b/pkg/roachpb/api.proto
@@ -3200,4 +3200,7 @@ message ScanStats {
   uint64 value_bytes = 8;
   uint64 point_count = 9;
   uint64 points_covered_by_range_tombstones = 10;
+  uint64 range_key_count = 11;
+  uint64 range_key_contained_points = 12;
+  uint64 range_key_skipped_points = 13;
 }
diff --git a/pkg/sql/importer/import_processor_test.go b/pkg/sql/importer/import_processor_test.go
@@ -42,7 +42,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/testutils/datapathutils"
 	"github.com/cockroachdb/cockroach/pkg/testutils/serverutils"
-	"github.com/cockroachdb/cockroach/pkg/testutils/skip"
 	"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
 	"github.com/cockroachdb/cockroach/pkg/util/ctxgroup"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
@@ -649,9 +648,6 @@ func TestCSVImportCanBeResumed(t *testing.T) {
 	defer leaktest.AfterTest(t)()
 	defer log.Scope(t).Close(t)
 
-	// Flaky test.
-	skip.WithIssue(t, 91828)
-
 	defer setImportReaderParallelism(1)()
 	const batchSize = 5
 	defer TestingSetParallelImporterReaderBatchSize(batchSize)()
@@ -674,6 +670,7 @@ func TestCSVImportCanBeResumed(t *testing.T) {
 	defer s.Stopper().Stop(ctx)
 
 	sqlDB := sqlutils.MakeSQLRunner(db)
+	setSmallIngestBufferSizes(t, sqlDB)
 	sqlDB.Exec(t, `CREATE DATABASE d`)
 	sqlDB.Exec(t, "CREATE TABLE t (id INT, data STRING)")
 	defer sqlDB.Exec(t, `DROP TABLE t`)

diff --git a/pkg/sql/importer/import_stmt_test.go b/pkg/sql/importer/import_stmt_test.go
@@ -2944,8 +2944,8 @@ func TestImportRetriesBreakerOpenFailure(t *testing.T) {
 	}
 
 	sqlDB := sqlutils.MakeSQLRunner(tc.ServerConn(0))
+	setSmallIngestBufferSizes(t, sqlDB)
 	sqlDB.Exec(t, `CREATE TABLE t (a INT, b STRING)`)
-	sqlDB.Exec(t, `SET CLUSTER SETTING kv.bulk_ingest.pk_buffer_size = '16MiB'`)
 	var tableID int64
 	sqlDB.QueryRow(t, `SELECT id FROM system.namespace WHERE name = 't'`).Scan(&tableID)
 
@@ -5271,6 +5271,15 @@ func TestImportControlJobRBAC(t *testing.T) {
 	}
 }
 
+// setSmallIngestBufferSizes lowers the initial buffering adder ingest
+// size to allow import jobs to run without exceeding the test memory
+// monitor.
+func setSmallIngestBufferSizes(t *testing.T, sqlDB *sqlutils.SQLRunner) {
+	t.Helper()
+	sqlDB.Exec(t, `SET CLUSTER SETTING kv.bulk_ingest.pk_buffer_size = '16MiB'`)
+	sqlDB.Exec(t, `SET CLUSTER SETTING kv.bulk_ingest.index_buffer_size = '16MiB'`)
+}
+
 // TestImportWorkerFailure tests that IMPORT retries after the failure of a
 // worker node.
 func TestImportWorkerFailure(t *testing.T) {
@@ -5289,11 +5298,7 @@ func TestImportWorkerFailure(t *testing.T) {
 	defer tc.Stopper().Stop(ctx)
 	conn := tc.ServerConn(0)
 	sqlDB := sqlutils.MakeSQLRunner(conn)
-
-	// Lower the initial buffering adder ingest size to allow import jobs to run
-	// without exceeding the test memory monitor.
-	sqlDB.Exec(t, `SET CLUSTER SETTING kv.bulk_ingest.pk_buffer_size = '16MiB'`)
-	sqlDB.Exec(t, `SET CLUSTER SETTING kv.bulk_ingest.index_buffer_size = '16MiB'`)
+	setSmallIngestBufferSizes(t, sqlDB)
 
 	srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		if r.Method == "GET" {

diff --git a/pkg/sql/logictest/logic.go b/pkg/sql/logictest/logic.go
@@ -1279,8 +1279,11 @@ func (t *logicTest) newTestServerCluster(bootstrapBinaryPath string, upgradeBina
 	if err != nil {
 		t.Fatal(err)
 	}
-	if err := ts.WaitForInit(); err != nil {
-		t.Fatal(err)
+	for i := 0; i < t.cfg.NumNodes; i++ {
+		// Wait for each node to be reachable.
+		if err := ts.WaitForInitFinishForNode(i); err != nil {
+			t.Fatal(err)
+		}
 	}
 
 	t.testserverCluster = ts
@@ -1815,7 +1818,7 @@ func (t *logicTest) setup(
 	t.testCleanupFuncs = append(t.testCleanupFuncs, tempExternalIODirCleanup)
 
 	if cfg.UseCockroachGoTestserver {
-		skip.WithIssue(t.t(), 92637)
+		skip.UnderStress(t.t(), "test takes a long time and downloads release artifacts")
 		if !bazel.BuiltWithBazel() {
 			skip.IgnoreLint(t.t(), "cockroach-go/testserver can only be uzed in bazel builds")
 		}

diff --git a/pkg/sql/logictest/logictestbase/logictestbase.go b/pkg/sql/logictest/logictestbase/logictestbase.go
@@ -475,16 +475,14 @@ var LogicTestConfigs = []TestClusterConfig{
 		Name:                        "cockroach-go-testserver-22.2-master",
 		UseCockroachGoTestserver:    true,
 		NumNodes:                    3,
-		CockroachGoBootstrapVersion: "v22.2.0-rc.1",
-		BootstrapVersion:            roachpb.Version{Major: 22, Minor: 2},
+		CockroachGoBootstrapVersion: "v22.2.1",
 	},
 	{
 		Name:                        "cockroach-go-testserver-22.1-22.2",
 		UseCockroachGoTestserver:    true,
 		NumNodes:                    3,
 		CockroachGoBootstrapVersion: "v22.1.6",
-		CockroachGoUpgradeVersion:   "v22.2.0-rc.1",
-		BootstrapVersion:            roachpb.Version{Major: 22, Minor: 1},
+		CockroachGoUpgradeVersion:   "v22.2.1",
 	},
 }
 

diff --git a/pkg/storage/mvcc.go b/pkg/storage/mvcc.go
@@ -3556,6 +3556,9 @@ func recordIteratorStats(ctx context.Context, iter MVCCIterator) {
 		ValueBytes:                     stats.InternalStats.ValueBytes,
 		PointCount:                     stats.InternalStats.PointCount,
 		PointsCoveredByRangeTombstones: stats.InternalStats.PointsCoveredByRangeTombstones,
+		RangeKeyCount:                  uint64(stats.RangeKeyStats.Count),
+		RangeKeyContainedPoints:        uint64(stats.RangeKeyStats.ContainedPoints),
+		RangeKeySkippedPoints:          uint64(stats.RangeKeyStats.SkippedPoints),
 	})
 }