Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
106860: ssmemstorage: improve lock contention on RecordStatement r=j82w a=j82w

1. The stats object lock was being held even during the insights event
creation and insert which has it's own lock. The insights logic is now
moved above the stats lock.
2. Moved latency calculation outside the stats lock.
3. Moved error code calculation outside the stats lock.
4. ss_mem_storage converted to use a RWMutex. Each object in the map
has it's own lock. Read lock allows mulitple threads to read from the
map at the same time which is the common case. Writes are only done the
first time a statement is seen.

Fixes: #106789, Fixes: #55285

```
./dev bench pkg/sql/sqlstats/sslocal -f BenchmarkRecordStatement  --ignore-cache --verbose  --test-args='-test.mutexprofile=mutex_pr.out -test.cpuprofile=cpu_pr.out -test.benchtime=30s' --timeout "10m"
```

```
// Master no changes
BenchmarkRecordStatement/RecordStatement:_Parallelism_10-10         	64775088	       558.8 ns/op
    sql_stats_test.go:655: -- test log scope end --
PASS
================================================================================
INFO: Elapsed time: 37.529s, Critical Path: 37.26s
INFO: 2 processes: 1 internal, 1 darwin-sandbox.
INFO: Build completed successfully, 2 total actions
//pkg/sql/sqlstats/sslocal:sslocal_test                                  PASSED in 37.2s
```
```
// Changes after the PR
BenchmarkRecordStatement/RecordStatement:_Parallelism_10-10         	67960656	       523.8 ns/op
    sql_stats_test.go:655: -- test log scope end --
PASS
================================================================================
INFO: Elapsed time: 37.031s, Critical Path: 36.69s
INFO: 2 processes: 1 internal, 1 darwin-sandbox.
INFO: Build completed successfully, 2 total actions
//pkg/sql/sqlstats/sslocal:sslocal_test                                  PASSED in 36.7s
```

Release note (performance improvement): Reduced lock contention on `ssmemstorage.RecordStatement`. This is useful for workloads that execute the same statement concurrently on the same SQL instance.

106912: telemetryccl: Split `TestTelemetry` into smaller sub-tests r=Santamaura a=Santamaura

This change breaks up `TestTelemetry` and the `multiregion` test into four tests due to the test being quite large. The tests are: `multiregion`, `multiregion_db`, `multiregion_table`, and `multiregion_row`.

Informs: #103004

Release note: None

107099: sql: fix `CREATE TABLE AS` sourcing `SHOW <show_subcmd> <table>` job failures r=chengxiong-ruan a=ecwall

Fixes #106260

Previously `CREATE TABLE AS`/`CREATE MATERIALIZED VIEW AS` sourcing from `SHOW <show_subcmd> <table>` generated a failing schema change job with a `relation "tbl" does not exist error` because the SHOW source table was not fully qualified.

This PR fixes this by respecting the `Builder.qualifyDataSourceNamesInAST` flag in `delegate.TryDelegate()` which implements the failing SHOW commands.

Release note (bug fix): Fix failing schema change job when CREATE TABLE AS or CREATE MATERAILIZED VIEW AS sources from a SHOW command:
1. CREATE TABLE t AS SELECT * FROM [SHOW CREATE TABLE tbl];
2. CREATE TABLE t AS SELECT * FROM [SHOW INDEXES FROM tbl];
3. CREATE TABLE t AS SELECT * FROM [SHOW COLUMNS FROM tbl];
4. CREATE TABLE t AS SELECT * FROM [SHOW CONSTRAINTS FROM tbl];
5. CREATE TABLE t AS SELECT * FROM [SHOW PARTITIONS FROM TABLE tbl];
6. CREATE TABLE t AS SELECT * FROM [SHOW PARTITIONS FROM INDEX tbl@tbl_pkey];

107170: ui: fix creation index syntax on console r=maryliag a=maryliag

With an update to make the table name fully qualified, the index name was also using the fully qualified name, which contains ".", and that causes an invalid syntax since index name can't have periods.
Having the qualified name is not important for the index name itself, so this commit fixes by ignoring that part and using just the table name, how it was doing previously. It also fixes an invalid syntax when there were spaces on the name generate.

It also add a little more observability into indexes being created with STORING clause, adding that to the suffix of the index creation.

Fixes #107168

Before
<img width="583" alt="Screenshot 2023-07-19 at 10 23 21 AM" src="https://github.com/cockroachdb/cockroach/assets/1017486/e5792419-31af-4e5f-994e-13ddb716009c">


