diff --git a/rs/execution_environment/src/execution_environment/tests.rs b/rs/execution_environment/src/execution_environment/tests.rs index db3854d7555a..7fa5e46d040b 100644 --- a/rs/execution_environment/src/execution_environment/tests.rs +++ b/rs/execution_environment/src/execution_environment/tests.rs @@ -18,7 +18,7 @@ use ic_registry_subnet_type::SubnetType; use ic_replicated_state::{ CanisterStatus, ReplicatedState, SystemState, canister_state::{DEFAULT_QUEUE_CAPACITY, WASM_PAGE_SIZE_IN_BYTES}, - metadata_state::subnet_call_context_manager::{PreSignatureStash, StopCanisterCall}, + metadata_state::subnet_call_context_manager::PreSignatureStash, metadata_state::testing::NetworkTopologyTesting, testing::{CanisterQueuesTesting, SystemStateTesting}, }; @@ -29,14 +29,13 @@ use ic_test_utilities_execution_environment::{ get_reject, get_reply, }; use ic_test_utilities_metrics::{fetch_histogram_vec_count, metric_vec}; -use ic_test_utilities_types::messages::IngressBuilder; use ic_types::{ CanisterId, CountBytes, PrincipalId, RegistryVersion, canister_http::{CanisterHttpMethod, Transform}, consensus::idkg::{IDkgMasterPublicKeyId, PreSigId}, ingress::{IngressState, IngressStatus, WasmResult}, messages::{ - CallbackId, CanisterCall, MAX_RESPONSE_COUNT_BYTES, NO_DEADLINE, Payload, RejectContext, + CallbackId, MAX_RESPONSE_COUNT_BYTES, NO_DEADLINE, Payload, RejectContext, RequestOrResponse, Response, }, time::UNIX_EPOCH, @@ -1752,50 +1751,6 @@ fn clean_in_progress_stop_canister_calls_from_subnet_call_context_manager() { ); } -// Verifies that `remove_orphaned_stop_canister_calls` removes `StopCanisterCall`s from -// `SubnetCallContextManager` that have no corresponding `StopCanisterContext` -// in the target canister, simulating the bug fixed in commit 52e7b89. -#[test] -fn cleanup_orphaned_stop_canister_calls() { - let mut test = ExecutionTestBuilder::new().with_manual_execution().build(); - - let canister_id = test - .create_canister_with_allocation(Cycles::new(1_000_000_000_000_000), None, None) - .unwrap(); - - // Simulate the pre-52e7b89 bug: push a StopCanisterCall for a Running canister - // without a corresponding StopCanisterContext in the target canister. - let time = test.time(); - test.state_mut() - .metadata - .subnet_call_context_manager - .push_stop_canister_call(StopCanisterCall { - call: CanisterCall::Ingress(Arc::new(IngressBuilder::new().build())), - effective_canister_id: canister_id, - time, - }); - - assert_eq!( - test.state() - .metadata - .subnet_call_context_manager - .stop_canister_calls_len(), - 1 - ); - - // remove_orphaned_stop_canister_calls should remove the orphaned call since there - // is no matching StopCanisterContext in the target canister. - test.state_mut().remove_orphaned_stop_canister_calls(); - - assert_eq!( - test.state() - .metadata - .subnet_call_context_manager - .stop_canister_calls_len(), - 0 - ); -} - #[test] fn subnet_split_cleans_in_progress_stop_canister_calls() { let own_subnet = subnet_test_id(1); diff --git a/rs/execution_environment/src/scheduler.rs b/rs/execution_environment/src/scheduler.rs index 03f70d819226..a9030c900094 100644 --- a/rs/execution_environment/src/scheduler.rs +++ b/rs/execution_environment/src/scheduler.rs @@ -1189,14 +1189,6 @@ impl Scheduler for SchedulerImpl { self.metrics.canister_ingress_queue_latencies.clone(), ); - { - let _timer = self - .metrics - .remove_orphaned_stop_canister_calls_duration - .start_timer(); - state.remove_orphaned_stop_canister_calls(); - } - // Round preparation. let mut scheduler_round_limits = { let _timer = self.metrics.round_preparation_duration.start_timer(); diff --git a/rs/execution_environment/src/scheduler/scheduler_metrics.rs b/rs/execution_environment/src/scheduler/scheduler_metrics.rs index d20a4413184d..34ebd5b42745 100644 --- a/rs/execution_environment/src/scheduler/scheduler_metrics.rs +++ b/rs/execution_environment/src/scheduler/scheduler_metrics.rs @@ -33,7 +33,6 @@ pub struct SchedulerMetrics { pub(super) inner_round_loop_consumed_max_instructions: IntCounter, pub(super) num_canisters_uninstalled_out_of_cycles: IntCounter, pub(super) round: ScopedMetrics, - pub(super) remove_orphaned_stop_canister_calls_duration: Histogram, pub(super) round_preparation_duration: Histogram, pub(super) round_preparation_ingress: Histogram, pub(super) round_consensus_queue: ScopedMetrics, @@ -183,7 +182,6 @@ impl SchedulerMetrics { metrics_registry, ), }, - remove_orphaned_stop_canister_calls_duration: round_phase_duration_histogram("remove orphaned stop canister calls", metrics_registry), round_preparation_duration: round_phase_duration_histogram("preparation", metrics_registry), // Expiration of messages in the ingress queue. round_preparation_ingress: round_preparation_phase_duration_histogram("expire ingress", metrics_registry), diff --git a/rs/replicated_state/src/metadata_state/subnet_call_context_manager.rs b/rs/replicated_state/src/metadata_state/subnet_call_context_manager.rs index 0ab147f98809..4b64e996eb69 100644 --- a/rs/replicated_state/src/metadata_state/subnet_call_context_manager.rs +++ b/rs/replicated_state/src/metadata_state/subnet_call_context_manager.rs @@ -404,15 +404,6 @@ impl SubnetCallContextManager { self.canister_management_calls.stop_canister_calls_len() } - pub fn iter_stop_canister_calls( - &self, - ) -> impl Iterator { - self.canister_management_calls - .stop_canister_call_manager - .stop_canister_calls - .iter() - } - pub fn push_raw_rand_request( &mut self, request: Request, diff --git a/rs/replicated_state/src/replicated_state.rs b/rs/replicated_state/src/replicated_state.rs index ea82e6c99fc3..545a085a14e8 100644 --- a/rs/replicated_state/src/replicated_state.rs +++ b/rs/replicated_state/src/replicated_state.rs @@ -1,9 +1,7 @@ use crate::canister_state::queues::{ CanisterInput, CanisterQueuesLoopDetector, refunds::RefundPool, }; -use crate::canister_state::system_state::{ - CanisterOutputQueuesIterator, CanisterStatus, push_input, -}; +use crate::canister_state::system_state::{CanisterOutputQueuesIterator, push_input}; use crate::metadata_state::subnet_call_context_manager::{ PreSignatureStash, ReshareChainKeyContext, SignWithThresholdContext, }; @@ -32,8 +30,7 @@ use ic_types::{ consensus::idkg::IDkgMasterPublicKeyId, ingress::IngressStatus, messages::{ - CallbackId, Ingress, MessageId, Refund, RequestOrResponse, Response, StopCanisterCallId, - SubnetMessage, + CallbackId, Ingress, MessageId, Refund, RequestOrResponse, Response, SubnetMessage, }, time::CoarseTime, }; @@ -494,42 +491,6 @@ impl ReplicatedState { } } - /// Removes `StopCanisterCall`s from `SubnetCallContextManager` that have no - /// corresponding `StopCanisterContext` in the target canister's stop contexts. - /// These can arise from a bug fixed in commit 52e7b89 where a `StopCanisterCall` - /// was pushed but never removed when the `stop_canister` completed, e.g., when - /// the target canister was already stopped or the sender was not a controller. - pub fn remove_orphaned_stop_canister_calls(&mut self) { - let call_ids_with_context: BTreeSet = self - .canister_states - .values() - .flat_map(|canister| match canister.system_state.get_status() { - CanisterStatus::Stopping { stop_contexts, .. } => stop_contexts.as_slice(), - _ => &[], - }) - .filter_map(|ctx| *ctx.call_id()) - .collect(); - - let orphaned: Vec = self - .metadata - .subnet_call_context_manager - .iter_stop_canister_calls() - .filter_map(|(call_id, _)| { - if call_ids_with_context.contains(call_id) { - None - } else { - Some(*call_id) - } - }) - .collect(); - - for call_id in orphaned { - self.metadata - .subnet_call_context_manager - .remove_stop_canister_call(call_id); - } - } - /// References into _all_ fields. #[allow(clippy::type_complexity)] pub fn component_refs(