Merge cockroachdb#62175 cockroachdb#62492 cockroachdb#62511 cockroach…

…db#62534 cockroachdb#62535 cockroachdb#62548 cockroachdb#62549

62175: sql: add crdb_internal.reset_sql_stats() builtin r=asubiotto a=Azhng

Previously, there was no mechanism to immediately clear SQL statistics. Users
would have to wait until the reset interval expires. This commit creates a
builtin to immediately clears SQL stats.

Release note (sql change): SQL stats can now be cleared using
crdb_internal.reset_sql_stats()

Addresses cockroachdb#33315 

62492: roachtest: add hibernate-spatial test r=rafiss a=otan

* Bump hibernate to 5.4.30
* Add hibernate-spatial test which tests cockroachdb against hibernate's
  spatial suite. Used a separate suite because the directory magic of
  copying may not work since the set of running tests overlap a bit.

Release note: None

62511: geo/geomfn: fix st_linelocatepoint to work with ZM coords r=otan a=andyyang890

Previously, st_linelocatepoint would panic when the
line had Z and/or M coordinates.

Release note: None

62534: sql: default to batch size 1 in allocator r=yuzefovich a=erikgrinaker

In cockroachdb#62282, the estimated row count was passed into the scan batch
allocator to avoid growing the batch from 1. However, this also changed
the default batch size from 1 to 1024 when no row count estimate was
available, giving significant overhead when fetching small result sets.
On `kv95/enc=false/nodes=1/cpu=32` this reduced performance from 66304
ops/s to 58862 ops/s (median of 5 runs), since these are single-row
reads without estimates.

This patch reverts the default batch size to 1 when no row count
estimate is available. This fully fixes the `kv95` performance
regression. YCSB/E takes a small hit going from 1895 ops/s to 1786
ops/s, but this only seems to happen because it takes a while for the
statistics to update: sometime within the first minute of the test
(after the 1-minute ramp-up period), throughput abruptly changes from
~700 ops/s to ~1800 ops/s, so using a 2-minute ramp-up period in
roachtest would mostly eliminate any difference.

Resolves cockroachdb#62524.

Release note: None

62535: roachtest: use 2-minute ramp times for YCSB workloads r=yuzefovich a=erikgrinaker

In cockroachdb#62534 it was shown that it takes up to two minutes before we have
good enough statistics to allocate appropriately sized batches. However,
the YCSB workloads only had a 1-minute ramp time, which would skew the
results as throughput would abruptly change when the statistics were
updated during the test.

This patch changes the ramp time for YCSB workloads to 2 minutes to make
sure we have appropriate statistics before starting the actual test.

Release note: None

62548: bazel: mark logictest as working in bazel r=rickystewart a=rickystewart

Release note: None

62549: workload/schemachange: temporarily disable ADD REGION r=ajstorm,postamar a=otan

This is causing flakes in CI.
Resolves cockroachdb#62503 

Release note: None

Co-authored-by: Azhng <[email protected]>
Co-authored-by: Oliver Tan <[email protected]>
Co-authored-by: Andy Yang <[email protected]>
Co-authored-by: Erik Grinaker <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>

docs/generated/http/full.md

