Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
61406: sql: validate against array type usages when dropping enum values r=ajwerner a=arulajmani

Previously, when dropping an enum value, we weren't validating if the
enum value was used in a column of array type. This patch fixes the bug.

Fixes #60004

Release justification: bug fix to new functionality
Release note: None

61650: roachtest: exclude `lost+found` directory r=knz a=rickystewart

This directory is created by the filesystem and unowned file chunks are
put there by `fsck`. The directory and its contents aren't readable by
anyone except `root`, so this can cause the `du -c /mnt/data1` that
`roachtest` performs to fail -- add an `--exclude` to handle this.

We already ignore this directory in other contexts (for example, see
`pkg/storage/mvcc.go`).

Fixes #53663.

Release justification: Non-production code change
Release note: None

61788: importccl: unskip userfile benchmark r=pbardea a=adityamaru

I've run this ~20 times and it averages ~13s to run. I suspect the fixes to linked issues mentioned in #59126 might have mitigated this. Going to unskip due to lack of reproducibility.

Fixes: #59126

Release note: None

61828: contention: store contention events on non-SQL keys r=yuzefovich a=yuzefovich

Previously, whenever we tried to add a contention event on a non-SQL
key, it would encounter an error during decoding tableID/indexID pair,
and the event was dropped. This commit extends the contention registry
to additionally store information about contention on non-SQL keys. That
information is stored in two levels:
- on the top level, all `SingleNonSQLKeyContention` objects are ordered
  by their keys
- on the bottom level, all `SingleTxnContention` objects are ordered by the
  number of times that transaction was observed to contend with other
  transactions.

`SingleTxnContention` protobuf message is moved out of
`SingleKeyContention` and is reused for non-SQL keys. This commit also
updates the status server API response. I assume that no changes are
needed with regards to backwards compatibility since the original
version was merged just a few weeks ago, and we haven't had a beta
released since then.

Fixes: #60669.

Release note (sql change): CockroachDB now also stores the information
about contention on non-SQL keys.

61862: cliccl: add `load show backups` to display backup collection r=pbardea a=Elliebababa

Previously, users can list backups created by `BACKUP INTO`
with `SHOW BACKUP IN`in a sql session. But this listing task
can be also done offline without a running cluster.

This PR updates `load show` with `backups` subcommand, 
which allows users to list backups in a backup collection 
created by `BACKUP INTO`. 
With the same purpose as other `load show` subcommands, 
this update allows users to list backups without running 
`SHOW BACKUP IN` in a sql session.

see #61131 #61829 to checkout other `load show` subcommand.