After
https://www.loom.com/share/33da8b46fc9e4f72944c1d0ab943dea0

Release note (bug fix): Index recommendation on the UI no longer uses the full qualified name of a table to create an index name, allowing the creating of indexes directly from the Console to work.

107189: tests: fix stories files r=maryliag a=maryliag

Stories files used for testing were missing some fixtures. This commit adds all missing parameters.

Epic: none

Release note: None

107197: schemachanger: skip flakey test r=rafiss a=DrewKimball

Skips `TestConcurrentDeclarativeSchemaChanges`.

Informs #106732

Release note: None

107207: bazel: make `fastbuild` the default `-c` again; never strip for `dev` builds r=rail a=rickystewart

This is a partial revert of `60e8bcec61269c6648dc40c8094fbf303b8a02aa`.

In that commit I made a change to make un-stripped binaries the default by making `dbg` the default `--compilation_mode`. This had unexpected consequences as this actually disables inlining and optimization, thereby breaking some tests, and overall it is surprising that a Go binary built by Bazel would be un-optimized by default since this is the opposite of `go build`'s default behavior.

Instead, we make `fastbuild` the default (again), and to solve the problem of binaries being unstripped we set `--strip never` for `dev` builds. (`dev doctor` makes you clarify that your build is a `dev` build on setup so no one can miss this.)

Now we have the following behavior for each `compilation_mode`:

* `dbg`: disable optimizations and inlining.
* `fastbuild` and `opt`: will produce the same binary, except `fastbuild` defaults to stripping in non-`dev` configurations.

If you want an un-optimized binary for some reason, it still works to build with `-c dbg`.

Epic: CRDB-17171
Release note: None

Co-authored-by: j82w <[email protected]>
Co-authored-by: Alex Santamaura <[email protected]>
Co-authored-by: Evan Wall <[email protected]>
Co-authored-by: maryliag <[email protected]>
Co-authored-by: Drew Kimball <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>
  • Loading branch information
7 people committed Jul 19, 2023
8 parents b005e97 + a792cd3 + 466c4e9 + 8e4578b + db4be30 + fb97251 + a05d100 + f52fe97 commit e23993f
Show file tree
Hide file tree
Showing 28 changed files with 853 additions and 536 deletions.
2 changes: 1 addition & 1 deletion .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ build --define gotags=bazel,gss
build --experimental_proto_descriptor_sets_include_source_info
build --incompatible_strict_action_env --incompatible_enable_cc_toolchain_resolution
build --symlink_prefix=_bazel/
build -c dbg
build:dev --strip=never
common --experimental_allow_tags_propagation
test --config=test --experimental_ui_max_stdouterr_bytes=10485760
build --ui_event_filters=-DEBUG
Expand Down
2 changes: 1 addition & 1 deletion build/teamcity/cockroach/ci/builds/build_impl.sh
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ fi

bazel build //pkg/cmd/bazci --config=ci
BAZEL_BIN=$(bazel info bazel-bin --config=ci)
"$BAZEL_BIN/pkg/cmd/bazci/bazci_/bazci" -- build \
"$BAZEL_BIN/pkg/cmd/bazci/bazci_/bazci" -- build -c opt \
--config "$CONFIG" --config ci $EXTRA_ARGS \
//pkg/cmd/cockroach-short //pkg/cmd/cockroach \
//pkg/cmd/cockroach-sql \
Expand Down
12 changes: 6 additions & 6 deletions build/teamcity/cockroach/nightlies/pebble_nightly_common.sh
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ mkdir -p "$PWD/bin"
chmod o+rwx "$PWD/bin"

# Build the roachtest binary.
bazel build //pkg/cmd/roachtest --config ci
BAZEL_BIN=$(bazel info bazel-bin --config ci)
bazel build //pkg/cmd/roachtest --config ci -c opt
BAZEL_BIN=$(bazel info bazel-bin --config ci -c opt)
cp $BAZEL_BIN/pkg/cmd/roachtest/roachtest_/roachtest bin
chmod a+w bin/roachtest