@@ -1369,7 +1369,7 @@ Support status: [reserved](#support-status)
 
 
 
-Request object for issing a query cancel request.
+Request object for issuing a query cancel request.
 
 
 | Field | Type | Label | Description | Support status |
@@ -2834,6 +2834,46 @@ Support status: [reserved](#support-status)
 
 
 
+## ResetSQLStats
+
+`POST /_status/resetsqlstats`
+
+
+
+Support status: [reserved](#support-status)
+
+#### Request Parameters
+
+
+
+
+Request object for issuing a SQL stats reset request.
+
+
+| Field | Type | Label | Description | Support status |
+| ----- | ---- | ----- | ----------- | -------------- |
+| node_id | [string](#cockroach.server.serverpb.ResetSQLStatsRequest-string) |  |  | [reserved](#support-status) |
+
+
+
+
+
+
+
+#### Response Parameters
+
+
+
+
+Response object returned by ResetSQLStats.
+
+
+
+
+
+
+
+
 ## Users
 
 `GET /_admin/v1/users`

docs/generated/sql/functions.md

@@ -2714,6 +2714,8 @@ SELECT * FROM crdb_internal.check_consistency(true, ‘\x02’, ‘\x04’)</p>
 </span></td></tr>
 <tr><td><a name="crdb_internal.range_stats"></a><code>crdb_internal.range_stats(key: <a href="bytes.html">bytes</a>) &rarr; jsonb</code></td><td><span class="funcdesc"><p>This function is used to retrieve range statistics information as a JSON object.</p>
 </span></td></tr>
+<tr><td><a name="crdb_internal.reset_sql_stats"></a><code>crdb_internal.reset_sql_stats() &rarr; <a href="bool.html">bool</a></code></td><td><span class="funcdesc"><p>This function is used to clear the collected SQL statistics.</p>
+</span></td></tr>
 <tr><td><a name="crdb_internal.round_decimal_values"></a><code>crdb_internal.round_decimal_values(val: <a href="decimal.html">decimal</a>, scale: <a href="int.html">int</a>) &rarr; <a href="decimal.html">decimal</a></code></td><td><span class="funcdesc"><p>This function is used internally to round decimal values during mutations.</p>
 </span></td></tr>
 <tr><td><a name="crdb_internal.round_decimal_values"></a><code>crdb_internal.round_decimal_values(val: <a href="decimal.html">decimal</a>[], scale: <a href="int.html">int</a>) &rarr; <a href="decimal.html">decimal</a>[]</code></td><td><span class="funcdesc"><p>This function is used internally to round decimal array values during mutations.</p>

pkg/ccl/logictestccl/BUILD.bazel

@@ -9,7 +9,7 @@ go_library(
 
 go_test(
     name = "logictestccl_test",
-    size = "large",
+    size = "enormous",
     srcs = [
         "logic_test.go",
         "main_test.go",

pkg/ccl/multiregionccl/BUILD.bazel

@@ -21,6 +21,7 @@ go_library(
 
 go_test(
     name = "multiregionccl_test",
+    size = "enormous",
     srcs = [
         "main_test.go",
         "multiregion_test.go",

pkg/ccl/workloadccl/allccl/BUILD.bazel

@@ -35,7 +35,7 @@ go_library(
 
 go_test(
     name = "allccl_test",
-    size = "medium",
+    size = "large",
     srcs = [
         "all_test.go",
         "main_test.go",

pkg/cmd/roachtest/hibernate.go

@@ -12,16 +12,57 @@ package main
 
 import (
 	"context"
+	"fmt"
 	"regexp"
 )
 
 var hibernateReleaseTagRegex = regexp.MustCompile(`^(?P<major>\d+)\.(?P<minor>\d+)\.(?P<point>\d+)$`)
-var supportedHibernateTag = "5.4.20"
+var supportedHibernateTag = "5.4.30"
+
+type hibernateOptions struct {
+	testName string
+	testDir  string
+	buildCmd,
+	testCmd string
+	blocklists  blocklistsForVersion
+	dbSetupFunc func(ctx context.Context, t *test, c *cluster)
+}
+
+var (
+	hibernateOpts = hibernateOptions{
+		testName: "hibernate",
+		testDir:  "hibernate-core",
+		buildCmd: `cd /mnt/data1/hibernate/hibernate-core/ && ./../gradlew test -Pdb=cockroachdb ` +
+			`--tests org.hibernate.jdbc.util.BasicFormatterTest.*`,
+		testCmd:     "./gradlew test -Pdb=cockroachdb",
+		blocklists:  hibernateBlocklists,
+		dbSetupFunc: nil,
+	}
+	hibernateSpatialOpts = hibernateOptions{
+		testName: "hibernate-spatial",
+		testDir:  "hibernate-spatial",
+		buildCmd: `cd /mnt/data1/hibernate/hibernate-spatial/ && ./../gradlew test -Pdb=cockroachdb_spatial ` +
+			`--tests org.hibernate.spatial.dialect.postgis.*`,
+		testCmd: `cd /mnt/data1/hibernate/hibernate-spatial && ` +
+			`HIBERNATE_CONNECTION_LEAK_DETECTION=true ./../gradlew test -Pdb=cockroachdb_spatial`,
+		blocklists: hibernateSpatialBlocklists,
+		dbSetupFunc: func(ctx context.Context, t *test, c *cluster) {
+			db := c.Conn(ctx, 1)
+			defer db.Close()
+			if _, err := db.ExecContext(
+				ctx,
+				"SET CLUSTER SETTING sql.spatial.experimental_box2d_comparison_operators.enabled = on",
+			); err != nil {
+				t.Fatal(err)
+			}
+		},
+	}
+)
 
-// This test runs hibernate-core's full test suite against a single cockroach
+// This test runs one of hibernate's test suite against a single cockroach
 // node.
 
-func registerHibernate(r *testRegistry) {
+func registerHibernate(r *testRegistry, opt hibernateOptions) {
 	runHibernate := func(
 		ctx context.Context,
 		t *test,
@@ -33,8 +74,15 @@ func registerHibernate(r *testRegistry) {
 		node := c.Node(1)
 		t.Status("setting up cockroach")
 		c.Put(ctx, cockroach, "./cockroach", c.All())
+		if err := c.PutLibraries(ctx, "./lib"); err != nil {
+			t.Fatal(err)
+		}
 		c.Start(ctx, t, c.All())
 
+		if opt.dbSetupFunc != nil {
+			opt.dbSetupFunc(ctx, t, c)
+		}
+
 		version, err := fetchCockroachVersion(ctx, c, node[0])
 		if err != nil {
 			t.Fatal(err)
@@ -98,29 +146,22 @@ func registerHibernate(r *testRegistry) {
 			c,
 			node,
 			"building hibernate (without tests)",
-			`cd /mnt/data1/hibernate/hibernate-core/ && ./../gradlew test -Pdb=cockroachdb `+
-				`--tests org.hibernate.jdbc.util.BasicFormatterTest.*`,
+			opt.buildCmd,
 		); err != nil {
 			t.Fatal(err)
 		}
 
-		blocklistName, expectedFailures, _, _ := hibernateBlocklists.getLists(version)
+		blocklistName, expectedFailures, _, _ := opt.blocklists.getLists(version)
 		if expectedFailures == nil {
 			t.Fatalf("No hibernate blocklist defined for cockroach version %s", version)
 		}
 		c.l.Printf("Running cockroach version %s, using blocklist %s", version, blocklistName)
 
 		t.Status("running hibernate test suite, will take at least 3 hours")
-		// When testing, it is helpful to run only a subset of the tests. To do so
-		// add "--tests org.hibernate.test.annotations.lob.*" to the end of the
-		// test run command.
 		// Note that this will take upwards of 3 hours.
 		// Also note that this is expected to return an error, since the test suite
 		// will fail. And it is safe to swallow it here.
-		_ = c.RunE(ctx, node,
-			`cd /mnt/data1/hibernate/ && `+
-				`HIBERNATE_CONNECTION_LEAK_DETECTION=true ./gradlew test -Pdb=cockroachdb`,
-		)
+		_ = c.RunE(ctx, node, opt.testCmd)
 
 		t.Status("collecting the test results")
 		// Copy all of the test results to the cockroach logs directory to be
@@ -132,7 +173,7 @@ func registerHibernate(r *testRegistry) {
 			c,
 			node,
 			"copy html report",
-			`cp /mnt/data1/hibernate/hibernate-core/target/reports/tests/test ~/logs/report -a`,
+			fmt.Sprintf(`cp /mnt/data1/hibernate/%s/target/reports/tests/test ~/logs/report -a`, opt.testDir),
 		); err != nil {
 			t.Fatal(err)
 		}
@@ -143,7 +184,7 @@ func registerHibernate(r *testRegistry) {
 			c,
 			node,
 			"copy test result files",
-			`cp /mnt/data1/hibernate/hibernate-core/target/test-results/test ~/logs/report/results -a`,
+			fmt.Sprintf(`cp /mnt/data1/hibernate/%s/target/test-results/test ~/logs/report/results -a`, opt.testDir),
 		); err != nil {
 			t.Fatal(err)
 		}
@@ -156,7 +197,7 @@ func registerHibernate(r *testRegistry) {
 			t.l,
 			node,
 			"get list of test files",
-			`ls /mnt/data1/hibernate/*/target/test-results/test/*.xml`,
+			fmt.Sprintf(`ls /mnt/data1/hibernate/%s/target/test-results/test/*.xml`, opt.testDir),
 		)
 		if err != nil {
 			t.Fatal(err)
@@ -172,7 +213,7 @@ func registerHibernate(r *testRegistry) {
 	}
 
 	r.Add(testSpec{
-		Name:    "hibernate",
+		Name:    opt.testName,
 		Owner:   OwnerSQLExperience,
 		Cluster: makeClusterSpec(1),
 		Tags:    []string{`default`, `orm`},

pkg/cmd/roachtest/hibernate_blocklist.go

@@ -21,9 +21,15 @@ var hibernateBlocklists = blocklistsForVersion{
 	{"v21.1", "hibernateBlockList21_1", hibernateBlockList21_1, "", nil},
 }
 
+var hibernateSpatialBlocklists = blocklistsForVersion{
+	{"v21.1", "hibernateSpatialBlockList21_1", hibernateSpatialBlockList21_1, "", nil},
+}
+
 // Please keep these lists alphabetized for easy diffing.
 // After a failed run, an updated version of this blocklist should be available
 // in the test log.
+var hibernateSpatialBlockList21_1 = blocklist{}
+
 var hibernateBlockList21_1 = blocklist{}
 
 var hibernateBlockList20_2 = blocklist{}

pkg/cmd/roachtest/registry.go

@@ -39,7 +39,8 @@ func registerTests(r *testRegistry) {
 	registerFollowerReads(r)
 	registerGopg(r)
 	registerGossip(r)
-	registerHibernate(r)
+	registerHibernate(r, hibernateOpts)
+	registerHibernate(r, hibernateSpatialOpts)
 	registerHotSpotSplits(r)
 	registerImportDecommissioned(r)
 	registerImportMixedVersion(r)

pkg/cmd/roachtest/ycsb.go

@@ -49,7 +49,7 @@ func registerYCSB(r *testRegistry) {
 		m := newMonitor(ctx, c, c.Range(1, nodes))
 		m.Go(func(ctx context.Context) error {
 			sfu := fmt.Sprintf(" --select-for-update=%t", t.IsBuildVersion("v19.2.0"))
-			ramp := " --ramp=" + ifLocal("0s", "1m")
+			ramp := " --ramp=" + ifLocal("0s", "2m")
 			duration := " --duration=" + ifLocal("10s", "10m")
 			cmd := fmt.Sprintf(
 				"./workload run ycsb --init --insert-count=1000000 --workload=%s --concurrency=%d"+

pkg/geo/geomfn/linestring.go

@@ -91,8 +91,9 @@ func LineLocatePoint(line geo.Geometry, point geo.Geometry) (float64, error) {
 	}
 
 	p := closestT.(*geom.Point)
+	lineStart := geom.Coord{lineString.Coord(0).X(), lineString.Coord(0).Y()}
 	// build new line segment to the closest point we found
-	lineSegment := geom.NewLineString(geom.XY).MustSetCoords([]geom.Coord{lineString.Coord(0), p.Coords()})
+	lineSegment := geom.NewLineString(geom.XY).MustSetCoords([]geom.Coord{lineStart, p.Coords()})
 
 	// compute fraction of new line segment compared to total line length
 	return lineSegment.Length() / lineString.Length(), nil

pkg/geo/geomfn/linestring_test.go

@@ -168,6 +168,11 @@ func TestLineLocatePoint(t *testing.T) {
 			point:      geom.NewPointFlat(geom.XY, []float64{3, 1}),
 			expected:   0.87,
 		},
+		{
+			lineString: geom.NewLineStringFlat(geom.XYZ, []float64{0, 0, 5, 1, 1, 26}),
+			point:      geom.NewPointFlat(geom.XYZ, []float64{0, 1, -1}),
+			expected:   0.5,
+		},
 	}
 
 	for index, tc := range testCases {

pkg/server/BUILD.bazel

@@ -36,6 +36,7 @@ go_library(
         "server_systemlog_gc.go",
         "settings_cache.go",
         "settingsworker.go",
+        "sql_stats.go",
         "statement_diagnostics_requests.go",
         "statements.go",
         "status.go",

pkg/server/serverpb/status.go

@@ -25,6 +25,7 @@ type SQLStatusServer interface {
 	CancelSession(context.Context, *CancelSessionRequest) (*CancelSessionResponse, error)
 	ListContentionEvents(context.Context, *ListContentionEventsRequest) (*ListContentionEventsResponse, error)
 	ListLocalContentionEvents(context.Context, *ListContentionEventsRequest) (*ListContentionEventsResponse, error)
+	ResetSQLStats(context.Context, *ResetSQLStatsRequest) (*ResetSQLStatsResponse, error)
 }
 
 // OptionalNodesStatusServer is a StatusServer that is only optionally present