Release note (cli change): Add `load show backups` to
display backup collection. Previously, users can list backups
created by `BACKUP INTO` via `SHOW BACKUP IN`in a sql
session. But this listing task can be also done offline without a
running cluster. Now, users are able to list backups in a collection
with `cockroach load show backups <collection_url>.

61877: bench/ddl_analysis: fix test for real r=ajwerner a=ajwerner

See individual commits. Last is the critical one. 

Fixes #61856.

61937: colexec: make vectorized stats concurrency safe r=yuzefovich a=yuzefovich

**colflow: clean up vectorized stats for rowexec processors**

Previously, the wrapped row-execution KV reading processors were
implementing `execinfra.KVReader` interface, but they were never used as
such, only the ColBatchScans would get used to retrieve the KV stats.
This is the case because the row-execution processors report their
execution stats themselves, and we don't want to duplicate that info.
This commit moves `KVReader` interface into `colexecop` package and now
only the ColBatchScans implement it. This allowed for some cleanup
around the vectorized stats code, but the main reason for performing
this change is that the contract of the interface will be modified by
the follow-up commit to mention the safety under concurrent usage, and
I didn't want to change the row-execution processors for that since the
relevant methods never get called anyway.

Additionally, this commit begins emitting of rows-read and bytes-read by
the zigzagJoiners and invertedJoiners to complete the metrics picture.

Release note: None

**colexec: make vectorized stats concurrency safe**

Previously, the collection of vectorized stats was not synchronized with
the operators themselves. Namely, it was possible to call methods like
`GetBytesRead` on the ColBatchScans and Inboxes from a different
goroutine (the root materializer or the outbox) than from the main
goroutine of the operator. This is now fixed by putting mutexes in place
and updating `colexecop.KVReader` interface to require concurrency-safe
implementations.

Fixes: #61899.

Release note: None

Co-authored-by: arulajmani <arulajmani@gmail.com>
Co-authored-by: Ricky Stewart <ricky@cockroachlabs.com>
Co-authored-by: Aditya Maru <adityamaru@gmail.com>
Co-authored-by: Yahor Yuzefovich <yahor@cockroachlabs.com>
Co-authored-by: elliebababa <ellie24.huang@gmail.com>
Co-authored-by: Andrew Werner <awerner32@gmail.com>
  • Loading branch information
7 people committed Mar 15, 2021
8 parents 2045895 + fe888db + eebeeb1 + 32c6608 + 2041f62 + 5d3f584 + d9ab451 + b2a05a9 commit ae3a358
Show file tree
Hide file tree
Showing 48 changed files with 2,703 additions and 1,274 deletions.
52 changes: 34 additions & 18 deletions docs/generated/http/full.md
Original file line number Diff line number Diff line change
Expand Up @@ -1410,14 +1410,22 @@ Response returned by target query's gateway node.
ListContentionEvents retrieves the contention events across the entire
cluster.

On the highest level, all IndexContentionEvents objects are ordered
according to their importance (as defined by the number of contention
events within each object).
On the middle level, all SingleKeyContention objects are ordered by their
keys lexicographically.
On the lowest level, all SingleTxnContention objects are ordered by the
number of times that transaction was observed to contend with other
transactions.
For SQL keys the following orderings are maintained:
- on the highest level, all IndexContentionEvents objects are ordered
according to their importance (as defined by the number of contention
events within each object).
- on the middle level, all SingleKeyContention objects are ordered by their
keys lexicographically.
- on the lowest level, all SingleTxnContention objects are ordered by the
number of times that transaction was observed to contend with other
transactions.

For non-SQL keys the following orderings are maintained:
- on the top level, all SingleNonSQLKeyContention objects are ordered
by their keys lexicographically.
- on the bottom level, all SingleTxnContention objects are ordered by the
number of times that transaction was observed to contend with other
transactions.

Support status: [reserved](#support-status)

Expand Down Expand Up @@ -1445,7 +1453,7 @@ Response object for ListContentionEvents and ListLocalContentionEvents.

| Field | Type | Label | Description | Support status |
| ----- | ---- | ----- | ----------- | -------------- |
| events | [cockroach.sql.contentionpb.IndexContentionEvents](#cockroach.server.serverpb.ListContentionEventsResponse-cockroach.sql.contentionpb.IndexContentionEvents) | repeated | A list of contention events on this node or cluster. | [reserved](#support-status) |
| events | [cockroach.sql.contentionpb.SerializedRegistry](#cockroach.server.serverpb.ListContentionEventsResponse-cockroach.sql.contentionpb.SerializedRegistry) | | All available contention information on this node or cluster. | [reserved](#support-status) |
| errors | [ListContentionEventsError](#cockroach.server.serverpb.ListContentionEventsResponse-cockroach.server.serverpb.ListContentionEventsError) | repeated | Any errors that occurred during fan-out calls to other nodes. | [reserved](#support-status) |


Expand Down Expand Up @@ -1474,14 +1482,22 @@ An error wrapper object for ListContentionEventsResponse.

ListLocalContentionEvents retrieves the contention events on this node.

On the highest level, all IndexContentionEvents objects are ordered
according to their importance (as defined by the number of contention
events within each object).
On the middle level, all SingleKeyContention objects are ordered by their
keys lexicographically.
On the lowest level, all SingleTxnContention objects are ordered by the
number of times that transaction was observed to contend with other
transactions.
For SQL keys the following orderings are maintained:
- on the highest level, all IndexContentionEvents objects are ordered
according to their importance (as defined by the number of contention
events within each object).
- on the middle level, all SingleKeyContention objects are ordered by their
keys lexicographically.
- on the lowest level, all SingleTxnContention objects are ordered by the
number of times that transaction was observed to contend with other
transactions.

For non-SQL keys the following orderings are maintained:
- on the top level, all SingleNonSQLKeyContention objects are ordered
by their keys lexicographically.
- on the bottom level, all SingleTxnContention objects are ordered by the
number of times that transaction was observed to contend with other
transactions.

Support status: [reserved](#support-status)

Expand Down Expand Up @@ -1509,7 +1525,7 @@ Response object for ListContentionEvents and ListLocalContentionEvents.

| Field | Type | Label | Description | Support status |
| ----- | ---- | ----- | ----------- | -------------- |
| events | [cockroach.sql.contentionpb.IndexContentionEvents](#cockroach.server.serverpb.ListContentionEventsResponse-cockroach.sql.contentionpb.IndexContentionEvents) | repeated | A list of contention events on this node or cluster. | [reserved](#support-status) |
| events | [cockroach.sql.contentionpb.SerializedRegistry](#cockroach.server.serverpb.ListContentionEventsResponse-cockroach.sql.contentionpb.SerializedRegistry) | | All available contention information on this node or cluster. | [reserved](#support-status) |
| errors | [ListContentionEventsError](#cockroach.server.serverpb.ListContentionEventsResponse-cockroach.server.serverpb.ListContentionEventsError) | repeated | Any errors that occurred during fan-out calls to other nodes. | [reserved](#support-status) |


Expand Down
29 changes: 10 additions & 19 deletions pkg/bench/ddl_analysis/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,21 +1,4 @@
load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")

go_library(
name = "ddl_analysis",
srcs = ["ddl_analysis_bench.go"],
importpath = "github.com/cockroachdb/cockroach/pkg/bench/ddl_analysis",
visibility = ["//visibility:public"],
deps = [
"//pkg/base",
"//pkg/kv/kvclient/kvcoord",
"//pkg/sql",
"//pkg/testutils/serverutils",
"//pkg/testutils/sqlutils",
"//pkg/util/log",
"//pkg/util/tracing",
"//pkg/util/tracing/tracingpb",
],
)
load("@io_bazel_rules_go//go:def.bzl", "go_test")

go_test(
name = "ddl_analysis_test",
Expand All @@ -24,6 +7,7 @@ go_test(
"alter_table_bench_test.go",
"bench_test.go",
"create_alter_role_bench_test.go",
"ddl_analysis_bench_test.go",
"drop_bench_test.go",
"grant_revoke_bench_test.go",
"grant_revoke_role_bench_test.go",
Expand All @@ -34,17 +18,24 @@ go_test(
"virtual_table_bench_test.go",
],
data = glob(["testdata/**"]),
embed = [":ddl_analysis"],
deps = [
"//pkg/base",
"//pkg/kv/kvclient/kvcoord",
"//pkg/security",
"//pkg/security/securitytest",
"//pkg/server",
"//pkg/sql",
"//pkg/testutils",
"//pkg/testutils/serverutils",
"//pkg/testutils/skip",
"//pkg/testutils/sqlutils",
"//pkg/testutils/testcluster",
"//pkg/util",
"//pkg/util/encoding/csv",
"//pkg/util/log",
"//pkg/util/randutil",
"//pkg/util/tracing",
"//pkg/util/tracing/tracingpb",
"@com_github_cockroachdb_errors//:errors",
"@com_github_stretchr_testify//require",
"@org_golang_x_sync//errgroup",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,9 +40,10 @@ type RoundTripBenchTestCase struct {
// RunRoundTripBenchmark sets up a db run the RoundTripBenchTestCase test cases
// and counts how many round trips the stmt specified by the test case performs.
func RunRoundTripBenchmark(b *testing.B, tests []RoundTripBenchTestCase) {
defer log.Scope(b).Close(b)
expData := readExpectationsFile(b)
for _, tc := range tests {
b.Run(tc.name, func(b *testing.B) {
defer log.Scope(b).Close(b)
var stmtToKvBatchRequests sync.Map

beforePlan := func(trace tracing.Recording, stmt string) {
Expand All @@ -59,6 +60,7 @@ func RunRoundTripBenchmark(b *testing.B, tests []RoundTripBenchTestCase) {
},
},
}
exp, haveExp := expData.find(strings.TrimPrefix(b.Name(), "Benchmark"))

s, db, _ := serverutils.StartServer(
b, params,
Expand All @@ -70,6 +72,7 @@ func RunRoundTripBenchmark(b *testing.B, tests []RoundTripBenchTestCase) {
roundTrips := 0
b.ResetTimer()
b.StopTimer()
var r tracing.Recording
for i := 0; i < b.N; i++ {
sql.Exec(b, "CREATE DATABASE bench;")
sql.Exec(b, tc.setup)
Expand All @@ -80,8 +83,8 @@ func RunRoundTripBenchmark(b *testing.B, tests []RoundTripBenchTestCase) {
b.StopTimer()

out, _ := stmtToKvBatchRequests.Load(tc.stmt)
r, ok := out.(tracing.Recording)
if !ok {
var ok bool
if r, ok = out.(tracing.Recording); !ok {
b.Fatalf(
"could not find number of round trips for statement: %s",
tc.stmt,
Expand All @@ -101,7 +104,11 @@ func RunRoundTripBenchmark(b *testing.B, tests []RoundTripBenchTestCase) {
sql.Exec(b, tc.reset)
}

b.ReportMetric(float64(roundTrips)/float64(b.N), "roundtrips")
res := float64(roundTrips) / float64(b.N)
if haveExp && !exp.matches(int(res)) && *rewriteFlag == "" {
b.Fatalf("got %v, expected %v. trace: \n%v", res, exp, r)
}
b.ReportMetric(res, "roundtrips")
})
}
}
Expand Down
128 changes: 64 additions & 64 deletions pkg/bench/ddl_analysis/testdata/benchmark_expectations
Original file line number Diff line number Diff line change
@@ -1,74 +1,74 @@
exp,benchmark
6,AlterRole/alter_role_with_1_option
7,AlterRole/alter_role_with_2_options
9,AlterRole/alter_role_with_3_options
15,AlterTableAddCheckConstraint/alter_table_add_1_check_constraint
15,AlterTableAddCheckConstraint/alter_table_add_2_check_constraints
15,AlterTableAddCheckConstraint/alter_table_add_3_check_constraints
15,AlterTableAddColumn/alter_table_add_1_column
15,AlterTableAddColumn/alter_table_add_2_columns
15,AlterTableAddColumn/alter_table_add_3_columns
23,AlterTableAddForeignKey/alter_table_add_1_foreign_key
31,AlterTableAddForeignKey/alter_table_add_2_foreign_keys
39,AlterTableAddForeignKey/alter_table_add_3_foreign_keys
23,AlterTableAddForeignKey/alter_table_add_foreign_key_with_3_columns
25,AlterTableConfigureZone/alter_table_configure_zone_5_replicas
25,AlterTableConfigureZone/alter_table_configure_zone_7_replicas_
25,AlterTableConfigureZone/alter_table_configure_zone_ranges
18,AlterTableDropColumn/alter_table_drop_1_column
21,AlterTableDropColumn/alter_table_drop_2_columns
24,AlterTableDropColumn/alter_table_drop_3_columns
16,AlterTableDropConstraint/alter_table_drop_1_check_constraint
17,AlterTableDropConstraint/alter_table_drop_2_check_constraints
18,AlterTableDropConstraint/alter_table_drop_3_check_constraints
11-12,AlterTableSplit/alter_table_split_at_1_value
16-17,AlterTableSplit/alter_table_split_at_2_values
21-22,AlterTableSplit/alter_table_split_at_3_values
8,AlterTableUnsplit/alter_table_unsplit_at_1_value
10,AlterTableUnsplit/alter_table_unsplit_at_2_values
12,AlterTableUnsplit/alter_table_unsplit_at_3_values
6,CreateRole/create_role_with_1_option
8,CreateRole/create_role_with_2_options
9,CreateRole/create_role_with_3_options
7,CreateRole/create_role_with_no_options
17,DropDatabase/drop_database_0_tables
24,DropDatabase/drop_database_1_table
31,DropDatabase/drop_database_2_tables
38,DropDatabase/drop_database_3_tables
17,DropRole/drop_1_role
25,DropRole/drop_2_roles
33,DropRole/drop_3_roles
17,DropSequence/drop_1_sequence
28,DropSequence/drop_2_sequences
39,DropSequence/drop_3_sequences
19,DropTable/drop_1_table
32,DropTable/drop_2_tables
45,DropTable/drop_3_tables
21,DropView/drop_1_view
43,DropView/drop_2_views
65,DropView/drop_3_views
18,Grant/grant_all_on_1_table
24,Grant/grant_all_on_2_tables
30,Grant/grant_all_on_3_tables
16,GrantRole/grant_1_role
19,GrantRole/grant_2_roles
7,AlterRole/alter_role_with_1_option
8,AlterRole/alter_role_with_2_options
10,AlterRole/alter_role_with_3_options
19,AlterTableAddCheckConstraint/alter_table_add_1_check_constraint
19,AlterTableAddCheckConstraint/alter_table_add_2_check_constraints
19,AlterTableAddCheckConstraint/alter_table_add_3_check_constraints
19,AlterTableAddColumn/alter_table_add_1_column
19,AlterTableAddColumn/alter_table_add_2_columns
19,AlterTableAddColumn/alter_table_add_3_columns
29,AlterTableAddForeignKey/alter_table_add_1_foreign_key
39,AlterTableAddForeignKey/alter_table_add_2_foreign_keys
49,AlterTableAddForeignKey/alter_table_add_3_foreign_keys
29,AlterTableAddForeignKey/alter_table_add_foreign_key_with_3_columns
28,AlterTableConfigureZone/alter_table_configure_zone_5_replicas
28,AlterTableConfigureZone/alter_table_configure_zone_7_replicas_
28,AlterTableConfigureZone/alter_table_configure_zone_ranges
23,AlterTableDropColumn/alter_table_drop_1_column
27,AlterTableDropColumn/alter_table_drop_2_columns
31,AlterTableDropColumn/alter_table_drop_3_columns
20,AlterTableDropConstraint/alter_table_drop_1_check_constraint
21,AlterTableDropConstraint/alter_table_drop_2_check_constraints
22,AlterTableDropConstraint/alter_table_drop_3_check_constraints
13-14,AlterTableSplit/alter_table_split_at_1_value
19-20,AlterTableSplit/alter_table_split_at_2_values
25-26,AlterTableSplit/alter_table_split_at_3_values
9,AlterTableUnsplit/alter_table_unsplit_at_1_value
11,AlterTableUnsplit/alter_table_unsplit_at_2_values
13,AlterTableUnsplit/alter_table_unsplit_at_3_values
7,CreateRole/create_role_with_1_option
9,CreateRole/create_role_with_2_options
10,CreateRole/create_role_with_3_options
8,CreateRole/create_role_with_no_options
21,DropDatabase/drop_database_0_tables
30,DropDatabase/drop_database_1_table
39,DropDatabase/drop_database_2_tables
48,DropDatabase/drop_database_3_tables
19,DropRole/drop_1_role
29,DropRole/drop_2_roles
39,DropRole/drop_3_roles
21,DropSequence/drop_1_sequence
35,DropSequence/drop_2_sequences
49,DropSequence/drop_3_sequences
24,DropTable/drop_1_table
41,DropTable/drop_2_tables
58,DropTable/drop_3_tables
27,DropView/drop_1_view
56,DropView/drop_2_views
85,DropView/drop_3_views
22,Grant/grant_all_on_1_table
31,Grant/grant_all_on_2_tables
40,Grant/grant_all_on_3_tables
19,GrantRole/grant_1_role
22,GrantRole/grant_2_roles
5,ORMQueries/activerecord_type_introspection_query
2,ORMQueries/django_table_introspection_1_table
2,ORMQueries/django_table_introspection_4_tables
2,ORMQueries/django_table_introspection_8_tables
18,Revoke/revoke_all_on_1_table
24,Revoke/revoke_all_on_2_tables
30,Revoke/revoke_all_on_3_tables
15,RevokeRole/revoke_1_role
17,RevokeRole/revoke_2_roles
22,Revoke/revoke_all_on_1_table
31,Revoke/revoke_all_on_2_tables
40,Revoke/revoke_all_on_3_tables
18,RevokeRole/revoke_1_role
20,RevokeRole/revoke_2_roles
1,SystemDatabaseQueries/select_system.users_with_empty_database_name
1,SystemDatabaseQueries/select_system.users_with_schema_name
2,SystemDatabaseQueries/select_system.users_without_schema_name
22,Truncate/truncate_1_column_0_rows
22,Truncate/truncate_1_column_1_row
22,Truncate/truncate_1_column_2_rows
22,Truncate/truncate_2_column_0_rows
22,Truncate/truncate_2_column_1_rows
22,Truncate/truncate_2_column_2_rows
28,Truncate/truncate_1_column_0_rows
28,Truncate/truncate_1_column_1_row
28,Truncate/truncate_1_column_2_rows
28,Truncate/truncate_2_column_0_rows
28,Truncate/truncate_2_column_1_rows
28,Truncate/truncate_2_column_2_rows
1,VirtualTableQueries/select_crdb_internal.invalid_objects_with_1_fk
1,VirtualTableQueries/select_crdb_internal.tables_with_1_fk
Loading

0 comments on commit ae3a358

Please sign in to comment.