Skip to content

Commit 9ae15a1

Browse files
mystenmarkmlogan
andauthored
Run each simulation in its own process; make simulator test state process-global (#27435)
## Description The deterministic simulator is gaining a blocking-task pool, so a simulation will run code across multiple threads. Per-simulation test state kept in `thread_local!`s (fail-point and debug-fatal registries, framework / advance-epoch / protocol-config injection overrides, the JWK injector, checkpoint-timeout override, fork-stake and synthetic-execution-time accounting, node-leak counter, sim IP/port allocator, RNG seed) is invisible across those threads — a value a test sets on the main thread is not seen on a pool thread. This moves all of it to process globals, which requires at most one simulation per process. That single-sim-per-process model retires the in-process determinism machinery: `MSIM_TEST_NUM` (many seeds in one process), `MSIM_TEST_CHECK_DETERMINISM` / `#[sim_test(check_determinism)]` (run twice in one process, compare RNG logs), and `init_static_initializers` (which existed only to make that in-process double-run deterministic, and whose own comment notes it has no effect on process-level determinism). Determinism is now checked externally, by running a test twice in separate processes and diffing the output (`scripts/simtest/check-determinism.sh`, run in the `simtest` CI job); multi-seed searches use `seed-search.py` / a seed loop. Independent of, and landing ahead of, the blocking pool itself: process globals behave identically single-threaded, so this is a no-op for current simulator behavior. ## Test plan - `scripts/simtest/check-determinism.sh` runs `test_net_determinism` twice in two processes with the same seed and diffs the normalized logs; verified locally it passes (byte-identical sim output). It runs in the `simtest` job on every PR. - Existing simulator tests continue to exercise the converted state; the `simulator_tests` determinism tests still run (now once each, as ordinary tests). ## Release notes Check each box that your change affects. If none of the boxes relate to your changes, release notes are not required. - [ ] Protocol: - [ ] Nodes (Validators and Full nodes): - [ ] gRPC: - [ ] JSON-RPC: - [ ] GraphQL: - [ ] CLI: - [ ] Rust SDK: --------- Co-authored-by: Mark Logan <mark@marklgn.com>
1 parent 82ac538 commit 9ae15a1

23 files changed

Lines changed: 252 additions & 445 deletions

File tree

.github/workflows/rust.yml

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -435,9 +435,9 @@ jobs:
435435
- name: cargo simtest
436436
run: |
437437
MSIM_TEST_SEED="$(printf "%lu\n" 0x$(git rev-parse HEAD | cut -c1-16))" scripts/simtest/cargo-simtest simtest --profile ci --cargo-quiet --no-fail-fast
438-
- name: check new tests for flakiness
438+
- name: check determinism
439439
run: |
440-
scripts/simtest/stress-new-tests.sh
440+
MSIM_TEST_SEED="$(printf "%lu\n" 0x$(git rev-parse HEAD | cut -c1-16))" scripts/simtest/check-determinism.sh
441441
442442
simtest-mainnet:
443443
permissions:
@@ -473,9 +473,6 @@ jobs:
473473
- name: cargo simtest
474474
run: |
475475
MSIM_TEST_SEED="$(printf "%lu\n" 0x$(git rev-parse HEAD | cut -c1-16))" scripts/simtest/cargo-simtest simtest --profile ci --cargo-quiet --no-fail-fast
476-
- name: check new tests for flakiness
477-
run: |
478-
scripts/simtest/stress-new-tests.sh
479476
480477
# This job ensures that Move unit tests are run if there are changes
481478
# to Move code but not Rust code (If there are Rust changes, they

.github/workflows/simulator-nightly.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ on:
1818
required: true
1919
default: main
2020
test_num:
21-
description: "MSIM_TEST_NUM (test iterations)"
21+
description: "Number of seeds to run (seed-search iterations)"
2222
type: string
2323
required: false
2424
default: "30"

crates/mysten-common/src/logging.rs

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -42,21 +42,17 @@ pub mod intercept_debug_fatal {
4242
pub callback: Arc<dyn Fn() + Send + Sync>,
4343
}
4444

45-
thread_local! {
46-
static INTERCEPT_DEBUG_FATAL: Mutex<Option<DebugFatalCallback>> = Mutex::new(None);
47-
}
45+
static INTERCEPT_DEBUG_FATAL: Mutex<Option<DebugFatalCallback>> = Mutex::new(None);
4846

4947
pub fn register_callback(message: &str, f: impl Fn() + Send + Sync + 'static) {
50-
INTERCEPT_DEBUG_FATAL.with(|m| {
51-
*m.lock().unwrap() = Some(DebugFatalCallback {
52-
pattern: message.to_string(),
53-
callback: Arc::new(f),
54-
});
48+
*INTERCEPT_DEBUG_FATAL.lock().unwrap() = Some(DebugFatalCallback {
49+
pattern: message.to_string(),
50+
callback: Arc::new(f),
5551
});
5652
}
5753

5854
pub fn get_callback() -> Option<DebugFatalCallback> {
59-
INTERCEPT_DEBUG_FATAL.with(|m| m.lock().unwrap().clone())
55+
INTERCEPT_DEBUG_FATAL.lock().unwrap().clone()
6056
}
6157
}
6258

crates/sui-config/src/local_ip_utils.rs

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -40,11 +40,12 @@ impl SimAddressManager {
4040

4141
#[cfg(msim)]
4242
fn get_sim_address_manager() -> Arc<SimAddressManager> {
43-
thread_local! {
44-
// Uses Arc so that we could return a clone of the thread local singleton.
45-
static SIM_ADDRESS_MANAGER: Arc<SimAddressManager> = Arc::new(SimAddressManager::new());
46-
}
47-
SIM_ADDRESS_MANAGER.with(|s| s.clone())
43+
// Uses Arc so that we could return a clone of the process-global singleton.
44+
static SIM_ADDRESS_MANAGER: std::sync::OnceLock<Arc<SimAddressManager>> =
45+
std::sync::OnceLock::new();
46+
SIM_ADDRESS_MANAGER
47+
.get_or_init(|| Arc::new(SimAddressManager::new()))
48+
.clone()
4849
}
4950

5051
/// In simtest, we generate a new unique IP each time this function is called.

crates/sui-core/src/authority.rs

Lines changed: 44 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -2873,15 +2873,14 @@ impl AuthorityState {
28732873
>,
28742874
fork_probability: f32,
28752875
) {
2876-
use std::cell::RefCell;
2877-
thread_local! {
2878-
static TOTAL_FAILING_STAKE: RefCell<u64> = RefCell::new(0);
2879-
}
2876+
static TOTAL_FAILING_STAKE: std::sync::Mutex<u64> = std::sync::Mutex::new(0);
28802877
if !certificate.data().intent_message().value.is_system_tx() {
28812878
let committee = epoch_store.committee();
28822879
let cur_stake = (**committee).weight(&self.name);
28832880
if cur_stake > 0 {
2884-
TOTAL_FAILING_STAKE.with_borrow_mut(|total_stake| {
2881+
{
2882+
let mut total_stake = TOTAL_FAILING_STAKE.lock().unwrap();
2883+
let total_stake = &mut *total_stake;
28852884
let already_forked = forked_validators
28862885
.lock()
28872886
.ok()
@@ -2937,7 +2936,7 @@ impl AuthorityState {
29372936
}
29382937
}
29392938
}
2940-
});
2939+
}
29412940
}
29422941
}
29432942
}
@@ -6554,17 +6553,15 @@ impl TransactionKeyValueStoreTrait for AuthorityState {
65546553
pub mod framework_injection {
65556554
use move_binary_format::CompiledModule;
65566555
use std::collections::BTreeMap;
6557-
use std::{cell::RefCell, collections::BTreeSet};
6556+
use std::collections::BTreeSet;
6557+
use std::sync::Mutex;
65586558
use sui_framework::{BuiltInFramework, SystemPackage};
65596559
use sui_types::base_types::{AuthorityName, ObjectID};
65606560
use sui_types::is_system_package;
65616561

65626562
type FrameworkOverrideConfig = BTreeMap<ObjectID, PackageOverrideConfig>;
65636563

6564-
// Thread local cache because all simtests run in a single unique thread.
6565-
thread_local! {
6566-
static OVERRIDE: RefCell<FrameworkOverrideConfig> = RefCell::new(FrameworkOverrideConfig::default());
6567-
}
6564+
static OVERRIDE: Mutex<FrameworkOverrideConfig> = Mutex::new(BTreeMap::new());
65686565

65696566
type Framework = Vec<CompiledModule>;
65706567

@@ -6588,60 +6585,61 @@ pub mod framework_injection {
65886585
}
65896586

65906587
pub fn set_override(package_id: ObjectID, modules: Vec<CompiledModule>) {
6591-
OVERRIDE.with(|bs| {
6592-
bs.borrow_mut()
6593-
.insert(package_id, PackageOverrideConfig::Global(modules))
6594-
});
6588+
OVERRIDE
6589+
.lock()
6590+
.unwrap()
6591+
.insert(package_id, PackageOverrideConfig::Global(modules));
65956592
}
65966593

65976594
pub fn set_override_cb(package_id: ObjectID, func: PackageUpgradeCallback) {
6598-
OVERRIDE.with(|bs| {
6599-
bs.borrow_mut()
6600-
.insert(package_id, PackageOverrideConfig::PerValidator(func))
6601-
});
6595+
OVERRIDE
6596+
.lock()
6597+
.unwrap()
6598+
.insert(package_id, PackageOverrideConfig::PerValidator(func));
66026599
}
66036600

66046601
pub fn set_system_packages(packages: Vec<SystemPackage>) {
6605-
OVERRIDE.with(|bs| {
6606-
let mut new_packages_not_to_include: BTreeSet<_> =
6607-
BuiltInFramework::all_package_ids().into_iter().collect();
6608-
for pkg in &packages {
6609-
new_packages_not_to_include.remove(&pkg.id);
6610-
}
6611-
for pkg in packages {
6612-
bs.borrow_mut()
6613-
.insert(pkg.id, PackageOverrideConfig::Global(pkg.modules()));
6614-
}
6615-
for empty_pkg in new_packages_not_to_include {
6616-
bs.borrow_mut()
6617-
.insert(empty_pkg, PackageOverrideConfig::Global(vec![]));
6618-
}
6619-
});
6602+
let mut cfg = OVERRIDE.lock().unwrap();
6603+
let mut new_packages_not_to_include: BTreeSet<_> =
6604+
BuiltInFramework::all_package_ids().into_iter().collect();
6605+
for pkg in &packages {
6606+
new_packages_not_to_include.remove(&pkg.id);
6607+
}
6608+
for pkg in packages {
6609+
cfg.insert(pkg.id, PackageOverrideConfig::Global(pkg.modules()));
6610+
}
6611+
for empty_pkg in new_packages_not_to_include {
6612+
cfg.insert(empty_pkg, PackageOverrideConfig::Global(vec![]));
6613+
}
66206614
}
66216615

66226616
pub fn get_override_bytes(package_id: &ObjectID, name: AuthorityName) -> Option<Vec<Vec<u8>>> {
6623-
OVERRIDE.with(|cfg| {
6624-
cfg.borrow().get(package_id).and_then(|entry| match entry {
6617+
OVERRIDE
6618+
.lock()
6619+
.unwrap()
6620+
.get(package_id)
6621+
.and_then(|entry| match entry {
66256622
PackageOverrideConfig::Global(framework) => {
66266623
Some(compiled_modules_to_bytes(framework))
66276624
}
66286625
PackageOverrideConfig::PerValidator(func) => {
66296626
func(name).map(|fw| compiled_modules_to_bytes(&fw))
66306627
}
66316628
})
6632-
})
66336629
}
66346630

66356631
pub fn get_override_modules(
66366632
package_id: &ObjectID,
66376633
name: AuthorityName,
66386634
) -> Option<Vec<CompiledModule>> {
6639-
OVERRIDE.with(|cfg| {
6640-
cfg.borrow().get(package_id).and_then(|entry| match entry {
6635+
OVERRIDE
6636+
.lock()
6637+
.unwrap()
6638+
.get(package_id)
6639+
.and_then(|entry| match entry {
66416640
PackageOverrideConfig::Global(framework) => Some(framework.clone()),
66426641
PackageOverrideConfig::PerValidator(func) => func(name),
66436642
})
6644-
})
66456643
}
66466644

66476645
pub fn get_override_system_package(
@@ -6666,12 +6664,12 @@ pub mod framework_injection {
66666664

66676665
pub fn get_extra_packages(name: AuthorityName) -> Vec<SystemPackage> {
66686666
let built_in = BTreeSet::from_iter(BuiltInFramework::all_package_ids().into_iter());
6669-
let extra: Vec<ObjectID> = OVERRIDE.with(|cfg| {
6670-
cfg.borrow()
6671-
.keys()
6672-
.filter_map(|package| (!built_in.contains(package)).then_some(*package))
6673-
.collect()
6674-
});
6667+
let extra: Vec<ObjectID> = OVERRIDE
6668+
.lock()
6669+
.unwrap()
6670+
.keys()
6671+
.filter_map(|package| (!built_in.contains(package)).then_some(*package))
6672+
.collect();
66756673

66766674
extra
66776675
.into_iter()

crates/sui-core/src/authority/execution_time_estimator.rs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -545,9 +545,7 @@ impl ExecutionTimeObserver {
545545
panic!("get_test_duration called in non-test configuration");
546546
}
547547

548-
thread_local! {
549-
static PER_TEST_SEED: u64 = random::<u64>();
550-
}
548+
static PER_TEST_SEED: std::sync::OnceLock<u64> = std::sync::OnceLock::new();
551549

552550
let mut hasher = std::collections::hash_map::DefaultHasher::new();
553551

@@ -564,7 +562,7 @@ impl ExecutionTimeObserver {
564562
.is_some();
565563

566564
if !checkpoint_digest_used {
567-
PER_TEST_SEED.with(|seed| seed.hash(&mut hasher));
565+
PER_TEST_SEED.get_or_init(random::<u64>).hash(&mut hasher);
568566
}
569567

570568
key.hash(&mut hasher);

crates/sui-core/src/checkpoints/checkpoint_executor/utils.rs

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -107,19 +107,17 @@ pub struct CheckpointTimeoutConfig {
107107
pub warning_timeout: Duration,
108108
}
109109

110-
// We use a thread local so that the config can be overridden on a per-test basis. This means
111-
// that get_scheduling_timeout() can be called multiple times in a multithreaded context, but
112-
// the function is still very cheap to call so this is okay.
113-
thread_local! {
114-
static SCHEDULING_TIMEOUT: once_cell::sync::OnceCell<CheckpointTimeoutConfig> =
115-
const { once_cell::sync::OnceCell::new() };
116-
}
110+
// The config can be overridden on a per-test basis. get_scheduling_timeout() can be called
111+
// multiple times in a multithreaded context, but the function is still very cheap to call so
112+
// this is okay.
113+
static SCHEDULING_TIMEOUT: std::sync::OnceLock<CheckpointTimeoutConfig> =
114+
std::sync::OnceLock::new();
117115

118116
#[cfg(msim)]
119117
pub fn init_checkpoint_timeout_config(config: CheckpointTimeoutConfig) {
120-
SCHEDULING_TIMEOUT.with(|s| {
121-
s.set(config).expect("SchedulingTimeoutConfig already set");
122-
});
118+
SCHEDULING_TIMEOUT
119+
.set(config)
120+
.expect("SchedulingTimeoutConfig already set");
123121
}
124122

125123
fn get_scheduling_timeout() -> CheckpointTimeoutConfig {
@@ -145,7 +143,7 @@ fn get_scheduling_timeout() -> CheckpointTimeoutConfig {
145143
}
146144
}
147145

148-
SCHEDULING_TIMEOUT.with(|s| *s.get_or_init(inner))
146+
*SCHEDULING_TIMEOUT.get_or_init(inner)
149147
}
150148

151149
pub(super) fn assert_not_forked(

crates/sui-e2e-tests/tests/full_node_tests.rs

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -528,11 +528,6 @@ async fn test_full_node_sync_flood() {
528528
do_test_full_node_sync_flood().await
529529
}
530530

531-
#[sim_test(check_determinism)]
532-
async fn test_full_node_sync_flood_determinism() {
533-
do_test_full_node_sync_flood().await
534-
}
535-
536531
async fn do_test_full_node_sync_flood() {
537532
let mut test_cluster = TestClusterBuilder::new()
538533
.disable_fullnode_pruning()

crates/sui-e2e-tests/tests/reconfiguration_tests.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ async fn test_passive_reconfig_testnet_smoke_test() {
9999
do_test_passive_reconfig(Some(Chain::Testnet)).await;
100100
}
101101

102-
#[sim_test(check_determinism)]
102+
#[sim_test]
103103
async fn test_passive_reconfig_determinism() {
104104
do_test_passive_reconfig(None).await;
105105
}

crates/sui-e2e-tests/tests/simulator_tests.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ async fn make_fut(i: usize) -> usize {
3333
i
3434
}
3535

36-
#[sim_test(check_determinism)]
36+
#[sim_test]
3737
async fn test_futures_ordered() {
3838
telemetry_subscribers::init_for_testing();
3939

@@ -46,7 +46,7 @@ async fn test_futures_ordered() {
4646
debug!("final rng state: {}", OsRng.r#gen::<u32>());
4747
}
4848

49-
#[sim_test(check_determinism)]
49+
#[sim_test]
5050
async fn test_futures_unordered() {
5151
telemetry_subscribers::init_for_testing();
5252

@@ -61,7 +61,7 @@ async fn test_futures_unordered() {
6161
debug!("final rng state: {}", OsRng.r#gen::<u32>());
6262
}
6363

64-
#[sim_test(check_determinism)]
64+
#[sim_test]
6565
async fn test_select_unbiased() {
6666
let mut f1 = FuturesUnordered::from_iter((0..200).map(make_fut));
6767
let mut f2 = FuturesUnordered::from_iter((0..200).map(make_fut));
@@ -92,7 +92,7 @@ async fn test_select_unbiased() {
9292
debug!("final rng state: {}", OsRng.r#gen::<u32>());
9393
}
9494

95-
#[sim_test(check_determinism)]
95+
#[sim_test]
9696
async fn test_hash_collections() {
9797
telemetry_subscribers::init_for_testing();
9898

@@ -123,7 +123,7 @@ async fn test_hash_collections() {
123123

124124
// Test that starting up a network + fullnode, and sending one transaction through that network is
125125
// repeatable and deterministic.
126-
#[sim_test(check_determinism)]
126+
#[sim_test]
127127
async fn test_net_determinism() {
128128
let mut test_cluster = TestClusterBuilder::new().build().await;
129129

0 commit comments

Comments
 (0)