Skip to content

Commit 99f724a

Browse files
committed
treewide: Replace in-process serial mutex with nextest config
`cargo test` only runs test in parallel within one test module. [`cargo nextest`] can execute tests in parallel across crates, but in doing so executes each test in its own process (supposedly for reduced flakiness). This means our existing in-process mutexes provided by the `serial_test` crate don't work when running under nextest. We write our own `#[serial]` procedural macro which, in addition to adding the same existing in-process mutex from the `serial_test` crate, also renames tests such that they can be identified by the nextest filterset DSL. (Although it has a limited ability to parse `cfg` attrs, there is no facility for inspecting arbitrary attrs attached to the test or anything like that; otherwise we could just use the existing `#[serial]` and somehow specify it in the filterset.) This allows creating [test groups] which don't allow parallel execution within them. We then get the benefits of parallel test execution across crates; so for the bulk of the time, 1 test in the mysql group is running and 1 test in the postgres group is running, getting us to (in a simple test on an M3 Max) 181% CPU usage and a commensurate 81% speedup in clock time. ```sh $ time TZ=UTC RUN_SLOW_TESTS=1 cargo nextest run \ -E 'not package(readyset-clustertest)' ... Summary [ 393.304s] 2667 tests run: 2667 passed, 84 skipped 620.70s user 207.41s system 181% cpu 7:35.10 total $ time TZ=UTC RUN_SLOW_TESTS=1 cargo test --all \ --exclude readyset-clustertest ... 710.37s user 154.68s system 104% cpu 13:44.92 total ``` [`cargo nextest`]: https://nexte.st/ [test groups]: https://nexte.st/docs/configuration/test-groups/ Change-Id: I75a6b762bb18b4ebe7927cfe60eb7cefea038ee8 Reviewed-on: https://gerrit.readyset.name/c/readyset/+/8507 Tested-by: Buildkite CI Reviewed-by: Johnathan Davis <[email protected]>
1 parent 09417c8 commit 99f724a

File tree

34 files changed

+517
-397
lines changed

34 files changed

+517
-397
lines changed

.config/nextest.toml

+26
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
[profile.default]
2+
# This one test is absurdly long locally even compared to other tests marked slow, and not very
3+
# valuable unless you're working on related code. In which case, run with --ignore-default-filter.
4+
default-filter = 'not test(test_large_packet_write)'
5+
6+
[test-groups]
7+
serial = { max-threads = 1 }
8+
serial-postgres = { max-threads = 1 }
9+
serial-postgres13 = { max-threads = 1 }
10+
serial-mysql = { max-threads = 1 }
11+
12+
[[profile.default.overrides]]
13+
filter = 'test(/_serial$/)'
14+
test-group = 'serial'
15+
16+
[[profile.default.overrides]]
17+
filter = 'test(/_serial_postgres$/)'
18+
test-group = 'serial-postgres'
19+
20+
[[profile.default.overrides]]
21+
filter = 'test(/_serial_postgres13$/)'
22+
test-group = 'serial-postgres13'
23+
24+
[[profile.default.overrides]]
25+
filter = 'test(/_serial_mysql$/)'
26+
test-group = 'serial-mysql'

Cargo.lock

+8-4
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

+19-8
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ members = [
5656
"tinylb",
5757
"test-utils",
5858
"timestamp-service",
59-
"tournament-kway"
59+
"tournament-kway",
6060
]
6161
resolver = "2"
6262

@@ -113,7 +113,13 @@ csv = "1.3"
113113
dashmap = "6.1.0"
114114
deadpool-postgres = "0.14.0"
115115
derive_builder = "0.20.2"
116-
derive_more = { version = "1.0.0", features = ["from", "into", "try_into", "deref", "display"] }
116+
derive_more = { version = "1.0.0", features = [
117+
"from",
118+
"into",
119+
"try_into",
120+
"deref",
121+
"display",
122+
] }
117123
diff = "0.1.13"
118124
enum-display-derive = "0.1.1"
119125
enum-kinds = "0.5.1"
@@ -160,7 +166,9 @@ native-tls = "0.2.11"
160166
ndarray = "0.16.1"
161167
nom = "7.1.3"
162168
nom_locate = "4.2.0"
163-
notify = { version = "7.0.0", default-features = false, features = ["macos_kqueue"] }
169+
notify = { version = "7.0.0", default-features = false, features = [
170+
"macos_kqueue",
171+
] }
164172
num-integer = "0.1.46"
165173
num_cpus = "1.16.0"
166174
opentelemetry = "0.21.0"
@@ -189,7 +197,10 @@ regex = "1.10.4"
189197
reqwest = "0.11.27"
190198
rlimit = "0.10.1"
191199
rmp-serde = "1.2.0"
192-
rocksdb = { version = "0.22.0", default-features = false, features = ["lz4", "jemalloc"] }
200+
rocksdb = { version = "0.22.0", default-features = false, features = [
201+
"lz4",
202+
"jemalloc",
203+
] }
193204
rust_decimal = "1.26"
194205
rusty-fork = "0.3.0"
195206
rustyline = "14.0"
@@ -228,7 +239,7 @@ tikv-jemalloc-sys = "0.5.4"
228239
tikv-jemallocator = "0.5.4"
229240
time = "0.3"
230241
timekeeper = { version = "0.3.2", default-features = false }
231-
tokio = { version = "1.37", features = ["full"] }
242+
tokio = { version = "1.37", features = ["full"] }
232243
tokio-native-tls = "0.3.1"
233244
tokio-retry = "0.3"
234245
tokio-scoped = "0.2.0"
@@ -259,12 +270,12 @@ zipf = "7.0.1"
259270
jobserver = "0.1.32"
260271

