Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
85464: ui: add Transaction and Statement fingerprint id r=maryliag a=j82w

This adds the Transaction and Statement fingerprint id
to SQL Activity Statements overview page. The columns
are hidden by default.

closes #78509

<img width="1337" alt="Screen Shot 2022-08-03 at 7 14 21 AM" src="https://user-images.githubusercontent.com/8868107/182594974-73bea276-0fb0-4cd4-a106-60e31962b1b9.png">

<img width="1341" alt="Screen Shot 2022-08-03 at 7 21 43 AM" src="https://user-images.githubusercontent.com/8868107/182596064-7a02e7da-3e07-4616-9be9-1a807b87b33d.png">

Release justification: Category 2: Bug fixes and
low-risk updates to new functionality

Release note (ui change): Adds Transaction and Statement fingerprint
id to correlating SQL Activity overview pages. New columns are
hidden by default.

86483: kvserver: decrease trace verbosity for change replicas db operations r=AlexTalks a=AlexTalks

While we introduced logging of traces from replicate queue processing
in #86007, it was noted that when collecting traces with verbose recording
enabled, a significant fraction of these traces were from low-level
database operations, particularly during the transaction run in
`execChangeReplicasTxn(..)`. This function performs necessary
bookkeeping of range descriptors when replica movement occurs, however
the high verbosity on the low-level database operations within the
transaction is not necessary when examining traces from higher-level
operations like replicate queue processing. Hence, this change
creates child tracing spans within `execChangeReplicasTxn` which
can be filtered out prior to logging the traces in the replicate queue
processing loop. This decreases the size of the logged trace by a factor
of 10, allowing the resulting log message to be much more succinct and
focused.

Release justification: Low-risk, high benefit observability change.
Release note: None

86779: ui: statement insight details page v1 r=ericharmeling a=ericharmeling

This PR adds the Statement Insights Details page to the DB Console.

https://www.loom.com/share/e9c7be3eb5da47b784b119b6fc9bbeb9

Release justification: low-risk updates to new functionality

Release note (ui change): Added statement insight details page to DB Console.

86829: sql: include regions information into the sampled query telemetry r=yuzefovich a=yuzefovich

**execstats: remove unused regions field**

This field was never actually populated nor used since it was introduced
in 9f716a7.

Release justification: low-risk cleanup.

Release note: None

**sql: include regions information into the sampled query telemetry**

This commit adds the regions (where SQL processors where planned for the
query) to the sampled query telemetry. This required a couple of minor
changes to derive the regions information stored in the instrumentation
helper earlier (before the logging takes place).

Fixes: #85427.

Release justification: low-risk improvement.

Release note (sql change): The structured payloads used for telemetry
logs now include the new `Regions` field which indicates the regions of
the nodes where SQL processing ran for the query.

86951: flowinfra: improve BenchmarkFlowSetup r=yuzefovich a=yuzefovich

This commit improves `BenchmarkFlowSetup` by pulling out parsing of the
statement from the hot path as well as more precise usage of the timer
(to exclude the construction of the internal planner). This way the
benchmark is better focused on its goal - the performance of the
planning and execution.

Release justification: test-only change.

Release note: None

86964: rowexec: close all ValueGenerators in the project set processor r=yuzefovich a=yuzefovich

Previously, we forgot to close the value generators when a new input row
is read by the project set processor which could lead to leaking of the
resources. This is now fixed. Most of the value generators don't need to
release any resources, a few need to close their memory account
(previously this would result in a sentry report, but no actual leak
would occur in production builds), the only concerning one is
`crdb_internal.payloads_for_trace` where we would not close the
`InternalRows` iterator. But that seems like an internal debugging tool,
so most likely the users weren't impacted.

Fixes: #85418.

Release justification: bug fix.

Release note: None (It seems like in most cases the users would not see
the impact.)

86965: pgcode: use custom error codes for home region errors r=msirek a=rafiss

The XC error class is reserved for Cockroach-specific errors to avoid
conflicting with any future error codes that Postgres might add.

Release note: None

Release justification: low risk change to new functionality.

Co-authored-by: j82w <jwilley@cockroachlabs.com>
Co-authored-by: Alex Sarkesian <sarkesian@cockroachlabs.com>
Co-authored-by: Xin Hao Zhang <xzhang@cockroachlabs.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
  • Loading branch information
