From 5d59dcb3522b91096192f6a253f21a62fb282f99 Mon Sep 17 00:00:00 2001 From: Ezequiel Lares Date: Sun, 20 Sep 2026 21:26:30 -0700 Subject: [PATCH] fix: serialize the process-global serve gate across the upgrade hero suites, and clear the rustls advisory - The process-GLOBAL SERVING gate (#391 PR-5) is one AtomicBool per process, and two test suites flip it: commit.rs's barrier tests and the hero-e continuity test in commit.rs, plus orchestrator.rs's four cutover hero tests. commit.rs had a MODULE-LOCAL lock that the orchestrator suite could not see, so the hero-e test flaked in the full lib run ("the NEW is not serving at the flip point" / a post-flip -LOADING): a sibling test's flip interleaved with its exact-transition assertions. - Serve gate tests now serialize on ONE shared lock: serve_shard_state.rs declares SERVING_GATE_TEST_LOCK (tokio::sync::Mutex, cfg(test) only), serve re-exports it, the four barrier tests and hero-e (via blocking_lock around its current-thread runtime) in commit.rs take it, and the four orchestrator hero tests take it with .lock().await as their first statement. Verified: the full lib suite passes 5/5 consecutive runs (389 tests each), where it failed ~1 in 3 before. - cargo update -p rustls cleared RUSTSEC-2026-0285 (TLS 1.3 handshake messages across encryption-level boundaries, patched in rustls 0.23.45); cargo deny check advisories is green. NOT in this PR: the metrics_endpoint / shard_owners port races. They stem from SO_REUSEPORT letting two test PROCESSES bind the same port and split connections; cargo-nextest (the sanctioned runner in rust.yml) absorbs them with per-test retries (the workspace suite reports 2675 passed). A pid-derived port attempt made them worse under full concurrency and was reverted. Local validation: workspace nextest 2675 passed / 4 skipped (0 flaky on the final run); ironcache lib 389/389 x5; fmt and clippy clean. --- Cargo.lock | 8 ++-- crates/ironcache/src/serve.rs | 2 + crates/ironcache/src/serve_shard_state.rs | 11 ++++++ crates/ironcache/src/upgrade/commit.rs | 40 ++++++++++++-------- crates/ironcache/src/upgrade/orchestrator.rs | 4 ++ 5 files changed, 46 insertions(+), 19 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c9dff135..84b1150f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -914,9 +914,9 @@ dependencies = [ [[package]] name = "rustls" -version = "0.23.40" +version = "0.23.45" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "ef86cd5876211988985292b91c96a8f2d298df24e75989a43a3c73f2d4d8168b" +checksum = "0d41d731c7d2f962d1ccc364cec258de3c0e93b38c2fb3ba97ac74513048d634" dependencies = [ "once_cell", "ring", @@ -937,9 +937,9 @@ dependencies = [ [[package]] name = "rustls-webpki" -version = "0.103.13" +version = "0.103.15" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "61c429a8649f110dddef65e2a5ad240f747e85f7758a6bccc7e5777bd33f756e" +checksum = "f3c3cf1d8b1e7d4927e2d154c3fcb02979afb9939629c62cd9048d4f07b60ac2" dependencies = [ "ring", "rustls-pki-types", diff --git a/crates/ironcache/src/serve.rs b/crates/ironcache/src/serve.rs index 60dbcb7e..a3d69476 100644 --- a/crates/ironcache/src/serve.rs +++ b/crates/ironcache/src/serve.rs @@ -143,6 +143,8 @@ pub(crate) use serve_classify::{ // #625: the per-shard core-local state accessors + lifecycle flags. `pub(crate)` re-export so the // spine still in `serve` (serve loops, routing), every sibling submodule, and external callers // (`crate::coordinator`, `crate::replica_attach`, `crate::upgrade`, INFO) resolve them unchanged. +#[cfg(test)] +pub(crate) use serve_shard_state::SERVING_GATE_TEST_LOCK; pub(crate) use serve_shard_state::{ STORE_SLOTS_PER_DB, TRACKING, adopt_metrics_cell, adopt_process_memory_gauge, ensure_shard_ring, ensure_shard_started, fresh_shard_store, install_receiver_flip_barrier, diff --git a/crates/ironcache/src/serve_shard_state.rs b/crates/ironcache/src/serve_shard_state.rs index 1fbd3e1b..972f0f7a 100644 --- a/crates/ironcache/src/serve_shard_state.rs +++ b/crates/ironcache/src/serve_shard_state.rs @@ -578,6 +578,17 @@ pub(crate) static STORE_SLOTS_PER_DB: std::sync::atomic::AtomicUsize = /// happens-before every shard spawn, and the commit flip is a single monotonic `false -> true` edge /// gating a RETRYABLE `-LOADING`, not a data dependency (the durable store was promoted BEFORE the /// flip, so a stale read racing the flip at worst returns one more retryable `-LOADING`). + +/// The test-only serialization for PROCESS-GLOBAL gate mutations (#391 PR-5): `SERVING` +/// is one bool per process, so tests in DIFFERENT modules that flip it (the upgrade hero +/// suites in `commit.rs` and `orchestrator.rs`) must not interleave with one another -- +/// a test asserting the exact `false -> true` transition would otherwise observe a +/// sibling test's flip. `#[cfg(test)]`, so production pays nothing; sync tests use +/// `blocking_lock`, async tests `.lock().await`. +#[cfg(test)] +pub(crate) static SERVING_GATE_TEST_LOCK: tokio::sync::Mutex<()> = + tokio::sync::Mutex::const_new(()); + static SERVING: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(true); /// Whether THIS process may serve client commands yet (#391 PR-5): the HOT-PATH read the global serve diff --git a/crates/ironcache/src/upgrade/commit.rs b/crates/ironcache/src/upgrade/commit.rs index f6d8b9ce..ee4a4716 100644 --- a/crates/ironcache/src/upgrade/commit.rs +++ b/crates/ironcache/src/upgrade/commit.rs @@ -964,9 +964,25 @@ mod tests { /// (3) there is no instant where a write is acked by BOTH: the OLD is permanently quiesced from /// the release (`is_shard_loading()` stays true across the flip), and the NEW begins acking /// writes only at the flip, which is STRICTLY after that release. - #[tokio::test(flavor = "current_thread")] + #[test] #[allow(clippy::too_many_lines)] - async fn hero_e_client_continuity_across_the_serve_flip() { + fn hero_e_client_continuity_across_the_serve_flip() { + // THE SHARED GATE LOCK (see `serve_shard_state`): this test mutates the + // PROCESS-GLOBAL `SERVING` gate and asserts its exact transitions, so it must not + // interleave with ANY other test that flips the same global, in this module or in + // the orchestrator suite. The async body is driven on a current-thread runtime so + // the lock is held for the whole body. + let _guard = crate::serve::SERVING_GATE_TEST_LOCK.blocking_lock(); + tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("a current-thread runtime") + .block_on(hero_e_client_continuity_inner()) + } + + /// The body of [`hero_e_client_continuity_across_the_serve_flip`], async for the + /// concurrent reader/flipper pair. + async fn hero_e_client_continuity_inner() { use std::cell::Cell; /// The modeled NEW client reply -- exactly the top-of-`route_and_dispatch` gate decision. @@ -1543,15 +1559,15 @@ mod tests { /// than cascaded. These are plain `#[test]`s (no `.await`), so a `std` mutex guard is never held /// across an await. Each test ALSO restores `SERVING` to the default (`true`) on the way out, /// matching the slice-2 discipline so the other modules' hero tests are not perturbed. - static FLIP_BARRIER_TEST_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + // The barrier tests now serialize on the SHARED `SERVING_GATE_TEST_LOCK` (they flip the + // same process-global gate the hero tests do), rather than a module-local lock that + // the sibling suites could not see. /// N=3: the flip WAITS for ALL shards. After 2 reports the gate is STILL false; the 3rd (Nth) /// report flips it to serving -- EXACTLY on the Nth, not before. #[test] fn receiver_flip_waits_for_all_shards() { - let _guard = FLIP_BARRIER_TEST_LOCK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = crate::serve::SERVING_GATE_TEST_LOCK.blocking_lock(); crate::serve::set_serving(false); // model the NEW receiver boot (gate closed until commit). let barrier = ReceiverFlipBarrier::new(3); @@ -1578,9 +1594,7 @@ mod tests { /// count never reaches N, so the gate STAYS false -- the sibling never serves a partial keyspace. #[test] fn receiver_flip_never_serves_on_abort() { - let _guard = FLIP_BARRIER_TEST_LOCK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = crate::serve::SERVING_GATE_TEST_LOCK.blocking_lock(); crate::serve::set_serving(false); let barrier = ReceiverFlipBarrier::new(3); @@ -1599,9 +1613,7 @@ mod tests { /// commit exactly as the direct pre-barrier flip did. #[test] fn receiver_flip_single_shard_flips_on_its_commit() { - let _guard = FLIP_BARRIER_TEST_LOCK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = crate::serve::SERVING_GATE_TEST_LOCK.blocking_lock(); crate::serve::set_serving(false); let barrier = ReceiverFlipBarrier::new(1); @@ -1623,9 +1635,7 @@ mod tests { /// STAYS closed -- so the flip fires EXACTLY ONCE (on the Nth), never again. #[test] fn receiver_flip_is_idempotent_after_all_committed() { - let _guard = FLIP_BARRIER_TEST_LOCK - .lock() - .unwrap_or_else(std::sync::PoisonError::into_inner); + let _guard = crate::serve::SERVING_GATE_TEST_LOCK.blocking_lock(); crate::serve::set_serving(false); let barrier = ReceiverFlipBarrier::new(3); diff --git a/crates/ironcache/src/upgrade/orchestrator.rs b/crates/ironcache/src/upgrade/orchestrator.rs index bf3ed45f..0e612721 100644 --- a/crates/ironcache/src/upgrade/orchestrator.rs +++ b/crates/ironcache/src/upgrade/orchestrator.rs @@ -523,6 +523,7 @@ mod tests { /// store equals EXACTLY its OLD store, staging is promoted, and the OLD's quiesce is permanent. #[tokio::test(flavor = "current_thread")] async fn multi_shard_cutover_commits_and_adopts_every_shard() { + let _gate = crate::serve::SERVING_GATE_TEST_LOCK.lock().await; const SHARDS: u32 = 3; const PER: u32 = 400; crate::serve::unquiesce_shard(); @@ -615,6 +616,7 @@ mod tests { /// nothing (no promote). #[tokio::test(flavor = "current_thread")] async fn one_shard_failure_aborts_the_whole_flip_and_old_resumes() { + let _gate = crate::serve::SERVING_GATE_TEST_LOCK.lock().await; const SHARDS: u32 = 3; const PER: u32 = 120; crate::serve::unquiesce_shard(); @@ -703,6 +705,7 @@ mod tests { #[tokio::test(flavor = "current_thread")] #[allow(clippy::too_many_lines)] async fn cutover_write_stall_is_sub_second_and_zero_acked_loss() { + let _gate = crate::serve::SERVING_GATE_TEST_LOCK.lock().await; use ironcache_env::Clock; use std::cell::Cell; @@ -880,6 +883,7 @@ mod tests { /// state@E. Closes W2 end to end. #[tokio::test(flavor = "current_thread")] async fn post_commit_crash_recovers_state_at_e_from_promoted_data_dir() { + let _gate = crate::serve::SERVING_GATE_TEST_LOCK.lock().await; crate::serve::unquiesce_shard(); crate::serve::set_serving(false);