261272
[profile.release]
262-
debug=true
263-
lto="thin"
273+
debug = true
274+
lto = "thin"
264275

265276
[profile.release-fat-lto]
266277
inherits = "release"
267-
lto="fat"
278+
lto = "fat"
268279

269280
[profile.release-dist]
270281
# configs for distro release packages (i.e. deb, rpm, etc.)

data-generator/Cargo.toml

+1-1
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,6 @@ readyset-data = { path = "../readyset-data/" }
2424
test-strategy = { workspace = true }
2525
proptest = { workspace = true }
2626
mysql_async = { workspace = true }
27-
tokio = { workspace = true, features = ["full"] }
2827
serial_test = { workspace = true }
28+
tokio = { workspace = true, features = ["full"] }
2929
test-utils = { path = "../test-utils" }

data-generator/tests/mysql.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@ use mysql_async::Value;
66
use nom_sql::{Dialect, DialectDisplay, SqlType};
77
use proptest::prop_assume;
88
use rand::{rngs::SmallRng, SeedableRng};
9-
use serial_test::serial;
109
use test_strategy::proptest;
10+
use test_utils::serial;
1111
use test_utils::slow;
1212

1313
async fn mysql_connection() -> mysql_async::Conn {
@@ -28,8 +28,8 @@ async fn mysql_connection() -> mysql_async::Conn {
2828
.unwrap()
2929
}
3030

31+
#[serial(mysql)]
3132
#[proptest]
32-
#[serial]
3333
#[slow]
3434
fn value_of_type_always_valid(
3535
#[any(generate_arrays = false, dialect = Some(Dialect::MySQL))] ty: SqlType,
@@ -56,8 +56,8 @@ fn value_of_type_always_valid(
5656
.unwrap();
5757
}
5858

59+
#[serial(mysql)]
5960
#[proptest]
60-
#[serial]
6161
#[slow]
6262
fn unique_value_of_type_always_valid(
6363
#[any(generate_arrays = false, dialect = Some(Dialect::MySQL))] ty: SqlType,
@@ -87,8 +87,8 @@ fn unique_value_of_type_always_valid(
8787
.unwrap();
8888
}
8989

90+
#[serial(mysql)]
9091
#[proptest]
91-
#[serial]
9292
#[slow]
9393
fn random_value_of_type_always_valid(
9494
#[any(generate_arrays = false, dialect = Some(Dialect::MySQL))] ty: SqlType,

query-generator/Cargo.toml

+2-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ edition = "2021"
88
[dependencies]
99
anyhow = { workspace = true }
1010
chrono = { workspace = true }
11-
clap = { workspace = true, features = ["derive","env"] }
11+
clap = { workspace = true, features = ["derive", "env"] }
1212
derive_more = { workspace = true }
1313
futures-util = { workspace = true }
1414
itertools = { workspace = true }
@@ -42,3 +42,4 @@ mysql_async.workspace = true
4242
serial_test = { workspace = true }
4343
tokio = { workspace = true, features = ["full"] }
4444
tokio-postgres.workspace = true
45+
test-utils = { path = "../test-utils" }

query-generator/tests/mysql.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ use mysql_async::prelude::Queryable;
44
use mysql_async::{OptsBuilder, Params};
55
use nom_sql::{CreateTableStatement, Dialect, DialectDisplay};
66
use query_generator::{GeneratorState, QuerySeed};
7-
use serial_test::serial;
87
use test_strategy::proptest;
8+
use test_utils::serial;
99

1010
async fn mysql_connection() -> mysql_async::Conn {
1111
let db_name = env::var("MYSQL_DB").unwrap_or_else(|_| "test".to_owned());
@@ -42,8 +42,8 @@ async fn mysql_connection() -> mysql_async::Conn {
4242
conn
4343
}
4444

45+
#[serial(mysql)]
4546
#[proptest]
46-
#[serial]
4747
#[ignore = "Currently failing"]
4848
fn queries_work_in_mysql(seed: QuerySeed) {
4949
let rt = tokio::runtime::Builder::new_multi_thread()

readyset-client/Cargo.toml

+21-4
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ categories = ["api-bindings", "database"]
1414
[dependencies]
1515
anyhow = { workspace = true }
1616
async-bincode = { workspace = true }
17-
clap = { workspace = true, features = ["derive","env"] }
17+
clap = { workspace = true, features = ["derive", "env"] }
1818
fail = { workspace = true }
1919
thiserror = { workspace = true }
2020
hyper = { workspace = true, features = ["stream", "client", "http2"] }
@@ -35,7 +35,14 @@ time = { workspace = true, features = ["local-offset"] }
3535
tower-service = { workspace = true }
3636
tower-layer = { workspace = true }
3737
tokio-tower = { workspace = true }
38-
tower = { workspace = true, features = ["limit", "balance", "buffer", "discover", "util", "timeout"] }
38+
tower = { workspace = true, features = [
39+
"limit",
40+
"balance",
41+
"buffer",
42+
"discover",
43+
"util",
44+
"timeout",
45+
] }
3946
tracing = { workspace = true, features = ["release_max_level_debug"] }
4047
tracing-futures = { workspace = true }
4148
slab = { workspace = true }
@@ -53,12 +60,21 @@ cloudflare-zlib = { workspace = true, features = ["arm-always"] }
5360
smallvec = { workspace = true }
5461
rocksdb.workspace = true
5562

56-
tokio-postgres = { workspace = true, features = ["with-chrono-0_4", "with-eui48-1", "with-uuid-0_8", "with-serde_json-1", "with-bit-vec-0_6"] }
63+
tokio-postgres = { workspace = true, features = [
64+
"with-chrono-0_4",
65+
"with-eui48-1",
66+
"with-uuid-0_8",
67+
"with-serde_json-1",
68+
"with-bit-vec-0_6",
69+
] }
5770
metrics = { workspace = true }
5871
metrics-util = { workspace = true }
5972
itertools = { workspace = true }
6073
bytes = { workspace = true }
61-
rust_decimal = { workspace = true, features = ["db-tokio-postgres", "serde-str"] }
74+
rust_decimal = { workspace = true, features = [
75+
"db-tokio-postgres",
76+
"serde-str",
77+
] }
6278
eui48 = { workspace = true }
6379
uuid = { workspace = true, features = ["v4"] }
6480
bit-vec = { workspace = true, features = ["serde"] }
@@ -92,6 +108,7 @@ replication-offset = { path = "../replication-offset" }
92108
[dev-dependencies]
93109
serial_test = { workspace = true }
94110
tempfile = { workspace = true }
111+
test-utils = { path = "../test-utils" }
95112

96113
[features]
97114
failure_injection = ["fail/failpoints"]

readyset-client/src/consensus/consul.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -984,7 +984,7 @@ mod tests {
984984
use rand::{thread_rng, Rng};
985985
use readyset_data::Dialect;
986986
use reqwest::Url;
987-
use serial_test::serial;
987+
use test_utils::serial;
988988

989989
use super::*;
990990
use crate::consensus::CacheDDLRequest;

readyset-clustertest/Cargo.toml

+3-3
Original file line numberDiff line numberDiff line change
@@ -31,14 +31,14 @@ readyset-tracing = { path = "../readyset-tracing" }
3131
readyset-errors = { path = "../readyset-errors" }
3232

3333
[dev-dependencies]
34-
serial_test = { workspace = true }
35-
criterion = { workspace = true, features=["async_tokio"]}
34+
criterion = { workspace = true, features = ["async_tokio"] }
3635
itertools = { workspace = true }
3736
readyset-client-metrics = { path = "../readyset-client-metrics" }
38-
test-utils = { path = "../test-utils" }
3937
readyset-clustertest-macros = { path = "./macros" }
4038
readyset-tracing = { path = "../readyset-tracing" }
4139
rust_decimal = { workspace = true }
40+
serial_test = { workspace = true }
41+
test-utils = { path = "../test-utils" }
4242

4343
[build-dependencies]
4444
anyhow = { workspace = true }

readyset-clustertest/macros/src/lib.rs

+4-3
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
extern crate proc_macro;
22
use proc_macro::TokenStream;
33
use quote::quote;
4-
use syn::{parse_macro_input, parse_quote, ItemFn};
4+
use syn::{parse_macro_input, parse_quote, Ident, ItemFn};
55

66
#[proc_macro_attribute]
7-
pub fn clustertest(_attr: TokenStream, input: TokenStream) -> TokenStream {
7+
pub fn clustertest(args: TokenStream, input: TokenStream) -> TokenStream {
8+
let group = parse_macro_input!(args as Option<Ident>);
89
let input_fn = parse_macro_input!(input as ItemFn);
910

1011
let fn_block = *input_fn.block;
@@ -25,7 +26,7 @@ pub fn clustertest(_attr: TokenStream, input: TokenStream) -> TokenStream {
2526

2627
let result = quote! {
2728
#[tokio::test(flavor = "multi_thread")]
28-
#[serial]
29+
#[serial(#group)]
2930
#test_with_tracing
3031
};
3132
result.into()

0 commit comments

Comments
 (0)