Skip to content

Optimize CockroachDB setup for tests (96% reduction) #493

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 25 commits into from
Dec 9, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
082c009
[nexus] Refactor test-utilities to helper crate, add test benchmarks
smklein Dec 8, 2021
444aaa0
No need to be screamy about disk posting, that's for another PR
smklein Dec 8, 2021
42727a7
Optimize CRDB setup by using 'compile-time' population of a seed data…
smklein Dec 8, 2021
d1c1345
Merge branch 'main' into benchmark-tests
smklein Dec 8, 2021
54b6f68
Add to top-level workspace
smklein Dec 9, 2021
6850f67
Merge branch 'benchmark-tests' into optimize-crdb-setup
smklein Dec 9, 2021
c736fe0
Add some explanations
smklein Dec 9, 2021
05aed1b
re-run if build.rs changes
smklein Dec 9, 2021
304d03d
The temporary directory might not exist
smklein Dec 9, 2021
61d3fd3
Merge branch 'main' into benchmark-tests
smklein Dec 9, 2021
a992ee8
Merge branch 'benchmark-tests' into optimize-crdb-setup
smklein Dec 9, 2021
c5427aa
proby dropshot
smklein Dec 9, 2021
fea3cd9
Remove test-utils from default workspace
smklein Dec 9, 2021
0153e19
Download database executables *before* we build/test all targets
smklein Dec 9, 2021
0bcdc28
Merge branch 'main' into benchmark-tests
smklein Dec 9, 2021
52b31c6
Merge branch 'benchmark-tests' into optimize-crdb-setup
smklein Dec 9, 2021
a0c88a4
Adjust path when building, we want that executable for our build script
smklein Dec 9, 2021
43b75c9
Merge branch 'main' into benchmark-tests
smklein Dec 9, 2021
3dd98d8
Merge branch 'benchmark-tests' into optimize-crdb-setup
smklein Dec 9, 2021
419d89e
Merge branch 'main' into benchmark-tests
smklein Dec 9, 2021
7b9dc1b
Merge branch 'benchmark-tests' into optimize-crdb-setup
smklein Dec 9, 2021
9ad763c
Merge branch 'main' into optimize-crdb-setup
smklein Dec 9, 2021
1b19c57
fixup
smklein Dec 9, 2021
4f9feef
review feedback
smklein Dec 9, 2021
8eb0549
OUT of the default members again gah
smklein Dec 9, 2021
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 12 additions & 12 deletions .github/buildomat/jobs/build-and-test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,18 @@ set -o xtrace
cargo --version
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The change here - and in the ".github/workflows/rust.yml" file - just make sure cockroach is downloaded and on the PATH before we cargo build --all-targets, since we're now using it in a build script.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you update the README to reflect this? Basically this part:

CockroachDB v21.1.10.
The test suite expects to be able to start a single-node CockroachDB cluster using the cockroach executable on your PATH.

I think that should now say "the build and test suite expect..."

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated

rustc --version

banner clickhouse
ptime -m ./tools/ci_download_clickhouse

banner cockroach
ptime -m bash ./tools/ci_download_cockroachdb

#
# Put "./cockroachdb/bin" and "./clickhouse" on the PATH for the test
# suite.
#
export PATH="$PATH:$PWD/cockroachdb/bin:$PWD/clickhouse"

#
# We build with:
#
Expand All @@ -39,18 +51,6 @@ ptime -m cargo +'nightly-2021-11-24' build --locked --all-targets --verbose
banner deploy-check
ptime -m cargo run --bin omicron-package -- check

banner clickhouse
ptime -m ./tools/ci_download_clickhouse

banner cockroach
ptime -m bash ./tools/ci_download_cockroachdb

#
# Put "./cockroachdb/bin" and "./clickhouse" on the PATH for the test
# suite.
#
export PATH="$PATH:$PWD/cockroachdb/bin:$PWD/clickhouse"