6 people committed Aug 26, 2022
8 parents 8e3b269 + 08f810f + 2aa6204 + cad9461 + 73e9ff0 + c92d3c4 + 66bf21c + 60b59e4 commit 50256c3
Show file tree
Hide file tree
Showing 66 changed files with 1,108 additions and 356 deletions.
1 change: 1 addition & 0 deletions docs/generated/eventlog.md
Original file line number Diff line number Diff line change
Expand Up @@ -2604,6 +2604,7 @@ contains common SQL event/execution details.
| `ApplyJoinCount` | The number of apply joins in the query plan. | no |
| `ZigZagJoinCount` | The number of zig zag joins in the query plan. | no |
| `ContentionNanos` | The duration of time in nanoseconds that the query experienced contention. | no |
| `Regions` | The regions of the nodes where SQL processors ran. | no |


#### Common fields
Expand Down
2 changes: 2 additions & 0 deletions pkg/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -1921,6 +1921,7 @@ GO_TARGETS = [
"//pkg/util/log/logcrash:logcrash_test",
"//pkg/util/log/logflags:logflags",
"//pkg/util/log/logpb:logpb",
"//pkg/util/log/logtestutils:logtestutils",
"//pkg/util/log/severity:severity",
"//pkg/util/log/testshout:testshout_test",
"//pkg/util/log:log",
Expand Down Expand Up @@ -2875,6 +2876,7 @@ GET_X_DATA_TARGETS = [
"//pkg/util/log/logcrash:get_x_data",
"//pkg/util/log/logflags:get_x_data",
"//pkg/util/log/logpb:get_x_data",
"//pkg/util/log/logtestutils:get_x_data",
"//pkg/util/log/severity:get_x_data",
"//pkg/util/log/testshout:get_x_data",
"//pkg/util/memzipper:get_x_data",
Expand Down
5 changes: 5 additions & 0 deletions pkg/ccl/telemetryccl/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,15 @@ go_test(
name = "telemetryccl_test",
srcs = [
"main_test.go",
"telemetry_logging_test.go",
"telemetry_test.go",
],
data = glob(["testdata/**"]),
shard_count = 16,
deps = [
"//pkg/base",
"//pkg/ccl",
"//pkg/ccl/multiregionccl/multiregionccltestutils",
"//pkg/ccl/utilccl",
"//pkg/roachpb",
"//pkg/security/securityassets",
Expand All @@ -20,10 +22,13 @@ go_test(
"//pkg/sql/sqltestutils",
"//pkg/testutils/serverutils",
"//pkg/testutils/skip",
"//pkg/testutils/sqlutils",
"//pkg/testutils/testcluster",
"//pkg/util/leaktest",
"//pkg/util/log",
"//pkg/util/log/logtestutils",
"//pkg/util/randutil",
"@com_github_cockroachdb_errors//:errors",
],
)

Expand Down
126 changes: 126 additions & 0 deletions pkg/ccl/telemetryccl/telemetry_logging_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
// Copyright 2022 The Cockroach Authors.
//
// Licensed as a CockroachDB Enterprise file under the Cockroach Community
// License (the "License"); you may not use this file except in compliance with
// the License. You may obtain a copy of the License at
//
// https://github.com/cockroachdb/cockroach/blob/master/licenses/CCL.txt

package telemetryccl

import (
"math"
"regexp"
"strings"
"testing"

"github.com/cockroachdb/cockroach/pkg/base"
"github.com/cockroachdb/cockroach/pkg/ccl/multiregionccl/multiregionccltestutils"
"github.com/cockroachdb/cockroach/pkg/testutils/sqlutils"
"github.com/cockroachdb/cockroach/pkg/util/leaktest"
"github.com/cockroachdb/cockroach/pkg/util/log"
"github.com/cockroachdb/cockroach/pkg/util/log/logtestutils"
"github.com/cockroachdb/errors"
)

func TestTelemetryLogRegions(t *testing.T) {
defer leaktest.AfterTest(t)()
sc := log.ScopeWithoutShowLogs(t)
defer sc.Close(t)

cleanup := logtestutils.InstallTelemetryLogFileSink(sc, t)
defer cleanup()

_, db, cleanup := multiregionccltestutils.TestingCreateMultiRegionCluster(
t, 3 /* numServers */, base.TestingKnobs{},
multiregionccltestutils.WithReplicationMode(base.ReplicationManual),
)
defer cleanup()
sqlDB := sqlutils.MakeSQLRunner(db)

// Create three tables, with each table touching one, two, and three
// regions, respectively.
sqlDB.Exec(t, `CREATE TABLE one_region (k INT PRIMARY KEY)`)
sqlDB.Exec(t, `INSERT INTO one_region SELECT generate_series(1, 1)`)
sqlDB.Exec(t, `ALTER TABLE one_region SPLIT AT SELECT generate_series(1, 1)`)
sqlDB.Exec(t, "ALTER TABLE one_region EXPERIMENTAL_RELOCATE VALUES (ARRAY[1], 1)")
sqlDB.Exec(t, `CREATE TABLE two_regions (k INT PRIMARY KEY)`)
sqlDB.Exec(t, `INSERT INTO two_regions SELECT generate_series(1, 2)`)
sqlDB.Exec(t, `ALTER TABLE two_regions SPLIT AT SELECT generate_series(1, 2)`)
sqlDB.Exec(t, "ALTER TABLE two_regions EXPERIMENTAL_RELOCATE VALUES (ARRAY[1], 1), (ARRAY[2], 2)")
sqlDB.Exec(t, `CREATE TABLE three_regions (k INT PRIMARY KEY)`)
sqlDB.Exec(t, `INSERT INTO three_regions SELECT generate_series(1, 3)`)
sqlDB.Exec(t, `ALTER TABLE three_regions SPLIT AT SELECT generate_series(1, 3)`)
sqlDB.Exec(t, "ALTER TABLE three_regions EXPERIMENTAL_RELOCATE VALUES (ARRAY[1], 1), (ARRAY[2], 2), (ARRAY[3], 3)")

// Enable the telemetry logging and increase the sampling frequency so that
// all statements are captured.
sqlDB.Exec(t, `SET CLUSTER SETTING sql.telemetry.query_sampling.enabled = true;`)
sqlDB.Exec(t, `SET CLUSTER SETTING sql.telemetry.query_sampling.max_event_frequency = 1000000`)

testData := []struct {
name string
query string
expectedLogStatement string
expectedRegions []string
}{
{
name: "one-region",
query: "SELECT * FROM one_region",
expectedLogStatement: `SELECT * FROM \"\".\"\".one_region`,
expectedRegions: []string{"us-east1"},
},
{
name: "two-regions",
query: "SELECT * FROM two_regions",
expectedLogStatement: `SELECT * FROM \"\".\"\".two_regions`,
expectedRegions: []string{"us-east1", "us-east2"},
},
{
name: "three-regions",
query: "SELECT * FROM three_regions",
expectedLogStatement: `SELECT * FROM \"\".\"\".three_regions`,
expectedRegions: []string{"us-east1", "us-east2", "us-east3"},
},
}

for _, tc := range testData {
sqlDB.Exec(t, tc.query)
}

log.Flush()

entries, err := log.FetchEntriesFromFiles(
0,
math.MaxInt64,
10000,
regexp.MustCompile(`"EventType":"sampled_query"`),
log.WithMarkedSensitiveData,
)

if err != nil {
t.Fatal(err)
}

if len(entries) == 0 {
t.Fatal(errors.Newf("no entries found"))
}

for _, tc := range testData {
var logEntriesCount int
for i := len(entries) - 1; i >= 0; i-- {
e := entries[i]
if strings.Contains(e.Message, tc.expectedLogStatement) {
logEntriesCount++
for _, region := range tc.expectedRegions {
if !strings.Contains(e.Message, region) {
t.Errorf("didn't find region %q in the log entry %s", region, e.Message)
}
}
}
}
if logEntriesCount != 1 {
t.Errorf("expected to find a single entry for %q: %v", tc.name, entries)
}
}
}
5 changes: 4 additions & 1 deletion pkg/kv/kvserver/replica.go
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ const (
// the key space being written is starting out empty.
optimizePutThreshold = 10

// Transaction names used for range changes.
// Transaction names and operations used for range changes.
// Note that those names are used by tests to perform request filtering
// in absence of better criteria. If names are changed, tests should be
// updated accordingly to avoid flakiness.
Expand All @@ -77,6 +77,9 @@ const (
splitTxnName = "split"
mergeTxnName = "merge"

replicaChangeTxnGetDescOpName = "change-replica-get-desc"
replicaChangeTxnUpdateDescOpName = "change-replica-update-desc"

defaultReplicaRaftMuWarnThreshold = 500 * time.Millisecond
)

Expand Down
Loading

0 comments on commit 50256c3

Please sign in to comment.