Expand All @@ -39,8 +39,8 @@ chmod a+w bin/roachtest
bazel run @go_sdk//:bin/go get github.com/cockroachdb/pebble@latest
NEW_DEPS_BZL_CONTENT=$(bazel run //pkg/cmd/mirror/go:mirror)
echo "$NEW_DEPS_BZL_CONTENT" > DEPS.bzl
bazel build @com_github_cockroachdb_pebble//cmd/pebble --config ci
BAZEL_BIN=$(bazel info bazel-bin --config ci)
bazel build @com_github_cockroachdb_pebble//cmd/pebble --config ci -c opt
BAZEL_BIN=$(bazel info bazel-bin --config ci -c opt)
cp $BAZEL_BIN/external/com_github_cockroachdb_pebble/cmd/pebble/pebble_/pebble ./pebble.linux
chmod a+w ./pebble.linux

Expand All @@ -67,8 +67,8 @@ function prepare_datadir() {
# Build the mkbench tool from within the Pebble repo. This is used to parse
# the benchmark data.
function build_mkbench() {
bazel build @com_github_cockroachdb_pebble//internal/mkbench --config ci
BAZEL_BIN=$(bazel info bazel-bin --config ci)
bazel build @com_github_cockroachdb_pebble//internal/mkbench --config ci -c opt
BAZEL_BIN=$(bazel info bazel-bin --config ci -c opt)
cp $BAZEL_BIN/external/com_github_cockroachdb_pebble/internal/mkbench/mkbench_/mkbench .
chmod a+w mkbench
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ mkdir -p "$PWD/bin"
chmod o+rwx "$PWD/bin"

# Build the roachtest binary.
bazel build //pkg/cmd/roachtest --config ci
BAZEL_BIN=$(bazel info bazel-bin --config ci)
bazel build //pkg/cmd/roachtest --config ci -c opt
BAZEL_BIN=$(bazel info bazel-bin --config ci -c opt)
cp $BAZEL_BIN/pkg/cmd/roachtest/roachtest_/roachtest bin
chmod a+w bin/roachtest

Expand All @@ -39,8 +39,8 @@ chmod a+w bin/roachtest
bazel run @go_sdk//:bin/go get github.com/cockroachdb/pebble@latest
NEW_DEPS_BZL_CONTENT=$(bazel run //pkg/cmd/mirror/go:mirror)
echo "$NEW_DEPS_BZL_CONTENT" > DEPS.bzl
bazel build @com_github_cockroachdb_pebble//cmd/pebble --config race --config ci
BAZEL_BIN=$(bazel info bazel-bin --config race --config ci)
bazel build @com_github_cockroachdb_pebble//cmd/pebble --config race --config ci -c opt
BAZEL_BIN=$(bazel info bazel-bin --config race --config ci -c opt)
cp $BAZEL_BIN/external/com_github_cockroachdb_pebble/cmd/pebble/pebble_/pebble ./pebble.linux
chmod a+w ./pebble.linux

Expand Down
8 changes: 4 additions & 4 deletions build/teamcity/cockroach/nightlies/roachtest_compile_bits.sh
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,21 @@ for platform in "${cross_builds[@]}"; do

echo "Building $platform, os=$os, arch=$arch..."
# Build cockroach, workload and geos libs.
bazel build --config $platform --config ci --config force_build_cdeps \
bazel build --config $platform --config ci -c opt --config force_build_cdeps \
//pkg/cmd/cockroach //pkg/cmd/workload \
//c-deps:libgeos
BAZEL_BIN=$(bazel info bazel-bin --config $platform --config ci)
BAZEL_BIN=$(bazel info bazel-bin --config $platform --config ci -c opt)

# N.B. roachtest is built once, for the host architecture.
if [[ $os == "linux" && $arch == $host_arch ]]; then
bazel build --config $platform --config ci //pkg/cmd/roachtest
bazel build --config $platform --config ci -c opt //pkg/cmd/roachtest

cp $BAZEL_BIN/pkg/cmd/roachtest/roachtest_/roachtest bin/roachtest
# Make it writable to simplify cleanup and copying (e.g., scp retry).
chmod a+w bin/roachtest
fi
# Build cockroach-short with assertions enabled.
bazel build --config $platform --config ci //pkg/cmd/cockroach-short --crdb_test
bazel build --config $platform --config ci -c opt //pkg/cmd/cockroach-short --crdb_test
# Copy the binaries.
cp $BAZEL_BIN/pkg/cmd/cockroach/cockroach_/cockroach bin/cockroach.$os-$arch
cp $BAZEL_BIN/pkg/cmd/workload/workload_/workload bin/workload.$os-$arch
Expand Down
Loading

0 comments on commit e23993f

Please sign in to comment.