From 20161f90c41878f3c933781c22de54b112731ccc Mon Sep 17 00:00:00 2001 From: irfan sharif Date: Fri, 24 Dec 2021 08:54:14 -0500 Subject: [PATCH 1/5] kvserver: de-flake TestProtectedTimestamps Fixed #75020. This test makes use of an "upsert-until-backpressure" primitive, where it continually writes blobs to a scratch table configured with a lower max range size (1<<18 from the default of 512<<20) until it experiences replica backpressure. Before #73876 (when using the system config span), the splitting off of the scratch table's ranges happened near instantly after the creation of the table itself. This changed slightly with the span configs infrastructure, where there's more of asynchrony built in and ranges may appear after a while longer. The test previously started upserting as soon as it created the table, being able to implicitly rely on the tight synchrony of already having the table's ranges carved out. This comes when deciding to record a replica's largest previous max range size -- something we only do if the replica's current size is larger than the new limit (see (*Replica).SetSpanCnfig). When we weren't upserting until the range applied the latest config, this was not possible. With span configs and its additional asynchrony, this assumption no longer held. It was possible then for the range to accept writes larger than the newest max range size, which unintentionally (for this test at least) triggers an escape hatch where we avoid backpressure when lowering a range's max size below its current size (#46863). The failure is easy to repro. This test runs in ~8s, and with the following we were reliably seeing timeouts where the test was waiting for backpressure to kick in (but it never did because of the escape hatch). dev test pkg/kv/kvserver \ -f TestProtectedTimestamps -v --show-logs \ --timeout 300s --ignore-cache --count 10 De-flaking this test is simple -- we just wait for the table's replicas to apply the latest config before issuing writes to it. Release note: None --- pkg/kv/kvserver/client_protectedts_test.go | 45 +++++++++++++++++++--- 1 file changed, 40 insertions(+), 5 deletions(-) diff --git a/pkg/kv/kvserver/client_protectedts_test.go b/pkg/kv/kvserver/client_protectedts_test.go index fa1a0867078d..da28e02c3ef9 100644 --- a/pkg/kv/kvserver/client_protectedts_test.go +++ b/pkg/kv/kvserver/client_protectedts_test.go @@ -68,8 +68,12 @@ func TestProtectedTimestamps(t *testing.T) { _, err = conn.Exec("SET CLUSTER SETTING kv.protectedts.poll_interval = '10ms';") require.NoError(t, err) - _, err = conn.Exec("ALTER TABLE foo CONFIGURE ZONE USING " + - "gc.ttlseconds = 1, range_max_bytes = 1<<18, range_min_bytes = 1<<10;") + _, err = conn.Exec("SET CLUSTER SETTING kv.closed_timestamp.target_duration = '100ms'") // speeds up the test + require.NoError(t, err) + + const tableRangeMaxBytes = 1 << 18 + _, err = conn.Exec("ALTER TABLE foo CONFIGURE ZONE USING "+ + "gc.ttlseconds = 1, range_max_bytes = $1, range_min_bytes = 1<<10;", tableRangeMaxBytes) require.NoError(t, err) rRand, _ := randutil.NewTestRand() @@ -86,7 +90,25 @@ func TestProtectedTimestamps(t *testing.T) { const processedPattern = `(?s)shouldQueue=true.*processing replica.*GC score after GC` processedRegexp := regexp.MustCompile(processedPattern) - getTableStartKey := func(table string) roachpb.Key { + waitForTableSplit := func() { + testutils.SucceedsSoon(t, func() error { + count := 0 + if err := conn.QueryRow( + "SELECT count(*) "+ + "FROM crdb_internal.ranges_no_leases "+ + "WHERE table_name = $1 "+ + "AND database_name = current_database()", + "foo").Scan(&count); err != nil { + return err + } + if count == 0 { + return errors.New("waiting for table split") + } + return nil + }) + } + + getTableStartKey := func() roachpb.Key { row := conn.QueryRow( "SELECT start_key "+ "FROM crdb_internal.ranges_no_leases "+ @@ -102,7 +124,7 @@ func TestProtectedTimestamps(t *testing.T) { } getStoreAndReplica := func() (*kvserver.Store, *kvserver.Replica) { - startKey := getTableStartKey("foo") + startKey := getTableStartKey() // Okay great now we have a key and can go find replicas and stores and what not. r := tc.LookupRangeOrFatal(t, startKey) l, _, err := tc.FindRangeLease(r, nil) @@ -112,6 +134,16 @@ func TestProtectedTimestamps(t *testing.T) { return getFirstStoreReplica(t, lhServer, startKey) } + waitForRangeMaxBytes := func(maxBytes int64) { + testutils.SucceedsSoon(t, func() error { + _, r := getStoreAndReplica() + if r.GetMaxBytes() != maxBytes { + return errors.New("waiting for range_max_bytes to be applied") + } + return nil + }) + } + gcSoon := func() { testutils.SucceedsSoon(t, func() error { upsertUntilBackpressure() @@ -134,13 +166,16 @@ func TestProtectedTimestamps(t *testing.T) { return thresh } + waitForTableSplit() + waitForRangeMaxBytes(tableRangeMaxBytes) + beforeWrites := s0.Clock().Now() gcSoon() pts := ptstorage.New(s0.ClusterSettings(), s0.InternalExecutor().(*sql.InternalExecutor), nil /* knobs */) ptsWithDB := ptstorage.WithDatabase(pts, s0.DB()) - startKey := getTableStartKey("foo") + startKey := getTableStartKey() ptsRec := ptpb.Record{ ID: uuid.MakeV4().GetBytes(), Timestamp: s0.Clock().Now(), From c02b3b186a319f8e68212f745cbe09bacbb44a54 Mon Sep 17 00:00:00 2001 From: Thomas Hardy Date: Mon, 24 Jan 2022 11:12:39 -0500 Subject: [PATCH 2/5] sql: revert formatting all query statements Previously, all statements were formatted prior to being sent to the frontend. However, the formatting was causing statements queries from the frontend to run noticeably more slowly. This change removes the logic that formats all queries, but keeps the addition of the new builtin function prettify_statement. Release note (sql change): statements are no longer formatted prior to being sent to the UI, but the new builtin function remains. --- .../serverccl/statusccl/tenant_status_test.go | 122 +++++------------- pkg/server/combined_statement_stats.go | 10 +- pkg/server/index_usage_stats.go | 8 +- pkg/server/status_test.go | 38 ++---- 4 files changed, 48 insertions(+), 130 deletions(-) diff --git a/pkg/ccl/serverccl/statusccl/tenant_status_test.go b/pkg/ccl/serverccl/statusccl/tenant_status_test.go index 9eb14676c565..ef0a87c06a17 100644 --- a/pkg/ccl/serverccl/statusccl/tenant_status_test.go +++ b/pkg/ccl/serverccl/statusccl/tenant_status_test.go @@ -28,7 +28,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/spanconfig" "github.com/cockroachdb/cockroach/pkg/sql/catalog/catconstants" "github.com/cockroachdb/cockroach/pkg/sql/catalog/descpb" - "github.com/cockroachdb/cockroach/pkg/sql/sem/tree" "github.com/cockroachdb/cockroach/pkg/sql/sqlstats" "github.com/cockroachdb/cockroach/pkg/sql/tests" "github.com/cockroachdb/cockroach/pkg/testutils" @@ -116,35 +115,19 @@ func TestTenantCannotSeeNonTenantStats(t *testing.T) { tenantStatusServer := tenant.StatusServer().(serverpb.SQLStatusServer) type testCase struct { - stmt string - formattedStmt string - fingerprint string - formattedFingerprint string + stmt string + fingerprint string } testCaseTenant := []testCase{ + {stmt: `CREATE DATABASE roachblog_t`}, + {stmt: `SET database = roachblog_t`}, + {stmt: `CREATE TABLE posts_t (id INT8 PRIMARY KEY, body STRING)`}, { - stmt: `CREATE DATABASE roachblog_t`, - formattedStmt: "CREATE DATABASE roachblog_t\n", - }, - { - stmt: `SET database = roachblog_t`, - formattedStmt: "SET database = roachblog_t\n", - }, - { - stmt: `CREATE TABLE posts_t (id INT8 PRIMARY KEY, body STRING)`, - formattedStmt: "CREATE TABLE posts_t (id INT8 PRIMARY KEY, body STRING)\n", - }, - { - stmt: `INSERT INTO posts_t VALUES (1, 'foo')`, - fingerprint: `INSERT INTO posts_t VALUES (_, '_')`, - formattedStmt: "INSERT INTO posts_t VALUES (1, 'foo')\n", - formattedFingerprint: "INSERT INTO posts_t VALUES (_, '_')\n", - }, - { - stmt: `SELECT * FROM posts_t`, - formattedStmt: "SELECT * FROM posts_t\n", + stmt: `INSERT INTO posts_t VALUES (1, 'foo')`, + fingerprint: `INSERT INTO posts_t VALUES (_, '_')`, }, + {stmt: `SELECT * FROM posts_t`}, } for _, stmt := range testCaseTenant { @@ -156,28 +139,14 @@ func TestTenantCannotSeeNonTenantStats(t *testing.T) { require.NoError(t, err) testCaseNonTenant := []testCase{ + {stmt: `CREATE DATABASE roachblog_nt`}, + {stmt: `SET database = roachblog_nt`}, + {stmt: `CREATE TABLE posts_nt (id INT8 PRIMARY KEY, body STRING)`}, { - stmt: `CREATE DATABASE roachblog_nt`, - formattedStmt: "CREATE DATABASE roachblog_nt\n", - }, - { - stmt: `SET database = roachblog_nt`, - formattedStmt: "SET database = roachblog_nt\n", - }, - { - stmt: `CREATE TABLE posts_nt (id INT8 PRIMARY KEY, body STRING)`, - formattedStmt: "CREATE TABLE posts_nt (id INT8 PRIMARY KEY, body STRING)\n", - }, - { - stmt: `INSERT INTO posts_nt VALUES (1, 'foo')`, - fingerprint: `INSERT INTO posts_nt VALUES (_, '_')`, - formattedStmt: "INSERT INTO posts_nt VALUES (1, 'foo')\n", - formattedFingerprint: "INSERT INTO posts_nt VALUES (_, '_')\n", - }, - { - stmt: `SELECT * FROM posts_nt`, - formattedStmt: "SELECT * FROM posts_nt\n", + stmt: `INSERT INTO posts_nt VALUES (1, 'foo')`, + fingerprint: `INSERT INTO posts_nt VALUES (_, '_')`, }, + {stmt: `SELECT * FROM posts_nt`}, } pgURL, cleanupGoDB := sqlutils.PGUrl( @@ -230,22 +199,14 @@ func TestTenantCannotSeeNonTenantStats(t *testing.T) { err = serverutils.GetJSONProto(nonTenant, path, &nonTenantCombinedStats) require.NoError(t, err) - checkStatements := func(t *testing.T, tc []testCase, actual *serverpb.StatementsResponse, combined bool) { + checkStatements := func(t *testing.T, tc []testCase, actual *serverpb.StatementsResponse) { t.Helper() var expectedStatements []string for _, stmt := range tc { var expectedStmt = stmt.stmt - if combined { - expectedStmt = stmt.formattedStmt - } if stmt.fingerprint != "" { - if combined { - expectedStmt = stmt.formattedFingerprint - } else { - expectedStmt = stmt.fingerprint - } + expectedStmt = stmt.fingerprint } - expectedStatements = append(expectedStatements, expectedStmt) } @@ -272,14 +233,14 @@ func TestTenantCannotSeeNonTenantStats(t *testing.T) { // First we verify that we have expected stats from tenants. t.Run("tenant-stats", func(t *testing.T) { - checkStatements(t, testCaseTenant, tenantStats, false) - checkStatements(t, testCaseTenant, tenantCombinedStats, true) + checkStatements(t, testCaseTenant, tenantStats) + checkStatements(t, testCaseTenant, tenantCombinedStats) }) // Now we verify the non tenant stats are what we expected. t.Run("non-tenant-stats", func(t *testing.T) { - checkStatements(t, testCaseNonTenant, &nonTenantStats, false) - checkStatements(t, testCaseNonTenant, &nonTenantCombinedStats, true) + checkStatements(t, testCaseNonTenant, &nonTenantStats) + checkStatements(t, testCaseNonTenant, &nonTenantCombinedStats) }) // Now we verify that tenant and non-tenant have no visibility into each other's stats. @@ -313,29 +274,10 @@ func TestTenantCannotSeeNonTenantStats(t *testing.T) { func testResetSQLStatsRPCForTenant( ctx context.Context, t *testing.T, testHelper *tenantTestHelper, ) { - - type testCase struct { - stmt string - formattedStmt string - } - stmts := []testCase{ - { - stmt: "SELECT 1", - formattedStmt: "SELECT 1\n", - }, - { - stmt: "SELECT 1, 1", - formattedStmt: "SELECT 1, 1\n", - }, - { - stmt: "SELECT 1, 1, 1", - formattedStmt: "SELECT 1, 1\n", - }, - } - - var expectedStatements []string - for _, tc := range stmts { - expectedStatements = append(expectedStatements, tc.formattedStmt) + stmts := []string{ + "SELECT 1", + "SELECT 1, 1", + "SELECT 1, 1, 1", } testCluster := testHelper.testCluster() @@ -365,8 +307,8 @@ func testResetSQLStatsRPCForTenant( }() for _, stmt := range stmts { - testCluster.tenantConn(randomServer).Exec(t, stmt.stmt) - controlCluster.tenantConn(randomServer).Exec(t, stmt.stmt) + testCluster.tenantConn(randomServer).Exec(t, stmt) + controlCluster.tenantConn(randomServer).Exec(t, stmt) } if flushed { @@ -383,7 +325,7 @@ func testResetSQLStatsRPCForTenant( require.NotEqual(t, 0, len(statsPreReset.Statements), "expected to find stats for at least one statement, but found: %d", len(statsPreReset.Statements)) - ensureExpectedStmtFingerprintExistsInRPCResponse(t, expectedStatements, statsPreReset, "test") + ensureExpectedStmtFingerprintExistsInRPCResponse(t, stmts, statsPreReset, "test") _, err = status.ResetSQLStats(ctx, &serverpb.ResetSQLStatsRequest{ ResetPersistedStats: true, @@ -420,7 +362,7 @@ func testResetSQLStatsRPCForTenant( }) require.NoError(t, err) - ensureExpectedStmtFingerprintExistsInRPCResponse(t, expectedStatements, statsFromControlCluster, "control") + ensureExpectedStmtFingerprintExistsInRPCResponse(t, stmts, statsFromControlCluster, "control") }) } } @@ -607,10 +549,10 @@ SET DATABASE=test_db1; SELECT * FROM test; `) - getCreateStmtQuery := fmt.Sprintf(` - SELECT prettify_statement(indexdef, %d, %d, %d) - FROM pg_catalog.pg_indexes - WHERE tablename = 'test' AND indexname = $1`, tree.ConsoleLineWidth, tree.PrettyAlignAndDeindent, tree.UpperCase) + getCreateStmtQuery := ` +SELECT indexdef +FROM pg_catalog.pg_indexes +WHERE tablename = 'test' AND indexname = $1` // Get index usage stats and assert expected results. resp := getTableIndexStats(testHelper, "test_db1") diff --git a/pkg/server/combined_statement_stats.go b/pkg/server/combined_statement_stats.go index 88f8b7447eaf..44183eeaf23c 100644 --- a/pkg/server/combined_statement_stats.go +++ b/pkg/server/combined_statement_stats.go @@ -116,18 +116,12 @@ func collectCombinedStatements( transaction_fingerprint_id, app_name, aggregated_ts, - jsonb_set( - metadata, - array['query'], - to_jsonb( - prettify_statement(metadata ->> 'query', %d, %d, %d) - ) - ), + metadata, statistics, sampled_plan, aggregation_interval FROM crdb_internal.statement_statistics - %s`, tree.ConsoleLineWidth, tree.PrettyAlignAndDeindent, tree.UpperCase, whereClause) + %s`, whereClause) const expectedNumDatums = 8 diff --git a/pkg/server/index_usage_stats.go b/pkg/server/index_usage_stats.go index 12332e6eae7c..102d864469c3 100644 --- a/pkg/server/index_usage_stats.go +++ b/pkg/server/index_usage_stats.go @@ -223,13 +223,14 @@ func getTableIndexUsageStats( q := makeSQLQuery() // TODO(#72930): Implement virtual indexes on index_usages_statistics and table_indexes - q.Append(`SELECT + q.Append(` + SELECT ti.index_id, ti.index_name, ti.index_type, total_reads, last_read, - prettify_statement(indexdef, $, $, $) + indexdef FROM crdb_internal.index_usage_statistics AS us JOIN crdb_internal.table_indexes AS ti ON us.index_id = ti.index_id AND us.table_id = ti.descriptor_id @@ -237,9 +238,6 @@ func getTableIndexUsageStats( JOIN pg_catalog.pg_indexes AS pgidxs ON pgidxs.crdb_oid = indexrelid AND indexname = ti.index_name WHERE ti.descriptor_id = $::REGCLASS`, - tree.ConsoleLineWidth, - tree.PrettyAlignAndDeindent, - tree.UpperCase, tableID, ) diff --git a/pkg/server/status_test.go b/pkg/server/status_test.go index a59ea2bf9edd..61a7193afc77 100644 --- a/pkg/server/status_test.go +++ b/pkg/server/status_test.go @@ -1949,33 +1949,17 @@ func TestStatusAPICombinedStatements(t *testing.T) { thirdServerSQL := sqlutils.MakeSQLRunner(testCluster.ServerConn(2)) statements := []struct { - stmt string - formattedStmt string - fingerprint string - formattedFingerprint string + stmt string + fingerprinted string }{ + {stmt: `CREATE DATABASE roachblog`}, + {stmt: `SET database = roachblog`}, + {stmt: `CREATE TABLE posts (id INT8 PRIMARY KEY, body STRING)`}, { - stmt: `CREATE DATABASE roachblog`, - formattedStmt: "CREATE DATABASE roachblog\n", - }, - { - stmt: `SET database = roachblog`, - formattedStmt: "SET database = roachblog\n", - }, - { - stmt: `CREATE TABLE posts (id INT8 PRIMARY KEY, body STRING)`, - formattedStmt: "CREATE TABLE posts (id INT8 PRIMARY KEY, body STRING)\n", - }, - { - stmt: `INSERT INTO posts VALUES (1, 'foo')`, - formattedStmt: "INSERT INTO posts VALUES (1, 'foo')\n", - fingerprint: `INSERT INTO posts VALUES (_, '_')`, - formattedFingerprint: "INSERT INTO posts VALUES (_, '_')\n", - }, - { - stmt: `SELECT * FROM posts`, - formattedStmt: "SELECT * FROM posts\n", + stmt: `INSERT INTO posts VALUES (1, 'foo')`, + fingerprinted: `INSERT INTO posts VALUES (_, '_')`, }, + {stmt: `SELECT * FROM posts`}, } for _, stmt := range statements { @@ -2032,9 +2016,9 @@ func TestStatusAPICombinedStatements(t *testing.T) { var expectedStatements []string for _, stmt := range statements { - var expectedStmt = stmt.formattedStmt - if stmt.fingerprint != "" { - expectedStmt = stmt.formattedFingerprint + var expectedStmt = stmt.stmt + if stmt.fingerprinted != "" { + expectedStmt = stmt.fingerprinted } expectedStatements = append(expectedStatements, expectedStmt) } From c538f2a905965110416a06b0a443db93a16202f5 Mon Sep 17 00:00:00 2001 From: irfan sharif Date: Fri, 21 Jan 2022 20:28:36 -0500 Subject: [PATCH 3/5] execbuilder: deflake TestExecBuild Fixes #74933. This test asserts on physical plans of statements after moving ranges around, i.e. whether distsql execution is "local" or "distributed". This is determined by: - where the distsql planner places processor cores, - which in turn is a function of the span resolver, - which delegates to the embedded distsender, - which operates off of a range cache. The range cache, like the name suggests, can hold stale data. This test moved replicas of indexes around to induce distributed execution, but was not accounting for the caches holding onto stale data. In my test runs every so often I was seeing stale range descriptors from before the relocate trip up the planner to generate a local execution, flaking the test. Fortunately for us, all we need to do is slap on a retry. To repro: # Took under a minute on my gceworker. Helped to trim down the test # file slightly. dev test pkg/sql/opt/exec/execbuilder \ -f 'TestExecBuild/5node' -v --show-logs \ --stress --stress-args '-p 4' Release note: None --- .../testdata/inverted_filter_geospatial_dist | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) diff --git a/pkg/sql/opt/exec/execbuilder/testdata/inverted_filter_geospatial_dist b/pkg/sql/opt/exec/execbuilder/testdata/inverted_filter_geospatial_dist index 4e43844fbd08..9a810f7b543e 100644 --- a/pkg/sql/opt/exec/execbuilder/testdata/inverted_filter_geospatial_dist +++ b/pkg/sql/opt/exec/execbuilder/testdata/inverted_filter_geospatial_dist @@ -1,5 +1,4 @@ # LogicTest: 5node -# cluster-opt: disable-span-configs statement ok CREATE TABLE geo_table( @@ -107,7 +106,7 @@ start_key end_key replicas lease_holder NULL /1152921574000000000 {1} 1 /1152921574000000000 NULL {2} 2 -# Distributed. +# Distributed. TODO(treilly): This claims to be distributed, but it isn't. What gives? query T EXPLAIN (DISTSQL) SELECT k FROM geo_table WHERE ST_Intersects('MULTIPOINT((2.2 2.2), (3.0 3.0))'::geometry, geom) ORDER BY k @@ -176,8 +175,9 @@ start_key end_key replicas lease_holder NULL /1152921574000000000 {2} 2 /1152921574000000000 NULL {2} 2 -# Filtering is placed at node 2. -query T +# Filtering is placed at node 2. We need a retry here to account for possibly +# stale dist sender caches. +query T retry EXPLAIN (DISTSQL) SELECT k FROM geo_table WHERE ST_Intersects('MULTIPOINT((2.2 2.2), (3.0 3.0))'::geometry, geom) ORDER BY k ---- @@ -205,6 +205,9 @@ vectorized: true Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJyUlN9v2jAQx9_3V1j3UtBcsJ3QrX6iP9ItE4UuMG3VjKqM3LqoYGe2mZgq_vcppGtLK9LGD4Y7-3OX71e2b8H9noOE6NvF4CgektZpPJ6MPw_aZBwNopMJuSFnyeicXKO58umPOZKvH6MkIs5f5dqjdTjzrrV3_mUwiS9G8XDSaomOIKIj2pS0gg4jQYe123tSfohG59EkuaRlrUWbjJLTKCHHl-QGKGiT4TBdoAP5HThQEDClUFgzQ-eMLdO3m01xtgLJKOS6WPoyPaUwMxZB3oLP_RxBwtDsm6LbAwoZ-jSfb7atKZilf4CcT68R5MGaPirM6wtPSgMSTDO0XbZVHu796ZfqrnKd4QoonJj5cqGdJDeVbKAwLtIy0VVwrNTqZ6bUijOlVuylCfabMlwBSXVGAkaM_4XWwS4beBMbYv0HrcfsLJ97tGi7fNuL_-vRqrDEaNLnkrhSNXE-tV5uVATvekoxwZRi7KUJCOqsKVaKf6KewmjpJenznT6IJj58Mrm-Ow1i12kobL5I7d-H1rQvdnYPmnS_dz_Y7l3l5dMbyjgLWTXE3S9nvPpzeHR4PxgPn8UPO7fG-_BZvCcfX_S-aL_C87CJ6rGxHm033Nbc5293lu9tlX_h6UjQFUY7fNXbwdZTCphdY_U8ObO0M7ywZrZpU4WjDbdJZOh8tXpQBbGulsoPfAzzWljUw6IWDurhoBYO6-GwFu7Vw71amD2Bp-s3_wIAAP__EGcZAQ== # Filtering is at gateway node since the filter is not distributable. +# +# TODO(treilly): What the text above claims does not square with the figure +# generated below. query T EXPLAIN (DISTSQL) SELECT k FROM geo_table WHERE ST_CoveredBy('MULTIPOINT((2.2 2.2), (3.0 3.0))'::geometry, geom) ORDER BY k @@ -236,7 +239,22 @@ Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJyUlF9v2jwUxu_fT statement ok SET CLUSTER SETTING sql.spatial.experimental_box2d_comparison_operators.enabled = on -query T +query TTTI colnames,rowsort +SELECT start_key, end_key, replicas, lease_holder FROM [SHOW RANGES FROM INDEX geo_table@geom_index] +---- +start_key end_key replicas lease_holder +NULL /1152921574000000000 {2} 2 +/1152921574000000000 NULL {2} 2 + +query ITTTI colnames,rowsort +SELECT range_id, start_key, end_key, replicas, lease_holder FROM [SHOW RANGES FROM TABLE geo_table] +---- +range_id start_key end_key replicas lease_holder +43 NULL NULL {2} 2 + +# We should see a distributed execution (though need to retry to purge possibly +# stale dist sender caches). +query T retry EXPLAIN (DISTSQL) SELECT k FROM geo_table WHERE geom && 'POINT(3.0 3.0)'::geometry ---- @@ -260,7 +278,7 @@ vectorized: true · Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJyUlFFv0zAQx9_5FNa9dJO81U5aQH4qjAyCura0lQDhagr1UaKldrAdFFT1u6MksK2dmq55sHRn_-78_8uXDbhfGQiIvkyGb-IROXsXz-azT8NzMouG0dWc3JHr6fiGrNDc-uR7huTzh2gaVfGayIKx4GWzks5kHI_mZ-ElI-ElO-8I8T4a30Tz6VegoI3CUbJGB-IbcKAQwIJCbs0SnTO2Sm_qQ7EqQTAKqc4LX6UXFJbGIogN-NRnCAJG5sLk3R5QUOiTNKuPbSmYwj9AzicrBNHf0keFeXvheSVwiolC22U75eFe_6BSfptqhSVQuDJZsdZOkDtaWwIUZnlSJboS3kpZ_lBSlpxJWbJjC1ycynAJJNGKhIwY_xOtg0M28FNsiPVvtB7VdZp5tGi7fNeL__tRmVtiNBlwQVylmjifWC9qFeGrvpQsYFIydmwBglqdilXi99RTGBdekAE_6ENwig8fTar_vYbg0GvIbbpO7J-H1nQQHOwentL93v1wt3eTF2QQ7M0f44yzp9_r3pO4I3aG85hpvZ1rHxnOKbrcaIfPmk62XVBAtcLmB-BMYZc4sWZZt2nCcc3VCYXON7v9Joh1s1Vd8DHMW-GgHQ5a4bAdDlvhXjvca4XZHrzYvvgbAAD__-uIzaE= -query T +query T retry EXPLAIN (DISTSQL) SELECT k FROM geo_table WHERE 'POINT(3.0 3.0)'::geometry::box2d && geom ---- @@ -284,7 +302,7 @@ vectorized: true · Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJyUk99v2jAQx9_3V1j3Qiu5wk5gm_zEaFONiUEHSEOaUZXhG4sa7Mx2pkyI_31Ksv6gHaHJg6W7y-fO3698O3C_UhAQLW_GH0YTcnY1mi_mX8bnZB6No8sFuSPXs-lnskFz6-PvKZKvH6NZRDqMM85efu97L-KOEMPpMrgiMmcseFufZcMtUNBG4STeogPxDThQCGBFIbNmjc4ZW6Z31U8jVYBgFBKd5b5MryisjUUQO_CJTxEETMyFybo9oKDQx0la_banYHL_CDkfbxBEf0-fNObNjRel8hnGCm2XHbSHB2MGpaLbRCssgMKlSfOtdoLc0Xup8ywuE10JQymLH0rKgjMpC3bqgIu2DJdAYq1IyIjxP9E6OGYDb2PDSP9G61FdJ6lHi7bLD724r0dFZonRZMAFcaVq4nxsvahUhO_6UrKAScnYqQMIatUWK8U_U09hmntBBvyoD0EbHz6ZRP97DcGx15DZZBvbP4-j6SA4Oj1sM_3B_fBwdp0XpDOcLs9CEtKQhOcd8d_tGwSv8KR3cKsTuzdDlxnt8FXLx_YrCqg2WO-3M7ld440162pMHU4rrkoodL6u9utgpOtSecGnMG-Eg2Y4aITDZjhshHvNcK8RZs_g1f7N3wAAAP__-TrK3A== -query T +query T retry EXPLAIN (DISTSQL) SELECT k FROM geo_table WHERE 'LINESTRING(1.0 1.0, 5.0 5.0)'::geometry ~ geom ---- @@ -308,7 +326,7 @@ vectorized: true · Diagram: https://cockroachdb.github.io/distsqlplan/decode.html#eJyUlGFv2jwQx98_n8K6N7SSC3YS9Eh-xdaFLhOFLiBt04yqjNxY1GBntpkyIfbZpySjBSRCsRRHd_bPl_9fvmzA_sxBQPj5YfQmGpOrd9F0Nv04uibTcBTezsgTGcaTe7JE_eiSbzmST-_DOCSdUTQOp7M4Gt9d8S4jvMso6XdZ9Vx3hLgLJ_fhLP5C_lToCigoneI4WaEF8RU4UPBgTqEweoHWalOlN_WmKC1BMAqZKtauSs8pLLRBEBtwmcsRBIz1jS56AVBI0SVZXm_bUtBr9wJZlywRRH9L9w7m7QfPKo0xJimaHjs4Hp4tGFSKHjOVYgkUbnW-XikryBPdSZ0WSZXoSXgrZfk9lbLkTMqSnZvg5lKGSyCJSonvEe1-oLFwygZ-iQ2R-oXGYTrMcocGTY8ferFbD8vCEK3IgAtiK9XEusQ4Uavw_-9LyTwmJWPnJiCo0kuxSvyRegqTtRNkwE_64F3iwwedqX-3wTt1GwqTrRLz-6U0HXgnq_uXVH923z-s3eQF6TDOPNaM3XtvDJk_bIt5ELDjuCMOenfgvcLR4EDTmc6N0RZaWXxV67LtnAKmS2z-DlavzQIfjF7UZZpwUnN1IkXrmtV-E0SqWao-cB_mrbDXDnutsN8O-61w0A4HrTA7gufb__4GAAD__zWM0xs= -query T +query T retry EXPLAIN (DISTSQL) SELECT k FROM geo_table WHERE geom ~ 'LINESTRING(1.0 1.0, 5.0 5.0)'::geometry::box2d ---- From 83f584d6c4c3cd2f41875cb1481631e1b38599ac Mon Sep 17 00:00:00 2001 From: irfan sharif Date: Mon, 24 Jan 2022 11:22:25 -0500 Subject: [PATCH 4/5] logictest: (speculatively) de-flake distsql_enum We opted this test out of using the span configs infra in #75281 after observing a CI flake. Following the analysis in the last commit, it may have been due to stale distsender caches affecting the physical plan generated. This fix is speculative because I was unable to repro the original flake under stress. Attempt (successful after 1000s of runs on GCE worker over 20+ minutes): dev test pkg/sql/logictest \ -f 'TestLogic/^5node-disk$/distsql_enum' \ --show-logs -v --stress --stress-args '-p 4' -- --test_arg -show-sql Release note: None --- pkg/sql/logictest/testdata/logic_test/distsql_enum | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pkg/sql/logictest/testdata/logic_test/distsql_enum b/pkg/sql/logictest/testdata/logic_test/distsql_enum index 05a37b0a5e3e..9b3523f9a13a 100644 --- a/pkg/sql/logictest/testdata/logic_test/distsql_enum +++ b/pkg/sql/logictest/testdata/logic_test/distsql_enum @@ -1,5 +1,4 @@ # LogicTest: 5node-default-configs !5node-metadata -# cluster-opt: disable-span-configs # Regression test for nested tuple enum hydration (#74189) statement ok @@ -66,7 +65,7 @@ ALTER TABLE t2 INJECT STATISTICS '[ } ]' -query T nodeidx=1 +query T nodeidx=1 retry EXPLAIN (VEC) SELECT x from t1 WHERE EXISTS (SELECT x FROM t2 WHERE t1.x=t2.x AND t2.y='hello') ---- From 39604d849be2cf1e7a43cfe08062d9d679baa34b Mon Sep 17 00:00:00 2001 From: Thomas Hardy Date: Mon, 24 Jan 2022 11:13:23 -0500 Subject: [PATCH 5/5] ui: removed formatting to statements on the details pages Previously, statements displayed on the statement/transaction/index details pages were formatted (formatting was added to allow for better readability of statements on these detail pages). However, statements queries from the frontend were noticeably slower due to this implementation. This change reverts the changes to statement formatting (updates the fixtures to show the non-formatted statements), but keeps the change that uses statement ID as an identifier in the URL instead of the raw statement. Release note (ui change): removed formatting to statements on the statement, transaction and index details pages, change to replace raw statement with statement ID in the URL remained. --- pkg/ccl/serverccl/statusccl/BUILD.bazel | 1 - .../src/sql/sqlhighlight.module.scss | 1 + .../statementDetails.fixture.ts | 9 +-- .../statementsPage/statementsPage.fixture.ts | 78 ++----------------- 4 files changed, 10 insertions(+), 79 deletions(-) diff --git a/pkg/ccl/serverccl/statusccl/BUILD.bazel b/pkg/ccl/serverccl/statusccl/BUILD.bazel index afc78d0bb431..adafbeec17c6 100644 --- a/pkg/ccl/serverccl/statusccl/BUILD.bazel +++ b/pkg/ccl/serverccl/statusccl/BUILD.bazel @@ -43,7 +43,6 @@ go_test( "//pkg/spanconfig", "//pkg/sql/catalog/catconstants", "//pkg/sql/catalog/descpb", - "//pkg/sql/sem/tree", "//pkg/sql/sqlstats", "//pkg/sql/tests", "//pkg/testutils", diff --git a/pkg/ui/workspaces/cluster-ui/src/sql/sqlhighlight.module.scss b/pkg/ui/workspaces/cluster-ui/src/sql/sqlhighlight.module.scss index 4d93d3edee08..820715c912d6 100644 --- a/pkg/ui/workspaces/cluster-ui/src/sql/sqlhighlight.module.scss +++ b/pkg/ui/workspaces/cluster-ui/src/sql/sqlhighlight.module.scss @@ -27,6 +27,7 @@ font-family: SFMono-Semibold; font-size: 14px; line-height: 1.57; + white-space: pre-line; word-wrap: break-word; span { diff --git a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.fixture.ts b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.fixture.ts index 55713277faf2..3e6705eb1e20 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.fixture.ts +++ b/pkg/ui/workspaces/cluster-ui/src/statementDetails/statementDetails.fixture.ts @@ -151,14 +151,7 @@ export const getStatementDetailsPropsFixture = (): StatementDetailsProps => ({ }, statement: { statement: - "CREATE TABLE IF NOT EXISTS promo_codes (\n" + - " code VARCHAR NOT NULL,\n" + - " description VARCHAR NULL,\n" + - " creation_time TIMESTAMP NULL,\n" + - " expiration_time TIMESTAMP NULL,\n" + - " rules JSONB NULL,\n" + - " PRIMARY KEY (code ASC)\n" + - ")", + "CREATE TABLE IF NOT EXISTS promo_codes (code VARCHAR NOT NULL, description VARCHAR NULL, creation_time TIMESTAMP NULL, expiration_time TIMESTAMP NULL, rules JSONB NULL, PRIMARY KEY (code ASC))", stats: statementStats, database: "defaultdb", byNode: [ diff --git a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts index 9d87bd91dbcf..dae84733ee78 100644 --- a/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts +++ b/pkg/ui/workspaces/cluster-ui/src/statementsPage/statementsPage.fixture.ts @@ -288,11 +288,7 @@ const statementsPagePropsFixture: StatementsPageProps = { { aggregatedFingerprintID: "1253500548539870016", label: - "SELECT IFNULL(a, b)\n" + - " FROM (\n" + - " SELECT (SELECT code FROM promo_codes WHERE code > $1 ORDER BY code LIMIT _) AS a,\n" + - " (SELECT code FROM promo_codes ORDER BY code LIMIT _) AS b\n" + - " )", + "SELECT IFNULL(a, b) FROM (SELECT (SELECT code FROM promo_codes WHERE code > $1 ORDER BY code LIMIT _) AS a, (SELECT code FROM promo_codes ORDER BY code LIMIT _) AS b)", summary: "SELECT IFNULL(a, b) FROM (SELECT)", aggregatedTs, aggregationInterval, @@ -315,11 +311,7 @@ const statementsPagePropsFixture: StatementsPageProps = { { aggregatedFingerprintID: "13649565517143827225", label: - "SELECT IFNULL(a, b)\n" + - " FROM (\n" + - " SELECT (SELECT id FROM users WHERE city = $1 AND id > $2 ORDER BY id LIMIT _) AS a,\n" + - " (SELECT id FROM users WHERE city = $1 ORDER BY id LIMIT _) AS b\n" + - " )", + "SELECT IFNULL(a, b) FROM (SELECT (SELECT id FROM users WHERE (city = $1) AND (id > $2) ORDER BY id LIMIT _) AS a, (SELECT id FROM users WHERE city = $1 ORDER BY id LIMIT _) AS b)", summary: "SELECT IFNULL(a, b) FROM (SELECT)", aggregatedTs, aggregationInterval, @@ -470,14 +462,7 @@ const statementsPagePropsFixture: StatementsPageProps = { { aggregatedFingerprintID: '11195381626529102926', label: - "CREATE TABLE IF NOT EXISTS promo_codes (\n" + - " code VARCHAR NOT NULL,\n" + - " description VARCHAR NULL,\n" + - " creation_time TIMESTAMP NULL,\n" + - " expiration_time TIMESTAMP NULL,\n" + - " rules JSONB NULL,\n" + - " PRIMARY KEY (code ASC)\n" + - " )", + "CREATE TABLE IF NOT EXISTS promo_codes (code VARCHAR NOT NULL, description VARCHAR NULL, creation_time TIMESTAMP NULL, expiration_time TIMESTAMP NULL, rules JSONB NULL, PRIMARY KEY (code ASC))", summary: "CREATE TABLE IF NOT EXISTS promo_codes (code VARCHAR NOT NULL, description VARCHAR NULL, creation_time TIMESTAMP NULL, expiration_time TIMESTAMP NULL, rules JSONB NULL, PRIMARY KEY (code ASC))", aggregatedTs, @@ -525,14 +510,7 @@ const statementsPagePropsFixture: StatementsPageProps = { { aggregatedFingerprintID: '13217779306501326587', label: - 'CREATE TABLE IF NOT EXISTS user_promo_codes (\n' + - ' city VARCHAR NOT NULL,\n' + - ' user_id UUID NOT NULL,\n' + - ' code VARCHAR NOT NULL,\n' + - ' "timestamp" TIMESTAMP NULL,\n' + - ' usage_count INT8 NULL,\n' + - ' PRIMARY KEY (city ASC, user_id ASC, code ASC)\n' + - ' )', + 'CREATE TABLE IF NOT EXISTS user_promo_codes (city VARCHAR NOT NULL, user_id UUID NOT NULL, code VARCHAR NOT NULL, "timestamp" TIMESTAMP NULL, usage_count INT8 NULL, PRIMARY KEY (city ASC, user_id ASC, code ASC))', summary: 'CREATE TABLE IF NOT EXISTS user_promo_codes (city VARCHAR NOT NULL, user_id UUID NOT NULL, code VARCHAR NOT NULL, "timestamp" TIMESTAMP NULL, usage_count INT8 NULL, PRIMARY KEY (city ASC, user_id ASC, code ASC))', aggregatedTs, @@ -591,22 +569,7 @@ const statementsPagePropsFixture: StatementsPageProps = { { aggregatedFingerprintID: '2695725667586429780', label: - "CREATE TABLE IF NOT EXISTS rides (\n" + - " id UUID NOT NULL,\n" + - " city VARCHAR NOT NULL,\n" + - " vehicle_city VARCHAR NULL,\n" + - " rider_id UUID NULL,\n" + - " vehicle_id UUID NULL,\n" + - " start_address VARCHAR NULL,\n" + - " end_address VARCHAR NULL,\n" + - " start_time TIMESTAMP NULL,\n" + - " end_time TIMESTAMP NULL,\n" + - " revenue DECIMAL(10,2) NULL,\n" + - " PRIMARY KEY (city ASC, id ASC),\n" + - " INDEX rides_auto_index_fk_city_ref_users (city ASC, rider_id ASC),\n" + - " INDEX rides_auto_index_fk_vehicle_city_ref_vehicles (vehicle_city ASC, vehicle_id ASC),\n" + - " CONSTRAINT check_vehicle_city_city CHECK (vehicle_city = city)\n" + - " )", + "CREATE TABLE IF NOT EXISTS rides (id UUID NOT NULL, city VARCHAR NOT NULL, vehicle_city VARCHAR NULL, rider_id UUID NULL, vehicle_id UUID NULL, start_address VARCHAR NULL, end_address VARCHAR NULL, start_time TIMESTAMP NULL, end_time TIMESTAMP NULL, revenue DECIMAL(10,2) NULL, PRIMARY KEY (city ASC, id ASC), INDEX rides_auto_index_fk_city_ref_users (city ASC, rider_id ASC), INDEX rides_auto_index_fk_vehicle_city_ref_vehicles (vehicle_city ASC, vehicle_id ASC), CONSTRAINT check_vehicle_city_city CHECK (vehicle_city = city))", summary: "CREATE TABLE IF NOT EXISTS rides (id UUID NOT NULL, city VARCHAR NOT NULL, vehicle_city VARCHAR NULL, rider_id UUID NULL, vehicle_id UUID NULL, start_address VARCHAR NULL, end_address VARCHAR NULL, start_time TIMESTAMP NULL, end_time TIMESTAMP NULL, revenue DECIMAL(10,2) NULL, PRIMARY KEY (city ASC, id ASC), INDEX rides_auto_index_fk_city_ref_users (city ASC, rider_id ASC), INDEX rides_auto_index_fk_vehicle_city_ref_vehicles (vehicle_city ASC, vehicle_id ASC), CONSTRAINT check_vehicle_city_city CHECK (vehicle_city = city))", aggregatedTs, @@ -619,18 +582,7 @@ const statementsPagePropsFixture: StatementsPageProps = { { aggregatedFingerprintID: '6754865160812330169', label: - "CREATE TABLE IF NOT EXISTS vehicles (\n" + - " id UUID NOT NULL,\n" + - " city VARCHAR NOT NULL,\n" + - " type VARCHAR NULL,\n" + - " owner_id UUID NULL,\n" + - " creation_time TIMESTAMP NULL,\n" + - " status VARCHAR NULL,\n" + - " current_location VARCHAR NULL,\n" + - " ext JSONB NULL,\n" + - " PRIMARY KEY (city ASC, id ASC),\n" + - " INDEX vehicles_auto_index_fk_city_ref_users (city ASC, owner_id ASC)\n" + - " )", + "CREATE TABLE IF NOT EXISTS vehicles (id UUID NOT NULL, city VARCHAR NOT NULL, type VARCHAR NULL, owner_id UUID NULL, creation_time TIMESTAMP NULL, status VARCHAR NULL, current_location VARCHAR NULL, ext JSONB NULL, PRIMARY KEY (city ASC, id ASC), INDEX vehicles_auto_index_fk_city_ref_users (city ASC, owner_id ASC))", summary: "CREATE TABLE IF NOT EXISTS vehicles (id UUID NOT NULL, city VARCHAR NOT NULL, type VARCHAR NULL, owner_id UUID NULL, creation_time TIMESTAMP NULL, status VARCHAR NULL, current_location VARCHAR NULL, ext JSONB NULL, PRIMARY KEY (city ASC, id ASC), INDEX vehicles_auto_index_fk_city_ref_users (city ASC, owner_id ASC))", aggregatedTs, @@ -676,14 +628,7 @@ const statementsPagePropsFixture: StatementsPageProps = { { aggregatedFingerprintID: '8695470234690735168', label: - "CREATE TABLE IF NOT EXISTS users (\n" + - " id UUID NOT NULL,\n" + - " city VARCHAR NOT NULL,\n" + - " name VARCHAR NULL,\n" + - " address VARCHAR NULL,\n" + - " credit_card VARCHAR NULL,\n" + - " PRIMARY KEY (city ASC, id ASC)\n" + - " )", + "CREATE TABLE IF NOT EXISTS users (id UUID NOT NULL, city VARCHAR NOT NULL, name VARCHAR NULL, address VARCHAR NULL, credit_card VARCHAR NULL, PRIMARY KEY (city ASC, id ASC))", summary: "CREATE TABLE IF NOT EXISTS users (id UUID NOT NULL, city VARCHAR NOT NULL, name VARCHAR NULL, address VARCHAR NULL, credit_card VARCHAR NULL, PRIMARY KEY (city ASC, id ASC))", aggregatedTs, @@ -696,14 +641,7 @@ const statementsPagePropsFixture: StatementsPageProps = { { aggregatedFingerprintID: '9261848985398568228', label: - 'CREATE TABLE IF NOT EXISTS vehicle_location_histories (\n' + - ' city VARCHAR NOT NULL,\n' + - ' ride_id UUID NOT NULL,\n' + - ' "timestamp" TIMESTAMP NOT NULL,\n' + - ' lat FLOAT8 NULL,\n' + - ' long FLOAT8 NULL,\n' + - ' PRIMARY KEY (city ASC, ride_id ASC, "timestamp" ASC)\n' + - ' )', + 'CREATE TABLE IF NOT EXISTS vehicle_location_histories (city VARCHAR NOT NULL, ride_id UUID NOT NULL, "timestamp" TIMESTAMP NOT NULL, lat FLOAT8 NULL, long FLOAT8 NULL, PRIMARY KEY (city ASC, ride_id ASC, "timestamp" ASC))', summary: 'CREATE TABLE IF NOT EXISTS vehicle_location_histories (city VARCHAR NOT NULL, ride_id UUID NOT NULL, "timestamp" TIMESTAMP NOT NULL, lat FLOAT8 NULL, long FLOAT8 NULL, PRIMARY KEY (city ASC, ride_id ASC, "timestamp" ASC))', aggregatedTs,