From ccaa617e6a1762b1db543a7d10b4b6886ee4b9ce Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Mon, 21 Oct 2024 16:26:05 +0200 Subject: [PATCH 01/43] chore(PocketIC): generate state labels randomly --- rs/pocket_ic_server/src/pocket_ic.rs | 81 +++++---------------- rs/pocket_ic_server/src/state_api/routes.rs | 28 ++++--- rs/pocket_ic_server/src/state_api/state.rs | 27 +++++-- 3 files changed, 55 insertions(+), 81 deletions(-) diff --git a/rs/pocket_ic_server/src/pocket_ic.rs b/rs/pocket_ic_server/src/pocket_ic.rs index 26e688f8a25..5ad6ae5f971 100644 --- a/rs/pocket_ic_server/src/pocket_ic.rs +++ b/rs/pocket_ic_server/src/pocket_ic.rs @@ -1,5 +1,5 @@ use crate::state_api::state::{HasStateLabel, OpOut, PocketIcError, StateLabel}; -use crate::{async_trait, copy_dir, BlobStore, OpId, Operation}; +use crate::{async_trait, copy_dir, BlobStore, InstanceId, OpId, Operation}; use askama::Template; use axum::{ extract::State, @@ -69,6 +69,8 @@ use pocket_ic::common::rest::{ RawCanisterCall, RawCanisterId, RawEffectivePrincipal, RawMessageId, RawSetStableMemory, SubnetInstructionConfig, SubnetKind, SubnetSpec, Topology, }; +use rand::rngs::StdRng; +use rand::SeedableRng; use serde::{Deserialize, Serialize}; use std::hash::Hash; use std::str::FromStr; @@ -229,10 +231,10 @@ pub struct PocketIc { routing_table: RoutingTable, /// Created on initialization and updated if a new subnet is created. topology: TopologyInternal, - // The initial state hash used for computing the state label - // to distinguish PocketIC instances with different initial configs. - initial_state_hash: [u8; 32], - // The following fields are used to create a new subnet. + /// Used for choosing a random state label for this instance. + /// This value is seeded for the sake of reproducibility. + randomness: StdRng, + state_label: StateLabel, range_gen: RangeGen, registry_data_provider: Arc, runtime: Arc, @@ -386,6 +388,7 @@ impl PocketIc { pub(crate) fn new( runtime: Arc, + instance_id: InstanceId, subnet_configs: ExtendedSubnetConfigSet, state_dir: Option, nonmainnet_features: bool, @@ -653,15 +656,6 @@ impl PocketIc { subnet.execute_round(); } - let mut hasher = Sha256::new(); - let subnet_configs_string = format!("{:?}", subnet_configs); - hasher.write(subnet_configs_string.as_bytes()); - let initial_state_hash = compute_state_label( - &hasher.finish(), - subnets.read().unwrap().values().cloned().collect(), - ) - .0; - let canister_http_adapters = Arc::new(TokioMutex::new( subnets .read() @@ -696,13 +690,17 @@ impl PocketIc { default_effective_canister_id, }; + let mut randomness = StdRng::seed_from_u64(instance_id as u64); + let state_label = StateLabel::new(&mut randomness); + Self { state_dir, subnets, canister_http_adapters, routing_table, topology, - initial_state_hash, + state_label, + randomness, range_gen, registry_data_provider, runtime, @@ -713,6 +711,10 @@ impl PocketIc { } } + pub(crate) fn refresh_state_label(&mut self) { + self.state_label = StateLabel::new(&mut self.randomness); + } + fn try_route_canister(&self, canister_id: CanisterId) -> Option> { let subnet_id = self.routing_table.route(canister_id.into()); subnet_id.map(|subnet_id| self.get_subnet_with_id(subnet_id).unwrap()) @@ -762,6 +764,7 @@ impl Default for PocketIc { fn default() -> Self { Self::new( Runtime::new().unwrap().into(), + 0, ExtendedSubnetConfigSet { application: vec![SubnetSpec::default()], ..Default::default() @@ -774,31 +777,9 @@ impl Default for PocketIc { } } -fn compute_state_label( - initial_state_hash: &[u8; 32], - subnets: Vec>, -) -> StateLabel { - let mut hasher = Sha256::new(); - hasher.write(initial_state_hash); - for subnet in subnets { - let subnet_state_hash = subnet - .state_manager - .latest_state_certification_hash() - .map(|(_, h)| h.0) - .unwrap_or_else(|| [0u8; 32].to_vec()); - let nanos = systemtime_to_unix_epoch_nanos(subnet.time()); - hasher.write(&subnet_state_hash[..]); - hasher.write(&nanos.to_be_bytes()); - } - StateLabel(hasher.finish()) -} - impl HasStateLabel for PocketIc { fn get_state_label(&self) -> StateLabel { - compute_state_label( - &self.initial_state_hash, - self.subnets.read().unwrap().values().cloned().collect(), - ) + self.state_label.clone() } } @@ -2606,29 +2587,6 @@ mod tests { assert_ne!(state0, state1); assert_ne!(state1, state2); assert_ne!(state0, state2); - - // Empyt IC. - let pic = PocketIc::default(); - let state1 = pic.get_state_label(); - let pic = PocketIc::default(); - let state2 = pic.get_state_label(); - - assert_eq!(state1, state2); - - // Two ICs with the same state. - let pic = PocketIc::default(); - let cid = pic.any_subnet().create_canister(None); - pic.any_subnet().add_cycles(cid, 2_000_000_000_000); - pic.any_subnet().stop_canister(cid).unwrap(); - let state3 = pic.get_state_label(); - - let pic = PocketIc::default(); - let cid = pic.any_subnet().create_canister(None); - pic.any_subnet().add_cycles(cid, 2_000_000_000_000); - pic.any_subnet().stop_canister(cid).unwrap(); - let state4 = pic.get_state_label(); - - assert_eq!(state3, state4); } #[test] @@ -2751,6 +2709,7 @@ mod tests { fn new_pic_counter_installed_system_subnet() -> (PocketIc, CanisterId) { let mut pic = PocketIc::new( Runtime::new().unwrap().into(), + 0, ExtendedSubnetConfigSet { ii: Some(SubnetSpec::default()), ..Default::default() diff --git a/rs/pocket_ic_server/src/state_api/routes.rs b/rs/pocket_ic_server/src/state_api/routes.rs index 6bea930e50d..8e603ab2091 100644 --- a/rs/pocket_ic_server/src/state_api/routes.rs +++ b/rs/pocket_ic_server/src/state_api/routes.rs @@ -1113,21 +1113,19 @@ pub async fn create_instance( None }; - let pocket_ic = tokio::task::spawn_blocking(move || { - PocketIc::new( - runtime, - subnet_configs, - instance_config.state_dir, - instance_config.nonmainnet_features, - log_level, - instance_config.bitcoind_addr, - ) - }) - .await - .expect("Failed to launch PocketIC"); - - let topology = pocket_ic.topology().clone(); - let instance_id = api_state.add_instance(pocket_ic).await; + let (instance_id, topology) = api_state + .add_instance(move |instance_id| { + PocketIc::new( + runtime, + instance_id, + subnet_configs, + instance_config.state_dir, + instance_config.nonmainnet_features, + log_level, + instance_config.bitcoind_addr, + ) + }) + .await; ( StatusCode::CREATED, Json(rest::CreateInstanceResponse::Created { diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index d2349164d04..8f3651d9a29 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -50,6 +50,8 @@ use pocket_ic::common::rest::{ HttpGatewayDetails, HttpGatewayInfo, MockCanisterHttpResponse, Topology, }; use pocket_ic::{ErrorCode, UserError, WasmResult}; +use rand::rngs::StdRng; +use rand::RngCore; use serde::{Deserialize, Serialize}; use std::{collections::HashMap, fmt, path::PathBuf, str::FromStr, sync::Arc, time::Duration}; use tokio::{ @@ -79,6 +81,14 @@ pub const STATE_LABEL_HASH_SIZE: usize = 32; #[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Default, Deserialize)] pub struct StateLabel(pub [u8; STATE_LABEL_HASH_SIZE]); +impl StateLabel { + pub fn new(rng: &mut StdRng) -> Self { + let mut state_label = [42; STATE_LABEL_HASH_SIZE]; + rng.fill_bytes(&mut state_label); + Self(state_label) + } +} + // The only error condition is if the vector has the wrong size. pub struct InvalidSize; @@ -705,15 +715,22 @@ impl ApiState { self.graph.clone() } - pub async fn add_instance(&self, instance: PocketIc) -> InstanceId { + pub async fn add_instance(&self, f: F) -> (InstanceId, Topology) + where + F: FnOnce(InstanceId) -> PocketIc + std::marker::Send + 'static, + { let mut instances = self.instances.write().await; let mut canister_http_adapters = self.canister_http_adapters.write().await; let mut progress_threads = self.progress_threads.write().await; let instance_id = instances.len(); + let instance = tokio::task::spawn_blocking(move || f(instance_id)) + .await + .expect("Failed to create PocketIC instance"); + let topology = instance.topology(); canister_http_adapters.insert(instance_id, instance.canister_http_adapters().clone()); instances.push(Mutex::new(InstanceState::Available(instance))); progress_threads.push(Mutex::new(None)); - instance_id + (instance_id, topology) } pub async fn delete_instance(&self, instance_id: InstanceId) { @@ -1391,6 +1408,7 @@ impl ApiState { op_id.0, ); let result = op.compute(&mut pocket_ic); + pocket_ic.refresh_state_label(); let new_state_label = pocket_ic.get_state_label(); // add result to graph, but grab instance lock first! let instances = instances.blocking_read(); @@ -1408,8 +1426,7 @@ impl ApiState { *instance_state = InstanceState::Available(pocket_ic); } trace!("bg_task::end instance_id={} op_id={}", instance_id, op_id.0); - // also return old_state_label so we can prune graph if we return quickly - (result, old_state_label) + result } }; @@ -1442,7 +1459,7 @@ impl ApiState { // note: this assumes that cancelling the JoinHandle does not stop the execution of the // background task. This only works because the background thread, in this case, is a // kernel thread. - if let Ok(Ok((op_out, _old_state_label))) = time::timeout(sync_wait_time, bg_handle).await { + if let Ok(Ok(op_out)) = time::timeout(sync_wait_time, bg_handle).await { trace!( "update_with_timeout::synchronous instance_id={} op_id={}", instance_id, From c5c516cca6efd1732f81a1bf5696111917b1547c Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Mon, 21 Oct 2024 17:06:06 +0200 Subject: [PATCH 02/43] wipe state label tests --- rs/pocket_ic_server/src/pocket_ic.rs | 223 --------------------------- 1 file changed, 223 deletions(-) diff --git a/rs/pocket_ic_server/src/pocket_ic.rs b/rs/pocket_ic_server/src/pocket_ic.rs index 5ad6ae5f971..3bd05ba6204 100644 --- a/rs/pocket_ic_server/src/pocket_ic.rs +++ b/rs/pocket_ic_server/src/pocket_ic.rs @@ -2567,226 +2567,3 @@ fn new_canister_http_adapter( CanisterHttp::new(config, log, metrics_registry) } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn state_label_test() { - // State label changes. - let pic = PocketIc::default(); - let state0 = pic.get_state_label(); - let canister_id = pic.any_subnet().create_canister(None); - pic.any_subnet().add_cycles(canister_id, 2_000_000_000_000); - let state1 = pic.get_state_label(); - pic.any_subnet().stop_canister(canister_id).unwrap(); - pic.any_subnet().delete_canister(canister_id).unwrap(); - let state2 = pic.get_state_label(); - - assert_ne!(state0, state1); - assert_ne!(state1, state2); - assert_ne!(state0, state2); - } - - #[test] - fn test_time() { - let mut pic = PocketIc::default(); - - let unix_time_ns = 1640995200000000000; // 1st Jan 2022 - let time = Time::from_nanos_since_unix_epoch(unix_time_ns); - compute_assert_state_change(&mut pic, SetTime { time }); - let actual_time = compute_assert_state_immutable(&mut pic, GetTime {}); - - match actual_time { - OpOut::Time(actual_time_ns) => assert_eq!(unix_time_ns, actual_time_ns), - _ => panic!("Unexpected OpOut: {:?}", actual_time), - }; - } - - #[test] - fn test_execute_message() { - let (mut pic, canister_id) = new_pic_counter_installed(); - let amount: u128 = 20_000_000_000_000; - let add_cycles = AddCycles { - canister_id, - amount, - }; - add_cycles.compute(&mut pic); - - let update = ExecuteIngressMessage(CanisterCall { - sender: PrincipalId::new_anonymous(), - canister_id, - method: "write".into(), - payload: vec![], - effective_principal: EffectivePrincipal::None, - }); - - compute_assert_state_change(&mut pic, update); - } - - #[test] - fn test_cycles_burn_app_subnet() { - let (mut pic, canister_id) = new_pic_counter_installed(); - let (_, update) = query_update_constructors(canister_id); - let cycles_balance = GetCyclesBalance { canister_id }; - let OpOut::Cycles(initial_balance) = - compute_assert_state_immutable(&mut pic, cycles_balance.clone()) - else { - unreachable!() - }; - compute_assert_state_change(&mut pic, update("write")); - let OpOut::Cycles(new_balance) = compute_assert_state_immutable(&mut pic, cycles_balance) - else { - unreachable!() - }; - assert_ne!(initial_balance, new_balance); - } - - #[test] - fn test_cycles_burn_system_subnet() { - let (mut pic, canister_id) = new_pic_counter_installed_system_subnet(); - let (_, update) = query_update_constructors(canister_id); - - let cycles_balance = GetCyclesBalance { canister_id }; - let OpOut::Cycles(initial_balance) = - compute_assert_state_immutable(&mut pic, cycles_balance.clone()) - else { - unreachable!() - }; - compute_assert_state_change(&mut pic, update("write")); - let OpOut::Cycles(new_balance) = compute_assert_state_immutable(&mut pic, cycles_balance) - else { - unreachable!() - }; - assert_eq!(initial_balance, new_balance); - } - - fn query_update_constructors( - canister_id: CanisterId, - ) -> ( - impl Fn(&str) -> Query, - impl Fn(&str) -> ExecuteIngressMessage, - ) { - let call = move |method: &str| CanisterCall { - sender: PrincipalId::new_anonymous(), - canister_id, - method: method.into(), - payload: vec![], - effective_principal: EffectivePrincipal::None, - }; - - let update = move |m: &str| ExecuteIngressMessage(call(m)); - let query = move |m: &str| Query(call(m)); - - (query, update) - } - - fn new_pic_counter_installed() -> (PocketIc, CanisterId) { - let mut pic = PocketIc::default(); - let canister_id = pic.any_subnet().create_canister(None); - - let amount: u128 = 20_000_000_000_000; - let add_cycles = AddCycles { - canister_id, - amount, - }; - add_cycles.compute(&mut pic); - - let module = counter_wasm(); - let install_op = InstallCanisterAsController { - canister_id, - mode: CanisterInstallMode::Install, - module, - payload: vec![], - }; - - compute_assert_state_change(&mut pic, install_op); - - (pic, canister_id) - } - - fn new_pic_counter_installed_system_subnet() -> (PocketIc, CanisterId) { - let mut pic = PocketIc::new( - Runtime::new().unwrap().into(), - 0, - ExtendedSubnetConfigSet { - ii: Some(SubnetSpec::default()), - ..Default::default() - }, - None, - false, - None, - None, - ); - let canister_id = pic.any_subnet().create_canister(None); - - let module = counter_wasm(); - let install_op = InstallCanisterAsController { - canister_id, - mode: CanisterInstallMode::Install, - module, - payload: vec![], - }; - - compute_assert_state_change(&mut pic, install_op); - - (pic, canister_id) - } - - fn compute_assert_state_change(pic: &mut PocketIc, op: impl Operation) -> OpOut { - let state0 = pic.get_state_label(); - let res = op.compute(pic); - let state1 = pic.get_state_label(); - assert_ne!(state0, state1); - res - } - - fn compute_assert_state_immutable(pic: &mut PocketIc, op: impl Operation) -> OpOut { - let state0 = pic.get_state_label(); - let res = op.compute(pic); - let state1 = pic.get_state_label(); - assert_eq!(state0, state1); - res - } - - fn counter_wasm() -> Vec { - wat::parse_str(COUNTER_WAT).unwrap().as_slice().to_vec() - } - - const COUNTER_WAT: &str = r#" -;; Counter with global variable ;; -(module - (import "ic0" "msg_reply" (func $msg_reply)) - (import "ic0" "msg_reply_data_append" - (func $msg_reply_data_append (param i32 i32))) - - (func $read - (i32.store - (i32.const 0) - (global.get 0) - ) - (call $msg_reply_data_append - (i32.const 0) - (i32.const 4)) - (call $msg_reply)) - - (func $write - (global.set 0 - (i32.add - (global.get 0) - (i32.const 1) - ) - ) - (call $read) - ) - - (memory $memory 1) - (export "memory" (memory $memory)) - (global (export "counter_global") (mut i32) (i32.const 0)) - (export "canister_query read" (func $read)) - (export "canister_query inc_read" (func $write)) - (export "canister_update write" (func $write)) -) - "#; -} From 7f6b81257f95cd0456da94488ea0f31e88503d43 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Tue, 22 Oct 2024 12:05:10 +0200 Subject: [PATCH 03/43] seq numbers --- rs/pocket_ic_server/src/pocket_ic.rs | 59 ++++++++++++++++++---- rs/pocket_ic_server/src/state_api/state.rs | 22 ++++---- 2 files changed, 62 insertions(+), 19 deletions(-) diff --git a/rs/pocket_ic_server/src/pocket_ic.rs b/rs/pocket_ic_server/src/pocket_ic.rs index 1d69f7240b8..e210464b77f 100644 --- a/rs/pocket_ic_server/src/pocket_ic.rs +++ b/rs/pocket_ic_server/src/pocket_ic.rs @@ -70,8 +70,6 @@ use pocket_ic::common::rest::{ RawCanisterCall, RawCanisterId, RawEffectivePrincipal, RawMessageId, RawSetStableMemory, SubnetInstructionConfig, SubnetKind, SubnetSpec, Topology, }; -use rand::rngs::StdRng; -use rand::SeedableRng; use serde::{Deserialize, Serialize}; use std::hash::Hash; use std::str::FromStr; @@ -232,9 +230,6 @@ pub struct PocketIc { routing_table: RoutingTable, /// Created on initialization and updated if a new subnet is created. topology: TopologyInternal, - /// Used for choosing a random state label for this instance. - /// This value is seeded for the sake of reproducibility. - randomness: StdRng, state_label: StateLabel, range_gen: RangeGen, registry_data_provider: Arc, @@ -691,8 +686,7 @@ impl PocketIc { default_effective_canister_id, }; - let mut randomness = StdRng::seed_from_u64(instance_id as u64); - let state_label = StateLabel::new(&mut randomness); + let state_label = StateLabel::new(instance_id); Self { state_dir, @@ -701,7 +695,6 @@ impl PocketIc { routing_table, topology, state_label, - randomness, range_gen, registry_data_provider, runtime, @@ -712,8 +705,8 @@ impl PocketIc { } } - pub(crate) fn refresh_state_label(&mut self) { - self.state_label = StateLabel::new(&mut self.randomness); + pub(crate) fn bump_state_label(&mut self) { + self.state_label.bump(); } fn try_route_canister(&self, canister_id: CanisterId) -> Option> { @@ -2568,3 +2561,49 @@ fn new_canister_http_adapter( CanisterHttp::new(config, log, metrics_registry) } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn state_label_test() { + // State label changes. + let mut pic0 = PocketIc::new( + Runtime::new().unwrap().into(), + 0, + ExtendedSubnetConfigSet { + application: vec![SubnetSpec::default()], + ..Default::default() + }, + None, + false, + None, + None, + ); + let mut pic1 = PocketIc::new( + Runtime::new().unwrap().into(), + 1, + ExtendedSubnetConfigSet { + application: vec![SubnetSpec::default()], + ..Default::default() + }, + None, + false, + None, + None, + ); + assert_ne!(pic0.get_state_label(), pic1.get_state_label()); + + let pic0_state_label = pic0.get_state_label(); + pic0.bump_state_label(); + assert_ne!(pic0.get_state_label(), pic0_state_label); + assert_ne!(pic0.get_state_label(), pic1.get_state_label()); + + let pic1_state_label = pic1.get_state_label(); + pic1.bump_state_label(); + assert_ne!(pic1.get_state_label(), pic0_state_label); + assert_ne!(pic1.get_state_label(), pic1_state_label); + assert_ne!(pic1.get_state_label(), pic0.get_state_label()); + } +} diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index f28f47ad5b7..107f9a019dd 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -51,8 +51,6 @@ use pocket_ic::common::rest::{ HttpGatewayDetails, HttpGatewayInfo, MockCanisterHttpResponse, Topology, }; use pocket_ic::{ErrorCode, UserError, WasmResult}; -use rand::rngs::StdRng; -use rand::RngCore; use serde::{Deserialize, Serialize}; use std::{collections::HashMap, fmt, path::PathBuf, str::FromStr, sync::Arc, time::Duration}; use tokio::{ @@ -76,17 +74,23 @@ const MIN_OPERATION_DELAY: Duration = Duration::from_millis(100); // The minimum delay between consecutive attempts to read the graph in auto progress mode. const READ_GRAPH_DELAY: Duration = Duration::from_millis(100); -pub const STATE_LABEL_HASH_SIZE: usize = 32; +pub const STATE_LABEL_HASH_SIZE: usize = 16; /// Uniquely identifies a state. -#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Default, Deserialize)] +#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Default)] pub struct StateLabel(pub [u8; STATE_LABEL_HASH_SIZE]); impl StateLabel { - pub fn new(rng: &mut StdRng) -> Self { - let mut state_label = [42; STATE_LABEL_HASH_SIZE]; - rng.fill_bytes(&mut state_label); - Self(state_label) + pub fn new(instance_id: InstanceId) -> Self { + let mut seq_no: u128 = instance_id.try_into().unwrap(); + seq_no <<= 64; + Self(seq_no.to_le_bytes()) + } + + pub fn bump(&mut self) { + let mut seq_no: u128 = u128::from_le_bytes(self.0); + seq_no += 1; + self.0 = seq_no.to_le_bytes(); } } @@ -1409,7 +1413,7 @@ impl ApiState { op_id.0, ); let result = op.compute(&mut pocket_ic); - pocket_ic.refresh_state_label(); + pocket_ic.bump_state_label(); let new_state_label = pocket_ic.get_state_label(); // add result to graph, but grab instance lock first! let instances = instances.blocking_read(); From eeee33ee9a374f2ca00d5c7263cc048b979e6d26 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Tue, 22 Oct 2024 12:22:53 +0200 Subject: [PATCH 04/43] fix delete_instance --- rs/pocket_ic_server/src/state_api/state.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index 23e3176f026..d138f17446e 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -747,9 +747,9 @@ impl ApiState { loop { let instances = self.instances.read().await; let mut instance = instances[instance_id].lock().await; - match std::mem::replace(&mut instance.state, InstanceState::Deleted) { - InstanceState::Available(pocket_ic) => { - std::mem::drop(pocket_ic); + match &instance.state { + InstanceState::Available(_) => { + let _ = std::mem::replace(&mut instance.state, InstanceState::Deleted); break; } InstanceState::Deleted => { From 0877d6c642f494444cf557f4589458ad51101dd0 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Tue, 22 Oct 2024 13:32:54 +0200 Subject: [PATCH 05/43] trace --- .github/workflows/ci-main.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/.github/workflows/ci-main.yml b/.github/workflows/ci-main.yml index 4d36c6574f7..db7212b2c2e 100644 --- a/.github/workflows/ci-main.yml +++ b/.github/workflows/ci-main.yml @@ -79,6 +79,8 @@ jobs: BAZEL_EXTRA_ARGS+=" --keep_going" fi + BAZEL_EXTRA_ARGS+=" --test_env=RUST_LOG=trace" + # Export BAZEL_EXTRA_ARGS to environment echo "BAZEL_EXTRA_ARGS=$BAZEL_EXTRA_ARGS" >> $GITHUB_ENV - name: Run Bazel Test All From c86fe528bf36bac0cfb60e8c594cc10cc46bbb34 Mon Sep 17 00:00:00 2001 From: IDX GitHub Automation Date: Tue, 22 Oct 2024 11:33:14 +0000 Subject: [PATCH 06/43] IDX GitHub Automation --- .github/workflows/ci-main.yml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.github/workflows/ci-main.yml b/.github/workflows/ci-main.yml index db7212b2c2e..4d36c6574f7 100644 --- a/.github/workflows/ci-main.yml +++ b/.github/workflows/ci-main.yml @@ -79,8 +79,6 @@ jobs: BAZEL_EXTRA_ARGS+=" --keep_going" fi - BAZEL_EXTRA_ARGS+=" --test_env=RUST_LOG=trace" - # Export BAZEL_EXTRA_ARGS to environment echo "BAZEL_EXTRA_ARGS=$BAZEL_EXTRA_ARGS" >> $GITHUB_ENV - name: Run Bazel Test All From 53f42827f6f651be5eb0d9ea80dd53e807fcc8a9 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Tue, 22 Oct 2024 13:42:53 +0200 Subject: [PATCH 07/43] trace --- .github/workflows/ci-main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-main.yml b/.github/workflows/ci-main.yml index 4d36c6574f7..c250f2b0d09 100644 --- a/.github/workflows/ci-main.yml +++ b/.github/workflows/ci-main.yml @@ -92,7 +92,7 @@ jobs: with: BAZEL_COMMAND: "test" BAZEL_TARGETS: "//..." - BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel" + BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel --test_env=RUST_LOG=trace" # check if PR title contains release and set timeout filters accordingly BAZEL_EXTRA_ARGS: ${{ env.BAZEL_EXTRA_ARGS }} BUILDEVENT_APIKEY: ${{ secrets.HONEYCOMB_API_TOKEN }} From 2a46400df58a33ddc9584127c26c167a19455605 Mon Sep 17 00:00:00 2001 From: IDX GitHub Automation Date: Tue, 22 Oct 2024 11:43:25 +0000 Subject: [PATCH 08/43] IDX GitHub Automation --- .github/workflows/ci-main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-main.yml b/.github/workflows/ci-main.yml index c250f2b0d09..4d36c6574f7 100644 --- a/.github/workflows/ci-main.yml +++ b/.github/workflows/ci-main.yml @@ -92,7 +92,7 @@ jobs: with: BAZEL_COMMAND: "test" BAZEL_TARGETS: "//..." - BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel --test_env=RUST_LOG=trace" + BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel" # check if PR title contains release and set timeout filters accordingly BAZEL_EXTRA_ARGS: ${{ env.BAZEL_EXTRA_ARGS }} BUILDEVENT_APIKEY: ${{ secrets.HONEYCOMB_API_TOKEN }} From 49e0f0b95b9f25e921794825f404d3951121a495 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Tue, 22 Oct 2024 17:32:04 +0200 Subject: [PATCH 09/43] short max request time --- packages/pocket-ic/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pocket-ic/src/lib.rs b/packages/pocket-ic/src/lib.rs index 50407153ceb..5da1df366d2 100644 --- a/packages/pocket-ic/src/lib.rs +++ b/packages/pocket-ic/src/lib.rs @@ -74,7 +74,7 @@ pub mod nonblocking; const EXPECTED_SERVER_VERSION: &str = "pocket-ic-server 6.0.0"; // the default timeout of a PocketIC operation -const DEFAULT_MAX_REQUEST_TIME_MS: u64 = 300_000; +const DEFAULT_MAX_REQUEST_TIME_MS: u64 = 30_000; const LOCALHOST: &str = "127.0.0.1"; From 8c104a2d4e790483e4d07d0664f3d5872432d29d Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 07:03:15 +0200 Subject: [PATCH 10/43] Revert "short max request time" This reverts commit 49e0f0b95b9f25e921794825f404d3951121a495. --- packages/pocket-ic/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pocket-ic/src/lib.rs b/packages/pocket-ic/src/lib.rs index 5da1df366d2..50407153ceb 100644 --- a/packages/pocket-ic/src/lib.rs +++ b/packages/pocket-ic/src/lib.rs @@ -74,7 +74,7 @@ pub mod nonblocking; const EXPECTED_SERVER_VERSION: &str = "pocket-ic-server 6.0.0"; // the default timeout of a PocketIC operation -const DEFAULT_MAX_REQUEST_TIME_MS: u64 = 30_000; +const DEFAULT_MAX_REQUEST_TIME_MS: u64 = 300_000; const LOCALHOST: &str = "127.0.0.1"; From 8a16accea00d0e6d10da2d9b627980a5f39c7bd2 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 07:08:51 +0200 Subject: [PATCH 11/43] ci-pic.yml --- .github/workflows/ci-pic.yml | 98 ++++++++++++++++++++++++++++++++++++ 1 file changed, 98 insertions(+) create mode 100644 .github/workflows/ci-pic.yml diff --git a/.github/workflows/ci-pic.yml b/.github/workflows/ci-pic.yml new file mode 100644 index 00000000000..5466f47fa55 --- /dev/null +++ b/.github/workflows/ci-pic.yml @@ -0,0 +1,98 @@ +name: CI PocketIC +on: + merge_group: + # Allows you to run this workflow manually from the Actions tab + workflow_dispatch: + push: + branches: + - master + - 'dev-gh-*' + pull_request: + branches-ignore: + - hotfix-* # This is to ensure that this workflow is not triggered twice on ic-private, as it's already triggered from release-testing + # Used as reusable workflow within release-testing workflow + workflow_call: +# runs for the same workflow are cancelled on PRs but not on master +# explanation: on push to master head_ref is not set, so we want it to fall back to run_id so it is not cancelled +concurrency: + group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + cancel-in-progress: true +env: + CI_COMMIT_SHA: ${{ github.sha }} + CI_JOB_NAME: ${{ github.job }} + CI_JOB_URL: "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" + CI_PIPELINE_SOURCE: ${{ github.event_name }} + CI_PROJECT_DIR: ${{ github.workspace }} + BRANCH_NAME: ${{ github.head_ref || github.ref_name }} + CI_RUN_ID: ${{ github.run_id }} + RUSTFLAGS: "--remap-path-prefix=${CI_PROJECT_DIR}=/ic" + BUILDEVENT_DATASET: "github-ci-dfinity" +jobs: + pic-test-all: + name: PocketIC Test + container: + image: ghcr.io/dfinity/ic-build@sha256:ced9611af39119f62feedc8a5ad75a8fac478cb43d2d56b623023766f6ce8ca7 + options: >- + -e NODE_NAME --privileged --cgroupns host -v /cache:/cache -v /var/sysimage:/var/sysimage -v /var/tmp:/var/tmp -v /ceph-s3-info:/ceph-s3-info + timeout-minutes: 90 + runs-on: + group: zh1 + labels: dind-large + steps: + - name: Checkout + uses: actions/checkout@v4 + with: + fetch-depth: ${{ github.event_name == 'pull_request' && 256 || 0 }} + - name: Before script + id: before-script + shell: bash + run: | + [ -n "${NODE_NAME:-}" ] && echo "Node: $NODE_NAME" + - name: Login to Dockerhub + shell: bash + run: ./ci/scripts/docker-login.sh + env: + DOCKER_HUB_USER: ${{ vars.DOCKER_HUB_USER }} + DOCKER_HUB_PASSWORD_RO: ${{ secrets.DOCKER_HUB_PASSWORD_RO }} + - name: Set BAZEL_EXTRA_ARGS + shell: bash + run: | + set -xeuo pipefail + + # Determine which tests to skip and append 'long_test' for pull requests or merge groups + EXCLUDED_TEST_TAGS=(system_test_hourly system_test_nightly system_test_nightly_nns system_test_staging system_test_hotfix system_test_benchmark fuzz_test fi_tests_nightly nns_tests_nightly) + [[ "${{ github.event_name }}" =~ ^(pull_request|merge_group)$ ]] && EXCLUDED_TEST_TAGS+=(long_test) + + # Export excluded tags as environment variable for ci/bazel-scripts/diff.sh + echo "EXCLUDED_TEST_TAGS=${EXCLUDED_TEST_TAGS[*]}" >> $GITHUB_ENV + + # Prepend tags with '-' and join them with commas for Bazel + TEST_TAG_FILTERS=$(IFS=,; echo "${EXCLUDED_TEST_TAGS[*]/#/-}") + + # Determine BAZEL_EXTRA_ARGS based on event type or branch name + BAZEL_EXTRA_ARGS="--test_tag_filters=$TEST_TAG_FILTERS" + if [[ "${{ github.event_name }}" == 'merge_group' ]]; then + BAZEL_EXTRA_ARGS+=" --test_timeout_filters=short,moderate --flaky_test_attempts=3" + elif [[ $BRANCH_NAME =~ ^hotfix-.* ]]; then + BAZEL_EXTRA_ARGS+=" --test_timeout_filters=short,moderate" + else + BAZEL_EXTRA_ARGS+=" --keep_going" + fi + + # Export BAZEL_EXTRA_ARGS to environment + echo "BAZEL_EXTRA_ARGS=$BAZEL_EXTRA_ARGS" >> $GITHUB_ENV + - name: Run Bazel Test All + id: bazel-test-all + uses: ./.github/actions/bazel-test-all/ + env: + AWS_SHARED_CREDENTIALS_CONTENT: ${{ secrets.AWS_SHARED_CREDENTIALS_FILE }} + # Only run ci/bazel-scripts/diff.sh on PRs that are not labeled with "CI_ALL_BAZEL_TARGETS". + RUN_ON_DIFF_ONLY: ${{ github.event_name == 'pull_request' && !contains(github.event.pull_request.labels.*.name, 'CI_ALL_BAZEL_TARGETS') }} + OVERRIDE_DIDC_CHECK: ${{ contains(github.event.pull_request.labels.*.name, 'CI_OVERRIDE_DIDC_CHECK') }} + with: + BAZEL_COMMAND: "test" + BAZEL_TARGETS: "//packages/pocket-ic:test" + BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel --test_arg='--nocapture' --test_output=streamed" + # check if PR title contains release and set timeout filters accordingly + BAZEL_EXTRA_ARGS: ${{ env.BAZEL_EXTRA_ARGS }} + BUILDEVENT_APIKEY: ${{ secrets.HONEYCOMB_API_TOKEN }} From 69ef8893193ef6cc976cc47b854344cf38238ca5 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 07:26:10 +0200 Subject: [PATCH 12/43] ci --- .github/workflows/ci-pic.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-pic.yml b/.github/workflows/ci-pic.yml index 5466f47fa55..7e24a11567b 100644 --- a/.github/workflows/ci-pic.yml +++ b/.github/workflows/ci-pic.yml @@ -92,7 +92,7 @@ jobs: with: BAZEL_COMMAND: "test" BAZEL_TARGETS: "//packages/pocket-ic:test" - BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel --test_arg='--nocapture' --test_output=streamed" + BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel --test_output=streamed" # check if PR title contains release and set timeout filters accordingly BAZEL_EXTRA_ARGS: ${{ env.BAZEL_EXTRA_ARGS }} BUILDEVENT_APIKEY: ${{ secrets.HONEYCOMB_API_TOKEN }} From 21dbd61f39285d7eeb61b8714b2b52b3779b2f93 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 07:26:30 +0200 Subject: [PATCH 13/43] ci --- .github/actions/bazel-test-all/action.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/actions/bazel-test-all/action.yaml b/.github/actions/bazel-test-all/action.yaml index b7b3f27b35f..783d76ee8a9 100644 --- a/.github/actions/bazel-test-all/action.yaml +++ b/.github/actions/bazel-test-all/action.yaml @@ -6,10 +6,10 @@ inputs: default: 'test' BAZEL_TARGETS: required: false - default: '//...' + default: '//packages/pocket-ic:test' BAZEL_CI_CONFIG: required: false - default: '--config=ci' + default: '--config=ci --test_output=streamed' BAZEL_EXTRA_ARGS: required: false default: '' From 92ed177f8c712f68e43fabfa00410ba46ec4dcca Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 07:27:04 +0200 Subject: [PATCH 14/43] ci --- .github/actions/bazel-test-all/action.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/actions/bazel-test-all/action.yaml b/.github/actions/bazel-test-all/action.yaml index 783d76ee8a9..98712177b94 100644 --- a/.github/actions/bazel-test-all/action.yaml +++ b/.github/actions/bazel-test-all/action.yaml @@ -9,7 +9,7 @@ inputs: default: '//packages/pocket-ic:test' BAZEL_CI_CONFIG: required: false - default: '--config=ci --test_output=streamed' + default: '--config=ci --test_output=streamed --test_arg="--nocapture"' BAZEL_EXTRA_ARGS: required: false default: '' From 4e6a9783c7f866689539f088a56acd31f1b365d6 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 07:34:11 +0200 Subject: [PATCH 15/43] run_on_diff --- .github/actions/bazel-test-all/action.yaml | 4 ++-- .github/workflows/ci-pic.yml | 2 -- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/.github/actions/bazel-test-all/action.yaml b/.github/actions/bazel-test-all/action.yaml index 98712177b94..b7b3f27b35f 100644 --- a/.github/actions/bazel-test-all/action.yaml +++ b/.github/actions/bazel-test-all/action.yaml @@ -6,10 +6,10 @@ inputs: default: 'test' BAZEL_TARGETS: required: false - default: '//packages/pocket-ic:test' + default: '//...' BAZEL_CI_CONFIG: required: false - default: '--config=ci --test_output=streamed --test_arg="--nocapture"' + default: '--config=ci' BAZEL_EXTRA_ARGS: required: false default: '' diff --git a/.github/workflows/ci-pic.yml b/.github/workflows/ci-pic.yml index 7e24a11567b..d01c65dbb00 100644 --- a/.github/workflows/ci-pic.yml +++ b/.github/workflows/ci-pic.yml @@ -86,8 +86,6 @@ jobs: uses: ./.github/actions/bazel-test-all/ env: AWS_SHARED_CREDENTIALS_CONTENT: ${{ secrets.AWS_SHARED_CREDENTIALS_FILE }} - # Only run ci/bazel-scripts/diff.sh on PRs that are not labeled with "CI_ALL_BAZEL_TARGETS". - RUN_ON_DIFF_ONLY: ${{ github.event_name == 'pull_request' && !contains(github.event.pull_request.labels.*.name, 'CI_ALL_BAZEL_TARGETS') }} OVERRIDE_DIDC_CHECK: ${{ contains(github.event.pull_request.labels.*.name, 'CI_OVERRIDE_DIDC_CHECK') }} with: BAZEL_COMMAND: "test" From 24d7fa9cc9ead6c5cb7e7779c0505f74e970490a Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 07:37:17 +0200 Subject: [PATCH 16/43] ci --- .github/workflows/ci-pic.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-pic.yml b/.github/workflows/ci-pic.yml index d01c65dbb00..136c9b7fce8 100644 --- a/.github/workflows/ci-pic.yml +++ b/.github/workflows/ci-pic.yml @@ -90,7 +90,7 @@ jobs: with: BAZEL_COMMAND: "test" BAZEL_TARGETS: "//packages/pocket-ic:test" - BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel --test_output=streamed" + BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel --test_arg='--nocapture' --test_output=streamed" # check if PR title contains release and set timeout filters accordingly BAZEL_EXTRA_ARGS: ${{ env.BAZEL_EXTRA_ARGS }} BUILDEVENT_APIKEY: ${{ secrets.HONEYCOMB_API_TOKEN }} From d70960b04d058f120b76b363c7ef4360c2c92cf0 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 07:40:27 +0200 Subject: [PATCH 17/43] ci --- .github/workflows/ci-pic.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-pic.yml b/.github/workflows/ci-pic.yml index 136c9b7fce8..10c3f4356e1 100644 --- a/.github/workflows/ci-pic.yml +++ b/.github/workflows/ci-pic.yml @@ -90,7 +90,7 @@ jobs: with: BAZEL_COMMAND: "test" BAZEL_TARGETS: "//packages/pocket-ic:test" - BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel --test_arg='--nocapture' --test_output=streamed" + BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel --test_arg='--nocapture' --test_output=streamed --cache_test_results=no" # check if PR title contains release and set timeout filters accordingly BAZEL_EXTRA_ARGS: ${{ env.BAZEL_EXTRA_ARGS }} BUILDEVENT_APIKEY: ${{ secrets.HONEYCOMB_API_TOKEN }} From 720a2608362c1d6e9be6996bff1c607216178fd0 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 07:46:15 +0200 Subject: [PATCH 18/43] ci --- .github/workflows/ci-pic.yml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci-pic.yml b/.github/workflows/ci-pic.yml index 10c3f4356e1..73685d31515 100644 --- a/.github/workflows/ci-pic.yml +++ b/.github/workflows/ci-pic.yml @@ -78,6 +78,7 @@ jobs: else BAZEL_EXTRA_ARGS+=" --keep_going" fi + BAZEL_EXTRA_ARGS+=" --test_arg='--nocapture'" # Export BAZEL_EXTRA_ARGS to environment echo "BAZEL_EXTRA_ARGS=$BAZEL_EXTRA_ARGS" >> $GITHUB_ENV @@ -90,7 +91,7 @@ jobs: with: BAZEL_COMMAND: "test" BAZEL_TARGETS: "//packages/pocket-ic:test" - BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel --test_arg='--nocapture' --test_output=streamed --cache_test_results=no" + BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel --test_output=streamed --cache_test_results=no" # check if PR title contains release and set timeout filters accordingly BAZEL_EXTRA_ARGS: ${{ env.BAZEL_EXTRA_ARGS }} BUILDEVENT_APIKEY: ${{ secrets.HONEYCOMB_API_TOKEN }} From 4a65cf2cd5e7fc8eca396c681c3bd37c7f926ff9 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 07:53:57 +0200 Subject: [PATCH 19/43] ci --- .github/workflows/ci-pic.yml | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/ci-pic.yml b/.github/workflows/ci-pic.yml index 73685d31515..59ae60e9c4d 100644 --- a/.github/workflows/ci-pic.yml +++ b/.github/workflows/ci-pic.yml @@ -78,7 +78,6 @@ jobs: else BAZEL_EXTRA_ARGS+=" --keep_going" fi - BAZEL_EXTRA_ARGS+=" --test_arg='--nocapture'" # Export BAZEL_EXTRA_ARGS to environment echo "BAZEL_EXTRA_ARGS=$BAZEL_EXTRA_ARGS" >> $GITHUB_ENV @@ -91,7 +90,7 @@ jobs: with: BAZEL_COMMAND: "test" BAZEL_TARGETS: "//packages/pocket-ic:test" - BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel --test_output=streamed --cache_test_results=no" + BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel --test_output=streamed --cache_test_results=no --test_arg='--nocapture'" # check if PR title contains release and set timeout filters accordingly BAZEL_EXTRA_ARGS: ${{ env.BAZEL_EXTRA_ARGS }} BUILDEVENT_APIKEY: ${{ secrets.HONEYCOMB_API_TOKEN }} From d1f7e94c1fcbb7fe810dbfa5084d3cba88fdb7f9 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 07:56:46 +0200 Subject: [PATCH 20/43] ci --- .github/workflows/ci-pic.yml | 41 ++---------------------------------- 1 file changed, 2 insertions(+), 39 deletions(-) diff --git a/.github/workflows/ci-pic.yml b/.github/workflows/ci-pic.yml index 59ae60e9c4d..c3e92f2b7e1 100644 --- a/.github/workflows/ci-pic.yml +++ b/.github/workflows/ci-pic.yml @@ -54,43 +54,6 @@ jobs: env: DOCKER_HUB_USER: ${{ vars.DOCKER_HUB_USER }} DOCKER_HUB_PASSWORD_RO: ${{ secrets.DOCKER_HUB_PASSWORD_RO }} - - name: Set BAZEL_EXTRA_ARGS - shell: bash - run: | - set -xeuo pipefail - - # Determine which tests to skip and append 'long_test' for pull requests or merge groups - EXCLUDED_TEST_TAGS=(system_test_hourly system_test_nightly system_test_nightly_nns system_test_staging system_test_hotfix system_test_benchmark fuzz_test fi_tests_nightly nns_tests_nightly) - [[ "${{ github.event_name }}" =~ ^(pull_request|merge_group)$ ]] && EXCLUDED_TEST_TAGS+=(long_test) - - # Export excluded tags as environment variable for ci/bazel-scripts/diff.sh - echo "EXCLUDED_TEST_TAGS=${EXCLUDED_TEST_TAGS[*]}" >> $GITHUB_ENV - - # Prepend tags with '-' and join them with commas for Bazel - TEST_TAG_FILTERS=$(IFS=,; echo "${EXCLUDED_TEST_TAGS[*]/#/-}") - - # Determine BAZEL_EXTRA_ARGS based on event type or branch name - BAZEL_EXTRA_ARGS="--test_tag_filters=$TEST_TAG_FILTERS" - if [[ "${{ github.event_name }}" == 'merge_group' ]]; then - BAZEL_EXTRA_ARGS+=" --test_timeout_filters=short,moderate --flaky_test_attempts=3" - elif [[ $BRANCH_NAME =~ ^hotfix-.* ]]; then - BAZEL_EXTRA_ARGS+=" --test_timeout_filters=short,moderate" - else - BAZEL_EXTRA_ARGS+=" --keep_going" - fi - - # Export BAZEL_EXTRA_ARGS to environment - echo "BAZEL_EXTRA_ARGS=$BAZEL_EXTRA_ARGS" >> $GITHUB_ENV - name: Run Bazel Test All - id: bazel-test-all - uses: ./.github/actions/bazel-test-all/ - env: - AWS_SHARED_CREDENTIALS_CONTENT: ${{ secrets.AWS_SHARED_CREDENTIALS_FILE }} - OVERRIDE_DIDC_CHECK: ${{ contains(github.event.pull_request.labels.*.name, 'CI_OVERRIDE_DIDC_CHECK') }} - with: - BAZEL_COMMAND: "test" - BAZEL_TARGETS: "//packages/pocket-ic:test" - BAZEL_CI_CONFIG: "--config=ci --repository_cache=/cache/bazel --test_output=streamed --cache_test_results=no --test_arg='--nocapture'" - # check if PR title contains release and set timeout filters accordingly - BAZEL_EXTRA_ARGS: ${{ env.BAZEL_EXTRA_ARGS }} - BUILDEVENT_APIKEY: ${{ secrets.HONEYCOMB_API_TOKEN }} + run: | + bazel test //packages/pocket-ic:test --cache_test_results=no --test_output=all --test_arg="--nocapture" From e503a56c838e2e2e8cccc52a973c474a1366236c Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 08:00:56 +0200 Subject: [PATCH 21/43] ci --- .github/workflows/ci-pic.yml | 27 ++------------------------- 1 file changed, 2 insertions(+), 25 deletions(-) diff --git a/.github/workflows/ci-pic.yml b/.github/workflows/ci-pic.yml index c3e92f2b7e1..854a9857269 100644 --- a/.github/workflows/ci-pic.yml +++ b/.github/workflows/ci-pic.yml @@ -1,32 +1,9 @@ name: CI PocketIC on: - merge_group: - # Allows you to run this workflow manually from the Actions tab - workflow_dispatch: - push: - branches: - - master - - 'dev-gh-*' pull_request: - branches-ignore: - - hotfix-* # This is to ensure that this workflow is not triggered twice on ic-private, as it's already triggered from release-testing - # Used as reusable workflow within release-testing workflow - workflow_call: -# runs for the same workflow are cancelled on PRs but not on master -# explanation: on push to master head_ref is not set, so we want it to fall back to run_id so it is not cancelled concurrency: - group: ${{ github.workflow }}-${{ github.head_ref || github.run_id }} + group: ${{ github.workflow }}-${{ github.head_ref}} cancel-in-progress: true -env: - CI_COMMIT_SHA: ${{ github.sha }} - CI_JOB_NAME: ${{ github.job }} - CI_JOB_URL: "${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}" - CI_PIPELINE_SOURCE: ${{ github.event_name }} - CI_PROJECT_DIR: ${{ github.workspace }} - BRANCH_NAME: ${{ github.head_ref || github.ref_name }} - CI_RUN_ID: ${{ github.run_id }} - RUSTFLAGS: "--remap-path-prefix=${CI_PROJECT_DIR}=/ic" - BUILDEVENT_DATASET: "github-ci-dfinity" jobs: pic-test-all: name: PocketIC Test @@ -56,4 +33,4 @@ jobs: DOCKER_HUB_PASSWORD_RO: ${{ secrets.DOCKER_HUB_PASSWORD_RO }} - name: Run Bazel Test All run: | - bazel test //packages/pocket-ic:test --cache_test_results=no --test_output=all --test_arg="--nocapture" + bazel test //packages/pocket-ic:test --cache_test_results=no --test_output=streamed --test_arg="--nocapture" From db5e8115af433c96793de1e2342a10d95d105a2c Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 08:07:47 +0200 Subject: [PATCH 22/43] ci --- rs/pocket_ic_server/src/state_api/state.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index d138f17446e..522190fc6ab 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -1295,8 +1295,11 @@ impl ApiState { let instances = self.instances.read().await; let mut res = vec![]; + println!("total instances: {}", instances.len()); for instance in &*instances { + println!("grabbing instance"); let instance = &*instance.lock().await; + println!("grabbed instance"); match &instance.state { InstanceState::Busy { state_label, op_id } => { res.push(format!("Busy({:?}, {:?})", state_label, op_id)) From b72de11cfadaa691ae7d4f009b9d01be10092072 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 08:15:31 +0200 Subject: [PATCH 23/43] ci --- .github/workflows/ci-pic.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci-pic.yml b/.github/workflows/ci-pic.yml index 854a9857269..c862683f04d 100644 --- a/.github/workflows/ci-pic.yml +++ b/.github/workflows/ci-pic.yml @@ -33,4 +33,4 @@ jobs: DOCKER_HUB_PASSWORD_RO: ${{ secrets.DOCKER_HUB_PASSWORD_RO }} - name: Run Bazel Test All run: | - bazel test //packages/pocket-ic:test --cache_test_results=no --test_output=streamed --test_arg="--nocapture" + bazel test //packages/pocket-ic:test --cache_test_results=no --test_output=streamed --test_arg="--nocapture" --test_env=RUST_LOG=trace From bc3058b0f934d9789513bc1a23c84abc31fdc966 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 08:28:23 +0200 Subject: [PATCH 24/43] polling --- packages/pocket-ic/src/nonblocking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pocket-ic/src/nonblocking.rs b/packages/pocket-ic/src/nonblocking.rs index 3d52bb811e3..6440c8fb099 100644 --- a/packages/pocket-ic/src/nonblocking.rs +++ b/packages/pocket-ic/src/nonblocking.rs @@ -35,7 +35,7 @@ use tracing_appender::non_blocking::WorkerGuard; use tracing_subscriber::EnvFilter; // wait time between polling requests -const POLLING_PERIOD_MS: u64 = 10; +const POLLING_PERIOD_MS: u64 = 500; const LOG_DIR_PATH_ENV_NAME: &str = "POCKET_IC_LOG_DIR"; const LOG_DIR_LEVELS_ENV_NAME: &str = "POCKET_IC_LOG_DIR_LEVELS"; From eeff6f736319d8713c8733b6fdccc74841243cf1 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 08:42:22 +0200 Subject: [PATCH 25/43] trace --- rs/pocket_ic_server/src/state_api/state.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index 522190fc6ab..d862a63c02c 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -1435,7 +1435,9 @@ impl ApiState { let new_state_label = pocket_ic.get_state_label(); // add result to graph, but grab instance lock first! let instances = instances.blocking_read(); + println!("waiting to grab graph: old={} op_id={}", old_state_label, op_id); let mut graph_guard = graph.blocking_write(); + println!("grabbed graph: old={} op_id={}", old_state_label, op_id); let cached_computations = graph_guard.entry(old_state_label.clone()).or_default(); cached_computations From f07d413bb9a0af16ade1a7fcbc108d0a56bba0ac Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 08:43:28 +0200 Subject: [PATCH 26/43] small --- packages/pocket-ic/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/pocket-ic/BUILD.bazel b/packages/pocket-ic/BUILD.bazel index 6a085ff5af5..d6b0948ceb8 100644 --- a/packages/pocket-ic/BUILD.bazel +++ b/packages/pocket-ic/BUILD.bazel @@ -46,7 +46,7 @@ rust_library( rust_test_suite( name = "test", - size = "medium", + size = "small", srcs = ["tests/tests.rs"], data = [ "//packages/pocket-ic/test_canister:test_canister.wasm", From 8027bf00a17614c3c6779dc4c4d752beb4cf528b Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 08:45:54 +0200 Subject: [PATCH 27/43] build --- rs/pocket_ic_server/src/state_api/state.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index d862a63c02c..08bc5c5efee 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -1435,9 +1435,9 @@ impl ApiState { let new_state_label = pocket_ic.get_state_label(); // add result to graph, but grab instance lock first! let instances = instances.blocking_read(); - println!("waiting to grab graph: old={} op_id={}", old_state_label, op_id); + println!("waiting to grab graph: old={:?} op_id={:?}", old_state_label, op_id); let mut graph_guard = graph.blocking_write(); - println!("grabbed graph: old={} op_id={}", old_state_label, op_id); + println!("grabbed graph: old={:?} op_id={:?}", old_state_label, op_id); let cached_computations = graph_guard.entry(old_state_label.clone()).or_default(); cached_computations From 2975019e4a5779dc86748bb25f1a1bd75a38a56c Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 08:51:39 +0200 Subject: [PATCH 28/43] trace --- rs/pocket_ic_server/src/state_api/state.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index 08bc5c5efee..87815365e91 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -1434,6 +1434,7 @@ impl ApiState { pocket_ic.bump_state_label(); let new_state_label = pocket_ic.get_state_label(); // add result to graph, but grab instance lock first! + println!("waiting to grab instane: old={:?} op_id={:?}", old_state_label, op_id); let instances = instances.blocking_read(); println!("waiting to grab graph: old={:?} op_id={:?}", old_state_label, op_id); let mut graph_guard = graph.blocking_write(); From 6f6e10c044572f755a9b8e5bdbd20efb73a64449 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 08:52:11 +0200 Subject: [PATCH 29/43] trace --- rs/pocket_ic_server/src/state_api/state.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index 87815365e91..75e66ceed0a 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -1445,6 +1445,7 @@ impl ApiState { .insert(op_id.clone(), (new_state_label, result.clone())); drop(graph_guard); let mut instance = instances[instance_id].blocking_lock(); + println!("grabbed instance: old={:?} op_id={:?}", old_state_label, op_id); if let InstanceState::Deleted = &instance.state { error!("The instance is deleted immediately after an operation. This is a bug!"); std::mem::drop(pocket_ic); From f57453caeb6c0b3b034620e1a0858b47c00c744a Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 08:59:46 +0200 Subject: [PATCH 30/43] trace --- rs/pocket_ic_server/src/state_api/state.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index 75e66ceed0a..0c85a991479 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -1389,7 +1389,9 @@ impl ApiState { let (bg_task, busy_outcome) = if let Some(instance_mutex) = instances_locked.get(instance_id) { + println!("waiting to grab instance mutex; instance_id={:?}, op_id={:?}", instance_id, op_id); let mut instance = instance_mutex.lock().await; + println!("grabbed instance mutex; instance_id={:?}, op_id={:?}", instance_id, op_id); // If this instance is busy, return the running op and initial state match &instance.state { InstanceState::Deleted => { From 8f069f81cacc2d47cb793c1e5a8796be498f32ab Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 11:05:31 +0200 Subject: [PATCH 31/43] try --- rs/pocket_ic_server/src/state_api/state.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index 0c85a991479..96a372ca48b 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -1521,6 +1521,7 @@ impl std::fmt::Debug for InstanceState { impl std::fmt::Debug for ApiState { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + /* let instances = self.instances.blocking_read(); let graph = self.graph.blocking_read(); @@ -1533,6 +1534,7 @@ impl std::fmt::Debug for ApiState { for (k, v) in graph.iter() { writeln!(f, " {k:?} => {v:?}")?; } + */ Ok(()) } } From a0a059ba98c8692c5c8e4ab92f718bd30c0798b6 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 11:09:04 +0200 Subject: [PATCH 32/43] try --- packages/pocket-ic/tests/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/pocket-ic/tests/tests.rs b/packages/pocket-ic/tests/tests.rs index a26aeb65de2..f6e4259704a 100644 --- a/packages/pocket-ic/tests/tests.rs +++ b/packages/pocket-ic/tests/tests.rs @@ -378,7 +378,7 @@ fn test_get_set_cycle_balance() { assert_eq!(balance, initial_balance + 420); } -#[test] +//#[test] fn test_create_and_drop_instances() { let pic = PocketIc::new(); let id = pic.instance_id(); @@ -622,7 +622,7 @@ fn test_too_large_call() { .unwrap_err(); } -#[tokio::test] +//#[tokio::test] async fn test_create_and_drop_instances_async() { let pic = pocket_ic::nonblocking::PocketIc::new().await; let id = pic.instance_id; From 93fd6fee8674c3b7aefee0e930fdfde19bcf77d4 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 11:23:07 +0200 Subject: [PATCH 33/43] dbg --- rs/pocket_ic_server/src/state_api/state.rs | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index 96a372ca48b..ba2d493705d 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -746,6 +746,7 @@ impl ApiState { self.stop_progress(instance_id).await; loop { let instances = self.instances.read().await; + trace!("grabbed instances in delete_instance; instance_id={}", instance_id); let mut instance = instances[instance_id].lock().await; match &instance.state { InstanceState::Available(_) => { @@ -759,6 +760,7 @@ impl ApiState { } drop(instance); drop(instances); + trace!("released instances in delete_instance; instance_id={}", instance_id); tokio::time::sleep(Duration::from_secs(1)).await; } } @@ -766,8 +768,10 @@ impl ApiState { pub async fn delete_all_instances(arc_self: Arc) { let mut tasks = JoinSet::new(); let instances = arc_self.instances.read().await; + trace!("delete_all_instances: grabbed instances"); let num_instances = instances.len(); drop(instances); + trace!("delete_all_instances: released instances"); for instance_id in 0..num_instances { let arc_self_clone = arc_self.clone(); tasks.spawn(async move { arc_self_clone.delete_instance(instance_id).await }); @@ -1169,6 +1173,7 @@ impl ApiState { let request_id = canister_http_request.request_id; let response = loop { let instances = instances.read().await; + trace!("process_canister_http_requests: grabbed instances; instance_id={}", instance_id); let instance = instances[instance_id].lock().await; if let InstanceState::Available(pocket_ic) = &instance.state { let canister_http_adapters = pocket_ic.canister_http_adapters(); @@ -1193,6 +1198,7 @@ impl ApiState { } drop(instance); drop(instances); + trace!("process_canister_http_requests: released instances; instance_id={}", instance_id); tokio::time::sleep(Duration::from_millis(10)).await; }; let mock_canister_http_response = MockCanisterHttpResponse { @@ -1228,6 +1234,7 @@ impl ApiState { let instances_clone = self.instances.clone(); let graph = self.graph.clone(); let instances = self.instances.read().await; + trace!("auto_progress: grabbed instances; instance_id={}", instance_id); let mut instance = instances[instance_id].lock().await; if instance.progress_thread.is_none() { let (tx, mut rx) = mpsc::channel::<()>(1); @@ -1272,19 +1279,24 @@ impl ApiState { } }); instance.progress_thread = Some(ProgressThread { handle, sender: tx }); + trace!("auto_progress: released instances; instance_id={}", instance_id); Ok(()) } else { + trace!("auto_progress: released instances; instance_id={}", instance_id); Err("Auto progress mode has already been enabled.".to_string()) } } pub async fn stop_progress(&self, instance_id: InstanceId) { let instances = self.instances.read().await; + trace!("stop_progress: grabbed instances; instance_id={}", instance_id); let mut instance = instances[instance_id].lock().await; let progress_thread = instance.progress_thread.take(); // drop locks otherwise we might end up with a deadlock drop(instance); drop(instances); + trace!("stop_progress: released instances; instance_id={}", instance_id); + let mut instance = instances[instance_id].lock().await; if let Some(t) = progress_thread { t.sender.send(()).await.unwrap(); t.handle.await.unwrap(); @@ -1292,6 +1304,7 @@ impl ApiState { } pub async fn list_instance_states(&self) -> Vec { + panic!(""); let instances = self.instances.read().await; let mut res = vec![]; @@ -1386,6 +1399,8 @@ impl ApiState { ); let instances_cloned = instances.clone(); let instances_locked = instances_cloned.read().await; + trace!("update_instances_with_timeout: grabbed instances; instance_id={}", instance_id); + let mut instance = instances[instance_id].lock().await; let (bg_task, busy_outcome) = if let Some(instance_mutex) = instances_locked.get(instance_id) { @@ -1438,6 +1453,7 @@ impl ApiState { // add result to graph, but grab instance lock first! println!("waiting to grab instane: old={:?} op_id={:?}", old_state_label, op_id); let instances = instances.blocking_read(); + trace!("update_instances_with_timeout-thread: grabbed instances; instance_id={}", instance_id); println!("waiting to grab graph: old={:?} op_id={:?}", old_state_label, op_id); let mut graph_guard = graph.blocking_write(); println!("grabbed graph: old={:?} op_id={:?}", old_state_label, op_id); @@ -1455,6 +1471,7 @@ impl ApiState { instance.state = InstanceState::Available(pocket_ic); } trace!("bg_task::end instance_id={} op_id={}", instance_id, op_id.0); + trace!("update_instances_with_timeout-thread: released instances; instance_id={}", instance_id); result } }; @@ -1470,6 +1487,7 @@ impl ApiState { }; // drop lock, otherwise we end up with a deadlock std::mem::drop(instances_locked); + trace!("update_instances_with_timeout: released instances; instance_id={}", instance_id); // We schedule a blocking background task on the tokio runtime. Note that if all // blocking workers are busy, the task is put on a queue (which is what we want). From cbcd773fe46320caf5f506995ab3ba559d45f914 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 11:26:13 +0200 Subject: [PATCH 34/43] fix --- rs/pocket_ic_server/src/state_api/state.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index ba2d493705d..5895fc49953 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -1296,7 +1296,6 @@ impl ApiState { drop(instance); drop(instances); trace!("stop_progress: released instances; instance_id={}", instance_id); - let mut instance = instances[instance_id].lock().await; if let Some(t) = progress_thread { t.sender.send(()).await.unwrap(); t.handle.await.unwrap(); @@ -1400,7 +1399,6 @@ impl ApiState { let instances_cloned = instances.clone(); let instances_locked = instances_cloned.read().await; trace!("update_instances_with_timeout: grabbed instances; instance_id={}", instance_id); - let mut instance = instances[instance_id].lock().await; let (bg_task, busy_outcome) = if let Some(instance_mutex) = instances_locked.get(instance_id) { From a986554a950abfe0d091a701b197c2fd61ee4bd6 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 11:35:27 +0200 Subject: [PATCH 35/43] dbg --- rs/pocket_ic_server/src/state_api/state.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index 5895fc49953..c9042155c87 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -751,9 +751,11 @@ impl ApiState { match &instance.state { InstanceState::Available(_) => { let _ = std::mem::replace(&mut instance.state, InstanceState::Deleted); + trace!("released instances in delete_instance; instance_id={}", instance_id); break; } InstanceState::Deleted => { + trace!("released instances in delete_instance; instance_id={}", instance_id); break; } InstanceState::Busy { .. } => {} From 20a51ba27d03803aef0bbaca79eccf9b9918842d Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 12:10:13 +0200 Subject: [PATCH 36/43] mutex --- rs/pocket_ic_server/src/state_api/state.rs | 28 +++++++++++----------- 1 file changed, 14 insertions(+), 14 deletions(-) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index c9042155c87..7bd7a40f506 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -140,7 +140,7 @@ impl std::fmt::Debug for Instance { /// The state of the PocketIC API. pub struct ApiState { // impl note: If locks are acquired on both fields, acquire first on `instances` and then on `graph`. - instances: Arc>>>, + instances: Arc>>>, graph: Arc>>, sync_wait_time: Duration, // PocketIC server port @@ -201,7 +201,7 @@ impl PocketIcApiStateBuilder { }) }) .collect(); - let instances = Arc::new(RwLock::new(instances)); + let instances = Arc::new(Mutex::new(instances)); let sync_wait_time = self.sync_wait_time.unwrap_or(DEFAULT_SYNC_WAIT_DURATION); @@ -658,7 +658,7 @@ impl ApiState { // Executes an operation to completion and returns its `OpOut` // or `None` if the auto progress mode received a stop signal. async fn execute_operation( - instances: Arc>>>, + instances: Arc>>>, graph: Arc>>, instance_id: InstanceId, op: impl Operation + Send + Sync + 'static, @@ -729,7 +729,7 @@ impl ApiState { where F: FnOnce(InstanceId) -> PocketIc + std::marker::Send + 'static, { - let mut instances = self.instances.write().await; + let mut instances = self.instances.lock().await; let instance_id = instances.len(); let instance = tokio::task::spawn_blocking(move || f(instance_id)) .await @@ -745,7 +745,7 @@ impl ApiState { pub async fn delete_instance(&self, instance_id: InstanceId) { self.stop_progress(instance_id).await; loop { - let instances = self.instances.read().await; + let instances = self.instances.lock().await; trace!("grabbed instances in delete_instance; instance_id={}", instance_id); let mut instance = instances[instance_id].lock().await; match &instance.state { @@ -769,7 +769,7 @@ impl ApiState { pub async fn delete_all_instances(arc_self: Arc) { let mut tasks = JoinSet::new(); - let instances = arc_self.instances.read().await; + let instances = arc_self.instances.lock().await; trace!("delete_all_instances: grabbed instances"); let num_instances = instances.len(); drop(instances); @@ -1151,7 +1151,7 @@ impl ApiState { } async fn process_canister_http_requests( - instances: Arc>>>, + instances: Arc>>>, graph: Arc>>, instance_id: InstanceId, rx: &mut Receiver<()>, @@ -1174,7 +1174,7 @@ impl ApiState { let subnet_id = canister_http_request.subnet_id; let request_id = canister_http_request.request_id; let response = loop { - let instances = instances.read().await; + let instances = instances.lock().await; trace!("process_canister_http_requests: grabbed instances; instance_id={}", instance_id); let instance = instances[instance_id].lock().await; if let InstanceState::Available(pocket_ic) = &instance.state { @@ -1235,7 +1235,7 @@ impl ApiState { let artificial_delay = Duration::from_millis(artificial_delay_ms.unwrap_or_default()); let instances_clone = self.instances.clone(); let graph = self.graph.clone(); - let instances = self.instances.read().await; + let instances = self.instances.lock().await; trace!("auto_progress: grabbed instances; instance_id={}", instance_id); let mut instance = instances[instance_id].lock().await; if instance.progress_thread.is_none() { @@ -1290,7 +1290,7 @@ impl ApiState { } pub async fn stop_progress(&self, instance_id: InstanceId) { - let instances = self.instances.read().await; + let instances = self.instances.lock().await; trace!("stop_progress: grabbed instances; instance_id={}", instance_id); let mut instance = instances[instance_id].lock().await; let progress_thread = instance.progress_thread.take(); @@ -1306,7 +1306,7 @@ impl ApiState { pub async fn list_instance_states(&self) -> Vec { panic!(""); - let instances = self.instances.read().await; + let instances = self.instances.lock().await; let mut res = vec![]; println!("total instances: {}", instances.len()); @@ -1383,7 +1383,7 @@ impl ApiState { /// Same as [Self::update] except that the timeout can be specified manually. This is useful in /// cases when clients want to enforce a long-running blocking call. async fn update_instances_with_timeout( - instances: Arc>>>, + instances: Arc>>>, graph: Arc>>, op: Arc, instance_id: InstanceId, @@ -1399,7 +1399,7 @@ impl ApiState { op_id, ); let instances_cloned = instances.clone(); - let instances_locked = instances_cloned.read().await; + let instances_locked = instances_cloned.lock().await; trace!("update_instances_with_timeout: grabbed instances; instance_id={}", instance_id); let (bg_task, busy_outcome) = if let Some(instance_mutex) = instances_locked.get(instance_id) @@ -1452,7 +1452,7 @@ impl ApiState { let new_state_label = pocket_ic.get_state_label(); // add result to graph, but grab instance lock first! println!("waiting to grab instane: old={:?} op_id={:?}", old_state_label, op_id); - let instances = instances.blocking_read(); + let instances = instances.blocking_lock(); trace!("update_instances_with_timeout-thread: grabbed instances; instance_id={}", instance_id); println!("waiting to grab graph: old={:?} op_id={:?}", old_state_label, op_id); let mut graph_guard = graph.blocking_write(); From 1c0593546b1723f7df76cbd7aff4701d46645d61 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 24 Oct 2024 12:17:01 +0200 Subject: [PATCH 37/43] dbg --- rs/pocket_ic_server/src/state_api/state.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index 7bd7a40f506..70cae4418f3 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -729,11 +729,14 @@ impl ApiState { where F: FnOnce(InstanceId) -> PocketIc + std::marker::Send + 'static, { + trace!("add_instance:start"); let mut instances = self.instances.lock().await; + trace!("add_instance:locked"); let instance_id = instances.len(); let instance = tokio::task::spawn_blocking(move || f(instance_id)) .await .expect("Failed to create PocketIC instance"); + trace!("add_instance:done_blocking"); let topology = instance.topology(); instances.push(Mutex::new(Instance { progress_thread: None, From d2f15ca1cc942b8830644cce0cfcbe9c67e8da11 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Fri, 25 Oct 2024 11:16:46 +0200 Subject: [PATCH 38/43] spawning_block --- packages/pocket-ic/tests/tests.rs | 4 ++-- rs/pocket_ic_server/src/pocket_ic.rs | 6 +++--- rs/pocket_ic_server/src/state_api/routes.rs | 4 ++-- rs/pocket_ic_server/src/state_api/state.rs | 24 ++++++++++----------- 4 files changed, 19 insertions(+), 19 deletions(-) diff --git a/packages/pocket-ic/tests/tests.rs b/packages/pocket-ic/tests/tests.rs index f6e4259704a..a26aeb65de2 100644 --- a/packages/pocket-ic/tests/tests.rs +++ b/packages/pocket-ic/tests/tests.rs @@ -378,7 +378,7 @@ fn test_get_set_cycle_balance() { assert_eq!(balance, initial_balance + 420); } -//#[test] +#[test] fn test_create_and_drop_instances() { let pic = PocketIc::new(); let id = pic.instance_id(); @@ -622,7 +622,7 @@ fn test_too_large_call() { .unwrap_err(); } -//#[tokio::test] +#[tokio::test] async fn test_create_and_drop_instances_async() { let pic = pocket_ic::nonblocking::PocketIc::new().await; let id = pic.instance_id; diff --git a/rs/pocket_ic_server/src/pocket_ic.rs b/rs/pocket_ic_server/src/pocket_ic.rs index 4dc13301948..f93d5496556 100644 --- a/rs/pocket_ic_server/src/pocket_ic.rs +++ b/rs/pocket_ic_server/src/pocket_ic.rs @@ -1,6 +1,6 @@ #![allow(clippy::disallowed_types)] use crate::state_api::state::{HasStateLabel, OpOut, PocketIcError, StateLabel}; -use crate::{async_trait, copy_dir, BlobStore, InstanceId, OpId, Operation}; +use crate::{async_trait, copy_dir, BlobStore, OpId, Operation}; use askama::Template; use axum::{ extract::State, @@ -386,7 +386,7 @@ impl PocketIc { pub(crate) fn new( runtime: Arc, - instance_id: InstanceId, + seed: u64, subnet_configs: ExtendedSubnetConfigSet, state_dir: Option, nonmainnet_features: bool, @@ -688,7 +688,7 @@ impl PocketIc { default_effective_canister_id, }; - let state_label = StateLabel::new(instance_id); + let state_label = StateLabel::new(seed); Self { state_dir, diff --git a/rs/pocket_ic_server/src/state_api/routes.rs b/rs/pocket_ic_server/src/state_api/routes.rs index 27b1c5b09d2..a2d196f5cf0 100644 --- a/rs/pocket_ic_server/src/state_api/routes.rs +++ b/rs/pocket_ic_server/src/state_api/routes.rs @@ -1113,10 +1113,10 @@ pub async fn create_instance( }; let (instance_id, topology) = api_state - .add_instance(move |instance_id| { + .add_instance(move |seed| { PocketIc::new( runtime, - instance_id, + seed, subnet_configs, instance_config.state_dir, instance_config.nonmainnet_features, diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index 70cae4418f3..2e72c7e254c 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -52,7 +52,7 @@ use pocket_ic::common::rest::{ }; use pocket_ic::{ErrorCode, UserError, WasmResult}; use serde::{Deserialize, Serialize}; -use std::{collections::HashMap, fmt, path::PathBuf, str::FromStr, sync::Arc, time::Duration}; +use std::{collections::HashMap, fmt, path::PathBuf, str::FromStr, sync::Arc, sync::atomic::AtomicU64, time::Duration}; use tokio::{ sync::mpsc::error::TryRecvError, sync::mpsc::Receiver, @@ -81,8 +81,8 @@ pub const STATE_LABEL_HASH_SIZE: usize = 16; pub struct StateLabel(pub [u8; STATE_LABEL_HASH_SIZE]); impl StateLabel { - pub fn new(instance_id: InstanceId) -> Self { - let mut seq_no: u128 = instance_id.try_into().unwrap(); + pub fn new(seed: u64) -> Self { + let mut seq_no: u128 = seed.try_into().unwrap(); seq_no <<= 64; Self(seq_no.to_le_bytes()) } @@ -142,6 +142,7 @@ pub struct ApiState { // impl note: If locks are acquired on both fields, acquire first on `instances` and then on `graph`. instances: Arc>>>, graph: Arc>>, + seed: AtomicU64, sync_wait_time: Duration, // PocketIC server port port: Option, @@ -208,6 +209,7 @@ impl PocketIcApiStateBuilder { Arc::new(ApiState { instances, graph, + seed: AtomicU64::new(0), sync_wait_time, port: self.port, http_gateways: Arc::new(RwLock::new(Vec::new())), @@ -727,17 +729,18 @@ impl ApiState { pub async fn add_instance(&self, f: F) -> (InstanceId, Topology) where - F: FnOnce(InstanceId) -> PocketIc + std::marker::Send + 'static, + F: FnOnce(u64) -> PocketIc + std::marker::Send + 'static, { + let seed = self.seed.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let instance = tokio::task::spawn_blocking(move || f(seed)) + .await + .expect("Failed to create PocketIC instance"); + let topology = instance.topology(); trace!("add_instance:start"); let mut instances = self.instances.lock().await; trace!("add_instance:locked"); let instance_id = instances.len(); - let instance = tokio::task::spawn_blocking(move || f(instance_id)) - .await - .expect("Failed to create PocketIC instance"); trace!("add_instance:done_blocking"); - let topology = instance.topology(); instances.push(Mutex::new(Instance { progress_thread: None, state: InstanceState::Available(instance), @@ -1308,7 +1311,6 @@ impl ApiState { } pub async fn list_instance_states(&self) -> Vec { - panic!(""); let instances = self.instances.lock().await; let mut res = vec![]; @@ -1542,8 +1544,7 @@ impl std::fmt::Debug for InstanceState { impl std::fmt::Debug for ApiState { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - /* - let instances = self.instances.blocking_read(); + let instances = self.instances.blocking_lock(); let graph = self.graph.blocking_read(); writeln!(f, "Instances:")?; @@ -1555,7 +1556,6 @@ impl std::fmt::Debug for ApiState { for (k, v) in graph.iter() { writeln!(f, " {k:?} => {v:?}")?; } - */ Ok(()) } } From a4e4fb5c1bd433a1ad404ceca6cb33d21f36e4f6 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Fri, 25 Oct 2024 11:23:58 +0200 Subject: [PATCH 39/43] Revert "mutex" This reverts commit 20a51ba27d03803aef0bbaca79eccf9b9918842d. --- rs/pocket_ic_server/src/state_api/state.rs | 26 +++++++++++----------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index 2e72c7e254c..4bb9f27790f 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -140,7 +140,7 @@ impl std::fmt::Debug for Instance { /// The state of the PocketIC API. pub struct ApiState { // impl note: If locks are acquired on both fields, acquire first on `instances` and then on `graph`. - instances: Arc>>>, + instances: Arc>>>, graph: Arc>>, seed: AtomicU64, sync_wait_time: Duration, @@ -202,7 +202,7 @@ impl PocketIcApiStateBuilder { }) }) .collect(); - let instances = Arc::new(Mutex::new(instances)); + let instances = Arc::new(RwLock::new(instances)); let sync_wait_time = self.sync_wait_time.unwrap_or(DEFAULT_SYNC_WAIT_DURATION); @@ -660,7 +660,7 @@ impl ApiState { // Executes an operation to completion and returns its `OpOut` // or `None` if the auto progress mode received a stop signal. async fn execute_operation( - instances: Arc>>>, + instances: Arc>>>, graph: Arc>>, instance_id: InstanceId, op: impl Operation + Send + Sync + 'static, @@ -751,7 +751,7 @@ impl ApiState { pub async fn delete_instance(&self, instance_id: InstanceId) { self.stop_progress(instance_id).await; loop { - let instances = self.instances.lock().await; + let instances = self.instances.read().await; trace!("grabbed instances in delete_instance; instance_id={}", instance_id); let mut instance = instances[instance_id].lock().await; match &instance.state { @@ -775,7 +775,7 @@ impl ApiState { pub async fn delete_all_instances(arc_self: Arc) { let mut tasks = JoinSet::new(); - let instances = arc_self.instances.lock().await; + let instances = arc_self.instances.read().await; trace!("delete_all_instances: grabbed instances"); let num_instances = instances.len(); drop(instances); @@ -1157,7 +1157,7 @@ impl ApiState { } async fn process_canister_http_requests( - instances: Arc>>>, + instances: Arc>>>, graph: Arc>>, instance_id: InstanceId, rx: &mut Receiver<()>, @@ -1180,7 +1180,7 @@ impl ApiState { let subnet_id = canister_http_request.subnet_id; let request_id = canister_http_request.request_id; let response = loop { - let instances = instances.lock().await; + let instances = instances.read().await; trace!("process_canister_http_requests: grabbed instances; instance_id={}", instance_id); let instance = instances[instance_id].lock().await; if let InstanceState::Available(pocket_ic) = &instance.state { @@ -1241,7 +1241,7 @@ impl ApiState { let artificial_delay = Duration::from_millis(artificial_delay_ms.unwrap_or_default()); let instances_clone = self.instances.clone(); let graph = self.graph.clone(); - let instances = self.instances.lock().await; + let instances = self.instances.read().await; trace!("auto_progress: grabbed instances; instance_id={}", instance_id); let mut instance = instances[instance_id].lock().await; if instance.progress_thread.is_none() { @@ -1296,7 +1296,7 @@ impl ApiState { } pub async fn stop_progress(&self, instance_id: InstanceId) { - let instances = self.instances.lock().await; + let instances = self.instances.read().await; trace!("stop_progress: grabbed instances; instance_id={}", instance_id); let mut instance = instances[instance_id].lock().await; let progress_thread = instance.progress_thread.take(); @@ -1311,7 +1311,7 @@ impl ApiState { } pub async fn list_instance_states(&self) -> Vec { - let instances = self.instances.lock().await; + let instances = self.instances.read().await; let mut res = vec![]; println!("total instances: {}", instances.len()); @@ -1388,7 +1388,7 @@ impl ApiState { /// Same as [Self::update] except that the timeout can be specified manually. This is useful in /// cases when clients want to enforce a long-running blocking call. async fn update_instances_with_timeout( - instances: Arc>>>, + instances: Arc>>>, graph: Arc>>, op: Arc, instance_id: InstanceId, @@ -1404,7 +1404,7 @@ impl ApiState { op_id, ); let instances_cloned = instances.clone(); - let instances_locked = instances_cloned.lock().await; + let instances_locked = instances_cloned.read().await; trace!("update_instances_with_timeout: grabbed instances; instance_id={}", instance_id); let (bg_task, busy_outcome) = if let Some(instance_mutex) = instances_locked.get(instance_id) @@ -1457,7 +1457,7 @@ impl ApiState { let new_state_label = pocket_ic.get_state_label(); // add result to graph, but grab instance lock first! println!("waiting to grab instane: old={:?} op_id={:?}", old_state_label, op_id); - let instances = instances.blocking_lock(); + let instances = instances.blocking_read(); trace!("update_instances_with_timeout-thread: grabbed instances; instance_id={}", instance_id); println!("waiting to grab graph: old={:?} op_id={:?}", old_state_label, op_id); let mut graph_guard = graph.blocking_write(); From f8d989d19d2ccfa7e43fd05a1bc5a20f9a156005 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Fri, 25 Oct 2024 11:25:49 +0200 Subject: [PATCH 40/43] simplify --- rs/pocket_ic_server/src/state_api/state.rs | 40 +--------------------- 1 file changed, 1 insertion(+), 39 deletions(-) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index 4bb9f27790f..e86d2fbad05 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -130,13 +130,6 @@ struct Instance { state: InstanceState, } -impl std::fmt::Debug for Instance { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - writeln!(f, "{:?}", self.state)?; - Ok(()) - } -} - /// The state of the PocketIC API. pub struct ApiState { // impl note: If locks are acquired on both fields, acquire first on `instances` and then on `graph`. @@ -737,7 +730,7 @@ impl ApiState { .expect("Failed to create PocketIC instance"); let topology = instance.topology(); trace!("add_instance:start"); - let mut instances = self.instances.lock().await; + let mut instances = self.instances.write().await; trace!("add_instance:locked"); let instance_id = instances.len(); trace!("add_instance:done_blocking"); @@ -1528,34 +1521,3 @@ impl ApiState { Ok(busy_outcome) } } - -impl std::fmt::Debug for InstanceState { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - match self { - Self::Busy { state_label, op_id } => { - write!(f, "Busy {{ {state_label:?}, {op_id:?} }}")? - } - Self::Available(pic) => write!(f, "Available({:?})", pic.get_state_label())?, - Self::Deleted => write!(f, "Deleted")?, - } - Ok(()) - } -} - -impl std::fmt::Debug for ApiState { - fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let instances = self.instances.blocking_lock(); - let graph = self.graph.blocking_read(); - - writeln!(f, "Instances:")?; - for (idx, instance) in instances.iter().enumerate() { - writeln!(f, " [{idx}] {instance:?}")?; - } - - writeln!(f, "Graph:")?; - for (k, v) in graph.iter() { - writeln!(f, " {k:?} => {v:?}")?; - } - Ok(()) - } -} From 124791a432d2373f84e6c20fd2227775c2e324ea Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Fri, 25 Oct 2024 11:36:43 +0200 Subject: [PATCH 41/43] simplify --- .github/workflows/ci-pic.yml | 36 ---------------------- packages/pocket-ic/src/nonblocking.rs | 2 +- rs/pocket_ic_server/src/state_api/state.rs | 29 ----------------- 3 files changed, 1 insertion(+), 66 deletions(-) delete mode 100644 .github/workflows/ci-pic.yml diff --git a/.github/workflows/ci-pic.yml b/.github/workflows/ci-pic.yml deleted file mode 100644 index c862683f04d..00000000000 --- a/.github/workflows/ci-pic.yml +++ /dev/null @@ -1,36 +0,0 @@ -name: CI PocketIC -on: - pull_request: -concurrency: - group: ${{ github.workflow }}-${{ github.head_ref}} - cancel-in-progress: true -jobs: - pic-test-all: - name: PocketIC Test - container: - image: ghcr.io/dfinity/ic-build@sha256:ced9611af39119f62feedc8a5ad75a8fac478cb43d2d56b623023766f6ce8ca7 - options: >- - -e NODE_NAME --privileged --cgroupns host -v /cache:/cache -v /var/sysimage:/var/sysimage -v /var/tmp:/var/tmp -v /ceph-s3-info:/ceph-s3-info - timeout-minutes: 90 - runs-on: - group: zh1 - labels: dind-large - steps: - - name: Checkout - uses: actions/checkout@v4 - with: - fetch-depth: ${{ github.event_name == 'pull_request' && 256 || 0 }} - - name: Before script - id: before-script - shell: bash - run: | - [ -n "${NODE_NAME:-}" ] && echo "Node: $NODE_NAME" - - name: Login to Dockerhub - shell: bash - run: ./ci/scripts/docker-login.sh - env: - DOCKER_HUB_USER: ${{ vars.DOCKER_HUB_USER }} - DOCKER_HUB_PASSWORD_RO: ${{ secrets.DOCKER_HUB_PASSWORD_RO }} - - name: Run Bazel Test All - run: | - bazel test //packages/pocket-ic:test --cache_test_results=no --test_output=streamed --test_arg="--nocapture" --test_env=RUST_LOG=trace diff --git a/packages/pocket-ic/src/nonblocking.rs b/packages/pocket-ic/src/nonblocking.rs index 6440c8fb099..3d52bb811e3 100644 --- a/packages/pocket-ic/src/nonblocking.rs +++ b/packages/pocket-ic/src/nonblocking.rs @@ -35,7 +35,7 @@ use tracing_appender::non_blocking::WorkerGuard; use tracing_subscriber::EnvFilter; // wait time between polling requests -const POLLING_PERIOD_MS: u64 = 500; +const POLLING_PERIOD_MS: u64 = 10; const LOG_DIR_PATH_ENV_NAME: &str = "POCKET_IC_LOG_DIR"; const LOG_DIR_LEVELS_ENV_NAME: &str = "POCKET_IC_LOG_DIR_LEVELS"; diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index e86d2fbad05..f1f8dd0fee5 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -729,11 +729,8 @@ impl ApiState { .await .expect("Failed to create PocketIC instance"); let topology = instance.topology(); - trace!("add_instance:start"); let mut instances = self.instances.write().await; - trace!("add_instance:locked"); let instance_id = instances.len(); - trace!("add_instance:done_blocking"); instances.push(Mutex::new(Instance { progress_thread: None, state: InstanceState::Available(instance), @@ -745,23 +742,19 @@ impl ApiState { self.stop_progress(instance_id).await; loop { let instances = self.instances.read().await; - trace!("grabbed instances in delete_instance; instance_id={}", instance_id); let mut instance = instances[instance_id].lock().await; match &instance.state { InstanceState::Available(_) => { let _ = std::mem::replace(&mut instance.state, InstanceState::Deleted); - trace!("released instances in delete_instance; instance_id={}", instance_id); break; } InstanceState::Deleted => { - trace!("released instances in delete_instance; instance_id={}", instance_id); break; } InstanceState::Busy { .. } => {} } drop(instance); drop(instances); - trace!("released instances in delete_instance; instance_id={}", instance_id); tokio::time::sleep(Duration::from_secs(1)).await; } } @@ -769,10 +762,8 @@ impl ApiState { pub async fn delete_all_instances(arc_self: Arc) { let mut tasks = JoinSet::new(); let instances = arc_self.instances.read().await; - trace!("delete_all_instances: grabbed instances"); let num_instances = instances.len(); drop(instances); - trace!("delete_all_instances: released instances"); for instance_id in 0..num_instances { let arc_self_clone = arc_self.clone(); tasks.spawn(async move { arc_self_clone.delete_instance(instance_id).await }); @@ -1174,7 +1165,6 @@ impl ApiState { let request_id = canister_http_request.request_id; let response = loop { let instances = instances.read().await; - trace!("process_canister_http_requests: grabbed instances; instance_id={}", instance_id); let instance = instances[instance_id].lock().await; if let InstanceState::Available(pocket_ic) = &instance.state { let canister_http_adapters = pocket_ic.canister_http_adapters(); @@ -1199,7 +1189,6 @@ impl ApiState { } drop(instance); drop(instances); - trace!("process_canister_http_requests: released instances; instance_id={}", instance_id); tokio::time::sleep(Duration::from_millis(10)).await; }; let mock_canister_http_response = MockCanisterHttpResponse { @@ -1235,7 +1224,6 @@ impl ApiState { let instances_clone = self.instances.clone(); let graph = self.graph.clone(); let instances = self.instances.read().await; - trace!("auto_progress: grabbed instances; instance_id={}", instance_id); let mut instance = instances[instance_id].lock().await; if instance.progress_thread.is_none() { let (tx, mut rx) = mpsc::channel::<()>(1); @@ -1280,23 +1268,19 @@ impl ApiState { } }); instance.progress_thread = Some(ProgressThread { handle, sender: tx }); - trace!("auto_progress: released instances; instance_id={}", instance_id); Ok(()) } else { - trace!("auto_progress: released instances; instance_id={}", instance_id); Err("Auto progress mode has already been enabled.".to_string()) } } pub async fn stop_progress(&self, instance_id: InstanceId) { let instances = self.instances.read().await; - trace!("stop_progress: grabbed instances; instance_id={}", instance_id); let mut instance = instances[instance_id].lock().await; let progress_thread = instance.progress_thread.take(); // drop locks otherwise we might end up with a deadlock drop(instance); drop(instances); - trace!("stop_progress: released instances; instance_id={}", instance_id); if let Some(t) = progress_thread { t.sender.send(()).await.unwrap(); t.handle.await.unwrap(); @@ -1307,11 +1291,8 @@ impl ApiState { let instances = self.instances.read().await; let mut res = vec![]; - println!("total instances: {}", instances.len()); for instance in &*instances { - println!("grabbing instance"); let instance = &*instance.lock().await; - println!("grabbed instance"); match &instance.state { InstanceState::Busy { state_label, op_id } => { res.push(format!("Busy({:?}, {:?})", state_label, op_id)) @@ -1398,13 +1379,10 @@ impl ApiState { ); let instances_cloned = instances.clone(); let instances_locked = instances_cloned.read().await; - trace!("update_instances_with_timeout: grabbed instances; instance_id={}", instance_id); let (bg_task, busy_outcome) = if let Some(instance_mutex) = instances_locked.get(instance_id) { - println!("waiting to grab instance mutex; instance_id={:?}, op_id={:?}", instance_id, op_id); let mut instance = instance_mutex.lock().await; - println!("grabbed instance mutex; instance_id={:?}, op_id={:?}", instance_id, op_id); // If this instance is busy, return the running op and initial state match &instance.state { InstanceState::Deleted => { @@ -1449,19 +1427,14 @@ impl ApiState { pocket_ic.bump_state_label(); let new_state_label = pocket_ic.get_state_label(); // add result to graph, but grab instance lock first! - println!("waiting to grab instane: old={:?} op_id={:?}", old_state_label, op_id); let instances = instances.blocking_read(); - trace!("update_instances_with_timeout-thread: grabbed instances; instance_id={}", instance_id); - println!("waiting to grab graph: old={:?} op_id={:?}", old_state_label, op_id); let mut graph_guard = graph.blocking_write(); - println!("grabbed graph: old={:?} op_id={:?}", old_state_label, op_id); let cached_computations = graph_guard.entry(old_state_label.clone()).or_default(); cached_computations .insert(op_id.clone(), (new_state_label, result.clone())); drop(graph_guard); let mut instance = instances[instance_id].blocking_lock(); - println!("grabbed instance: old={:?} op_id={:?}", old_state_label, op_id); if let InstanceState::Deleted = &instance.state { error!("The instance is deleted immediately after an operation. This is a bug!"); std::mem::drop(pocket_ic); @@ -1469,7 +1442,6 @@ impl ApiState { instance.state = InstanceState::Available(pocket_ic); } trace!("bg_task::end instance_id={} op_id={}", instance_id, op_id.0); - trace!("update_instances_with_timeout-thread: released instances; instance_id={}", instance_id); result } }; @@ -1485,7 +1457,6 @@ impl ApiState { }; // drop lock, otherwise we end up with a deadlock std::mem::drop(instances_locked); - trace!("update_instances_with_timeout: released instances; instance_id={}", instance_id); // We schedule a blocking background task on the tokio runtime. Note that if all // blocking workers are busy, the task is put on a queue (which is what we want). From ff65bb597be74ae604090fa476f56328e0802b0b Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Fri, 25 Oct 2024 11:38:54 +0200 Subject: [PATCH 42/43] comment --- rs/pocket_ic_server/src/state_api/state.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index f1f8dd0fee5..490bd0bc173 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -52,7 +52,10 @@ use pocket_ic::common::rest::{ }; use pocket_ic::{ErrorCode, UserError, WasmResult}; use serde::{Deserialize, Serialize}; -use std::{collections::HashMap, fmt, path::PathBuf, str::FromStr, sync::Arc, sync::atomic::AtomicU64, time::Duration}; +use std::{ + collections::HashMap, fmt, path::PathBuf, str::FromStr, sync::atomic::AtomicU64, sync::Arc, + time::Duration, +}; use tokio::{ sync::mpsc::error::TryRecvError, sync::mpsc::Receiver, @@ -725,6 +728,7 @@ impl ApiState { F: FnOnce(u64) -> PocketIc + std::marker::Send + 'static, { let seed = self.seed.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + // create the instance using `spawn_blocking` before acquiring a lock let instance = tokio::task::spawn_blocking(move || f(seed)) .await .expect("Failed to create PocketIC instance"); From 6d34554d1fa49893990bb0aa0a5c73215fe85e2e Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Fri, 25 Oct 2024 11:42:41 +0200 Subject: [PATCH 43/43] clippy --- rs/pocket_ic_server/src/state_api/state.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rs/pocket_ic_server/src/state_api/state.rs b/rs/pocket_ic_server/src/state_api/state.rs index 490bd0bc173..976ef947e5f 100644 --- a/rs/pocket_ic_server/src/state_api/state.rs +++ b/rs/pocket_ic_server/src/state_api/state.rs @@ -85,7 +85,7 @@ pub struct StateLabel(pub [u8; STATE_LABEL_HASH_SIZE]); impl StateLabel { pub fn new(seed: u64) -> Self { - let mut seq_no: u128 = seed.try_into().unwrap(); + let mut seq_no: u128 = seed.into(); seq_no <<= 64; Self(seq_no.to_le_bytes()) }