#
# NOTE: We're using using the same RUSTFLAGS and RUSTDOCFLAGS as above to avoid
# having to rebuild here.
Expand Down
14 changes: 7 additions & 7 deletions .github/workflows/rust.yml
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,12 @@ jobs:
with:
key: ${{ runner.os }}-clickhouse-binary-${{ hashFiles('tools/clickhouse_checksums') }}
path: "clickhouse"
- name: Download ClickHouse
if: steps.cache-clickhouse.outputs.cache-hit != 'true'
run: ./tools/ci_download_clickhouse
- name: Download CockroachDB binary
if: steps.cache-cockroachdb.outputs.cache-hit != 'true'
run: bash ./tools/ci_download_cockroachdb
- name: Build
# We build with:
# - RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings": disallow warnings
Expand All @@ -119,13 +125,7 @@ jobs:
# also gives us a record of which dependencies were used for each CI
# run. Building with `--locked` ensures that the checked-in Cargo.lock
# is up to date.
run: RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings" cargo build --locked --all-targets --verbose
- name: Download ClickHouse
if: steps.cache-clickhouse.outputs.cache-hit != 'true'
run: ./tools/ci_download_clickhouse
- name: Download CockroachDB binary
if: steps.cache-cockroachdb.outputs.cache-hit != 'true'
run: bash ./tools/ci_download_cockroachdb
run: PATH="$PATH:$PWD/cockroachdb/bin:$PWD/clickhouse" RUSTFLAGS="-D warnings" RUSTDOCFLAGS="-D warnings" cargo build --locked --all-targets --verbose
- name: Run tests
# Use the same RUSTFLAGS and RUSTDOCFLAGS as above to avoid having to
# rebuild here.
Expand Down
79 changes: 40 additions & 39 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ default-members = [
"common",
"nexus",
"nexus/src/db/db-macros",
"nexus/test-utils",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note: this has been removed from the default members, but we're definitely still using / testing it.

This just means that cargo build doesn't, by default, build a "test-only" helper crate - doing so would also require "cockroachdb" to be on the PATH earlier for some of our checks.

Note: This still gets built with cargo build --all-targets and cargo test, so no worries on coverage.

"package",
"rpaths",
"sled-agent",
Expand Down
2 changes: 1 addition & 1 deletion README.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ example, on Helios, you'd want `/usr/bin` on your PATH.
--
. CockroachDB v21.1.10.
+
The test suite expects to be able to start a single-node CockroachDB cluster using the `cockroach` executable on your PATH.
The build and test suite expects to be able to start a single-node CockroachDB cluster using the `cockroach` executable on your PATH.
On illumos, MacOS, and Linux, you should be able to use the `tools/ci_download_cockroachdb` script to fetch the official CockroachDB binary. It will be put into `./cockroachdb/bin/cockroach`.
Alternatively, you can follow the https://www.cockroachlabs.com/docs/stable/install-cockroachdb.html[official CockroachDB installation instructions for your platform].

Expand Down
7 changes: 6 additions & 1 deletion nexus/test-utils/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ license = "MPL-2.0"
anyhow = "1.0"
bytes = "1.0.1"
chrono = { version = "0.4", features = [ "serde" ] }
dropshot = { git = "https://github.com/oxidecomputer/dropshot", branch = "main" }
dropshot = { git = "https://github.com/oxidecomputer/dropshot", branch = "main", features = [ "usdt-probes" ] }
http = "0.2.5"
hyper = "0.14"
omicron-common = { path = "../../common" }
Expand All @@ -24,3 +24,8 @@ serde = { version = "1.0", features = [ "derive" ] }
serde_json = "1.0"
slog = { version = "2.7", features = [ "max_level_trace", "release_max_level_debug" ] }
uuid = { version = "0.8", features = [ "serde", "v4" ] }

[build-dependencies]
dropshot = { git = "https://github.com/oxidecomputer/dropshot", branch = "main", features = [ "usdt-probes" ] }
omicron-test-utils = { path = "../../test-utils" }
tokio = { version = "1.14" }
31 changes: 31 additions & 0 deletions nexus/test-utils/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
// This Source Code Form is subject to the terms of the Mozilla Public
// License, v. 2.0. If a copy of the MPL was not distributed with this
// file, You can obtain one at https://mozilla.org/MPL/2.0/.

use dropshot::{test_util::LogContext, ConfigLogging, ConfigLoggingLevel};
use omicron_test_utils::dev::test_setup_database_seed;

// Creates a "pre-populated" CockroachDB storage directory, which
// subsequent tests can copy instead of creating themselves.
//
// Is it critical this happens at build-time? No. However, it
// makes it more convenient for tests to assume this seeded
// directory exists, rather than all attempting to create it
// concurrently.
//
// Refer to the documentation of [`test_setup_database_seed`] for
// more context.
#[tokio::main]
async fn main() {
println!("cargo:rerun-if-changed=build.rs");
println!("cargo:rerun-if-changed=../../common/src/sql/dbinit.sql");
println!("cargo:rerun-if-changed=../../tools/cockroachdb_checksums");
println!("cargo:rerun-if-changed=../../tools/cockroachdb_version");

let logctx = LogContext::new(
"crdb_seeding",
&ConfigLogging::StderrTerminal { level: ConfigLoggingLevel::Info },
);

test_setup_database_seed(&logctx.log).await;
}
Loading