diff --git a/rs/cycles_account_manager/src/cycles_account_manager.rs b/rs/cycles_account_manager/src/cycles_account_manager.rs index c213d59d95cf..406c8aa5ffcb 100644 --- a/rs/cycles_account_manager/src/cycles_account_manager.rs +++ b/rs/cycles_account_manager/src/cycles_account_manager.rs @@ -348,7 +348,7 @@ impl CyclesAccountManager { cost_schedule, canister.system_state.reserved_balance(), ); - if canister.has_paused_execution() || canister.has_paused_install_code() { + if canister.has_paused_execution_or_install_code() { if canister.system_state.debited_balance() < cycles + threshold { return Err(CanisterOutOfCyclesError { canister_id: canister.canister_id(), diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index 029ae296cc91..f65daa394dc8 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -1,4 +1,5 @@ use crate::as_round_instructions; +use crate::canister_settings::{CanisterSettings, ValidatedCanisterSettings}; use crate::execution::common::{ validate_controller, validate_controller_or_subnet_admin, validate_snapshot_visibility, validate_subnet_admin, @@ -9,21 +10,15 @@ use crate::execution_environment::{ CompilationCostHandling, RoundContext, RoundCounters, RoundLimits, }; use crate::execution_environment_metrics::ExecutionEnvironmentMetrics; -use crate::util::MIGRATION_CANISTER_ID; -use crate::{ - canister_settings::{CanisterSettings, ValidatedCanisterSettings}, - hypervisor::Hypervisor, - types::{IngressResponse, Response}, - util::GOVERNANCE_CANISTER_ID, -}; +use crate::hypervisor::Hypervisor; +use crate::types::{IngressResponse, Response}; +use crate::util::{GOVERNANCE_CANISTER_ID, MIGRATION_CANISTER_ID}; use ic_base_types::NumSeconds; use ic_config::embedders::Config as EmbeddersConfig; use ic_config::flag_status::FlagStatus; use ic_cycles_account_manager::{CyclesAccountManager, ResourceSaturation}; -use ic_embedders::{ - wasm_utils::decoding::decode_wasm, - wasmtime_embedder::system_api::{ExecutionParameters, InstructionLimits}, -}; +use ic_embedders::wasm_utils::decoding::decode_wasm; +use ic_embedders::wasmtime_embedder::system_api::{ExecutionParameters, InstructionLimits}; use ic_error_types::{ErrorCode, RejectCode, UserError}; use ic_interfaces::execution_environment::{MessageMemoryUsage, SubnetAvailableMemory}; use ic_limits::LOG_CANISTER_OPERATION_CYCLES_THRESHOLD; @@ -38,35 +33,30 @@ use ic_management_canister_types_private::{ }; use ic_registry_provisional_whitelist::ProvisionalWhitelist; use ic_replicated_state::canister_state::WASM_PAGE_SIZE_IN_BYTES; -use ic_replicated_state::canister_state::execution_state::{CustomSectionType, SandboxMemory}; +use ic_replicated_state::canister_state::canister_snapshots::{ + CanisterSnapshot, CanisterSnapshots, ValidatedSnapshotMetadata, +}; +use ic_replicated_state::canister_state::execution_state::{ + CustomSectionType, Memory, SandboxMemory, WasmExecutionMode, +}; +use ic_replicated_state::canister_state::system_state::ReservationError; use ic_replicated_state::canister_state::system_state::wasm_chunk_store::{ - CHUNK_SIZE, ChunkValidationResult, WasmChunkHash, + self, CHUNK_SIZE, ChunkValidationResult, WasmChunkHash, WasmChunkStore, }; -use ic_replicated_state::page_map::Buffer; +use ic_replicated_state::metadata_state::subnet_call_context_manager::InstallCodeCallId; +use ic_replicated_state::page_map::{Buffer, PageAllocatorFileDescriptor}; use ic_replicated_state::{ CallOrigin, CanisterState, NetworkTopology, ReplicatedState, SchedulerState, SystemState, - canister_state::{ - NextExecution, - canister_snapshots::{CanisterSnapshot, CanisterSnapshots, ValidatedSnapshotMetadata}, - execution_state::Memory, - execution_state::WasmExecutionMode, - system_state::{ - ReservationError, - wasm_chunk_store::{self, WasmChunkStore}, - }, - }, - metadata_state::subnet_call_context_manager::InstallCodeCallId, - page_map::PageAllocatorFileDescriptor, +}; +use ic_types::ingress::{IngressState, IngressStatus}; +use ic_types::messages::{ + CanisterCall, Payload, RejectContext, Response as CanisterResponse, SignedIngressContent, + StopCanisterCallId, StopCanisterContext, }; use ic_types::{ CanisterId, CanisterTimer, ComputeAllocation, DEFAULT_AGGREGATE_LOG_MEMORY_LIMIT, MAX_AGGREGATE_LOG_MEMORY_LIMIT, MemoryAllocation, NumBytes, NumInstructions, PrincipalId, SnapshotId, Time, - ingress::{IngressState, IngressStatus}, - messages::{ - CanisterCall, Payload, RejectContext, Response as CanisterResponse, SignedIngressContent, - StopCanisterCallId, StopCanisterContext, - }, }; use ic_types_cycles::{ CanisterCreation, CanisterCyclesCostSchedule, CompoundCycles, Cycles, CyclesUseCase, @@ -2280,17 +2270,14 @@ impl CanisterManager { // Check the precondition: // Unable to start executing a `load_canister_snapshot` // if there is already a long-running message in progress for the specified canister. - match canister.next_execution() { - NextExecution::None | NextExecution::StartNew => {} - NextExecution::ContinueLong | NextExecution::ContinueInstallCode => { - metrics.long_execution_already_in_progress.inc(); - error!( - self.log, - "[EXC-BUG] Attempted to start a new `load_canister_snapshot` execution while the previous execution is still in progress for {}.", - canister_id - ); - return Err(CanisterManagerError::LongExecutionAlreadyInProgress { canister_id }); - } + if canister.has_long_execution_or_install_code() { + metrics.long_execution_already_in_progress.inc(); + error!( + self.log, + "[EXC-BUG] Attempted to start a new `load_canister_snapshot` execution while the previous execution is still in progress for {}.", + canister_id + ); + return Err(CanisterManagerError::LongExecutionAlreadyInProgress { canister_id }); } // All basic checks have passed, prepay cycles for instructions. diff --git a/rs/execution_environment/src/execution_environment.rs b/rs/execution_environment/src/execution_environment.rs index ca966c21420a..f22e7c955239 100644 --- a/rs/execution_environment/src/execution_environment.rs +++ b/rs/execution_environment/src/execution_environment.rs @@ -1,24 +1,20 @@ -use crate::{ - canister_logs::fetch_canister_logs, - canister_manager::{ - CanisterManager, - types::{ - CanisterManagerError, CanisterManagerResponse, DtsInstallCodeResult, - InstallCodeContext, PausedInstallCodeExecution, UploadChunkResult, - }, - }, - canister_settings::CanisterSettings, - execution::{ - call_or_task::execute_call_or_task, common::validate_controller, inspect_message, - response::execute_response, - }, - execution_environment_metrics::{ - ExecutionEnvironmentMetrics, SUBMITTED_OUTCOME_LABEL, SUCCESS_STATUS_LABEL, - }, - hypervisor::Hypervisor, - ic00_permissions::Ic00MethodPermissions, - metrics::{CallTreeMetrics, CallTreeMetricsImpl, IngressFilterMetrics}, +use crate::canister_logs::fetch_canister_logs; +use crate::canister_manager::CanisterManager; +use crate::canister_manager::types::{ + CanisterManagerError, CanisterManagerResponse, DtsInstallCodeResult, InstallCodeContext, + PausedInstallCodeExecution, UploadChunkResult, +}; +use crate::canister_settings::CanisterSettings; +use crate::execution::call_or_task::execute_call_or_task; +use crate::execution::common::validate_controller; +use crate::execution::inspect_message; +use crate::execution::response::execute_response; +use crate::execution_environment_metrics::{ + ExecutionEnvironmentMetrics, SUBMITTED_OUTCOME_LABEL, SUCCESS_STATUS_LABEL, }; +use crate::hypervisor::Hypervisor; +use crate::ic00_permissions::Ic00MethodPermissions; +use crate::metrics::{CallTreeMetrics, CallTreeMetricsImpl, IngressFilterMetrics}; use candid::Encode; use ic_base_types::PrincipalId; use ic_config::execution_environment::Config as ExecutionConfig; @@ -52,37 +48,35 @@ use ic_metrics::MetricsRegistry; use ic_registry_provisional_whitelist::ProvisionalWhitelist; use ic_registry_resource_limits::ResourceLimits; use ic_registry_subnet_type::SubnetType; +use ic_replicated_state::canister_state::{NextExecution, system_state::PausedExecutionId}; +use ic_replicated_state::metadata_state::subnet_call_context_manager::{ + EcdsaArguments, InstallCodeCall, InstallCodeCallId, PreSignatureStash, ReshareChainKeyContext, + SchnorrArguments, SetupInitialDkgContext, SignWithThresholdContext, StopCanisterCall, + SubnetCallContext, ThresholdArguments, VetKdArguments, +}; use ic_replicated_state::{ CanisterState, CanisterStatus, ExecutionTask, NetworkTopology, ReplicatedState, - canister_state::NextExecution, - canister_state::system_state::PausedExecutionId, - metadata_state::subnet_call_context_manager::{ - EcdsaArguments, InstallCodeCall, InstallCodeCallId, PreSignatureStash, - ReshareChainKeyContext, SchnorrArguments, SetupInitialDkgContext, SignWithThresholdContext, - StopCanisterCall, SubnetCallContext, ThresholdArguments, VetKdArguments, - }, }; +use ic_types::batch::ChainKeyData; +use ic_types::canister_http::{CanisterHttpRequestContext, MAX_CANISTER_HTTP_RESPONSE_BYTES}; +use ic_types::consensus::idkg::IDkgMasterPublicKeyId; +use ic_types::crypto::{ + ExtendedDerivationPath, + canister_threshold_sig::{MasterPublicKey, PublicKey}, + threshold_sig::ni_dkg::{NiDkgMasterPublicKeyId, NiDkgTargetId}, +}; +use ic_types::ingress::{IngressState, IngressStatus, WasmResult}; +use ic_types::messages::{ + CanisterCall, CanisterCallOrTask, CanisterMessage, CanisterMessageOrTask, CanisterTask, + MAX_INTER_CANISTER_PAYLOAD_IN_BYTES, MessageId, Payload, RejectContext, Request, Response, + SignedIngress, StopCanisterCallId, StopCanisterContext, SubnetMessage, + extract_effective_canister_id, +}; +use ic_types::methods::{Callback, SystemMethod, WasmMethod}; use ic_types::{ CanisterId, ExecutionRound, Height, NumBytes, NumInstructions, RegistryVersion, ReplicaVersion, SubnetId, Time, - batch::ChainKeyData, - canister_http::{CanisterHttpRequestContext, MAX_CANISTER_HTTP_RESPONSE_BYTES}, - consensus::idkg::IDkgMasterPublicKeyId, - crypto::{ - ExtendedDerivationPath, - canister_threshold_sig::{MasterPublicKey, PublicKey}, - threshold_sig::ni_dkg::{NiDkgMasterPublicKeyId, NiDkgTargetId}, - }, - ingress::{IngressState, IngressStatus, WasmResult}, - messages::{ - CanisterCall, CanisterCallOrTask, CanisterMessage, CanisterMessageOrTask, CanisterTask, - MAX_INTER_CANISTER_PAYLOAD_IN_BYTES, Payload, RejectContext, Request, Response, - SignedIngress, StopCanisterCallId, StopCanisterContext, SubnetMessage, - extract_effective_canister_id, - }, - methods::{Callback, SystemMethod}, }; -use ic_types::{messages::MessageId, methods::WasmMethod}; use ic_types_cycles::{ CanisterCyclesCostSchedule, CompoundCycles, Cycles, CyclesUseCase, ECDSAOutcalls, Instructions, NominalCycles, SchnorrOutcalls, VetKd, @@ -92,15 +86,13 @@ use ic_wasm_types::WasmHash; use phantom_newtype::AmountOf; use prometheus::IntCounter; use rand::RngCore; +use std::collections::{BTreeMap, BTreeSet, HashMap}; +use std::convert::{Into, TryFrom}; +use std::fmt; use std::num::NonZeroU64; -use std::{ - collections::{BTreeMap, BTreeSet, HashMap}, - convert::{Into, TryFrom}, - fmt, - str::FromStr, - sync::{Arc, Mutex}, - time::{Duration, Instant}, -}; +use std::str::FromStr; +use std::sync::{Arc, Mutex}; +use std::time::{Duration, Instant}; use strum::ParseError; #[cfg(test)] @@ -2088,16 +2080,11 @@ impl ExecutionEnvironment { subnet_size: usize, cost_schedule: CanisterCyclesCostSchedule, ) -> ExecuteMessageResult { - match canister.next_execution() { - NextExecution::None | NextExecution::StartNew => {} - NextExecution::ContinueLong | NextExecution::ContinueInstallCode => { - // We should never try to execute a canister message in - // replicated mode if there is a pending long execution. - panic!( - "Replicated execution with another pending DTS execution: {:?}", - canister.next_execution() - ); - } + if canister.has_long_execution_or_install_code() { + panic!( + "Replicated execution with a pending DTS task: {:?}", + canister.system_state.task_queue.paused_or_aborted_task() + ); } let round_counters = RoundCounters { @@ -3830,13 +3817,10 @@ impl ExecutionEnvironment { }; // Check the precondition. - match old_canister.next_execution() { - NextExecution::None | NextExecution::StartNew => {} - NextExecution::ContinueLong | NextExecution::ContinueInstallCode => { - panic!( - "Attempt to start a new `install_code` execution while the previous execution is still in progress." - ); - } + if old_canister.has_long_execution_or_install_code() { + panic!( + "Attempt to start a new `install_code` execution while the previous execution is still in progress." + ); } let canister_id = old_canister.canister_id(); diff --git a/rs/execution_environment/src/scheduler.rs b/rs/execution_environment/src/scheduler.rs index 70e19c91341c..00ddf8dacc4d 100644 --- a/rs/execution_environment/src/scheduler.rs +++ b/rs/execution_environment/src/scheduler.rs @@ -199,14 +199,11 @@ impl SchedulerImpl { ) -> ReplicatedState { let mut ongoing_long_install_code = false; for canister_id in long_running_canisters.iter() { - match state.canister_state(canister_id) { - None => continue, - Some(canister) => match canister.next_execution() { - NextExecution::None | NextExecution::StartNew | NextExecution::ContinueLong => { - continue; - } - NextExecution::ContinueInstallCode => {} - }, + let Some(canister) = state.canister_state(canister_id) else { + continue; + }; + if !canister.has_long_install_code() { + continue; } let instruction_limits = InstructionLimits::new( self.config.max_instructions_per_install_code, @@ -257,15 +254,10 @@ impl SchedulerImpl { replica_version: &ReplicaVersion, chain_key_data: &ChainKeyData, ) -> ReplicatedState { - let mut ongoing_long_install_code = - state - .canisters_iter() - .any(|canister| match canister.next_execution() { - NextExecution::None | NextExecution::StartNew | NextExecution::ContinueLong => { - false - } - NextExecution::ContinueInstallCode => true, - }); + let mut ongoing_long_install_code = state + .canisters_iter() + .any(|canister| canister.has_long_install_code()); + loop { let mut available_subnet_messages = false; let mut loop_detector = state.subnet_queues_loop_detector(); @@ -380,38 +372,35 @@ impl SchedulerImpl { let has_heartbeat = has_heartbeat(canister); let has_active_timer = has_active_timer(canister, now); if !has_heartbeat && !has_active_timer { - // Canister has no heartbeat and no active global timer. + // No heartbeat and no active global timer. continue; } - match canister.next_execution() { - NextExecution::ContinueLong | NextExecution::ContinueInstallCode => { - // Do not add a heartbeat task if a long execution is pending. - } - NextExecution::None | NextExecution::StartNew => { - // Skip canisters that don't have enough cycles to execute a heartbeat/timer. - if self - .cycles_account_manager - .can_prepay_execution_cycles( - canister, - self.config.max_instructions_per_message, - subnet_size, - cost_schedule, - ) - .is_err() - { - continue; - } - - let canister = Arc::make_mut(canister); - maybe_add_heartbeat_or_global_timer_tasks( - canister, - has_heartbeat, - has_active_timer, - &mut heartbeat_and_timer_canisters, - ); - } + // Skip canisters with pending long execution or install code. + if canister.has_long_execution_or_install_code() { + continue; } + // Skip canisters that don't have enough cycles to execute a heartbeat/timer. + if self + .cycles_account_manager + .can_prepay_execution_cycles( + canister, + self.config.max_instructions_per_message, + subnet_size, + cost_schedule, + ) + .is_err() + { + continue; + } + + let canister = Arc::make_mut(canister); + maybe_add_heartbeat_or_global_timer_tasks( + canister, + has_heartbeat, + has_active_timer, + &mut heartbeat_and_timer_canisters, + ); } heartbeat_and_timer_canisters } @@ -837,7 +826,7 @@ impl SchedulerImpl { // Postpone charging for resources when a canister has a paused execution // to avoid modifying the balance of a canister during an unfinished operation. - if canister.has_paused_execution() || canister.has_paused_install_code() { + if canister.has_paused_execution_or_install_code() { continue; } @@ -1197,10 +1186,11 @@ impl Scheduler for SchedulerImpl { long_running_canisters = state .canister_states() .iter() - .filter_map(|(&canister_id, canister)| match canister.next_execution() { - NextExecution::None | NextExecution::StartNew => None, - NextExecution::ContinueLong | NextExecution::ContinueInstallCode => { - Some(canister_id) + .filter_map(|(canister_id, canister)| { + if canister.has_long_execution_or_install_code() { + Some(*canister_id) + } else { + None } }) .collect(); @@ -1211,9 +1201,7 @@ impl Scheduler for SchedulerImpl { let has_any_paused_execution = long_running_canisters.iter().any(|canister_id| { state .canister_state(canister_id) - .map(|canister| { - canister.has_paused_execution() || canister.has_paused_install_code() - }) + .map(|canister| canister.has_paused_execution_or_install_code()) .unwrap_or(false) }); if !has_any_paused_execution { diff --git a/rs/execution_environment/src/scheduler/round_schedule.rs b/rs/execution_environment/src/scheduler/round_schedule.rs index 58d71013b1cb..ddb9a57e1d13 100644 --- a/rs/execution_environment/src/scheduler/round_schedule.rs +++ b/rs/execution_environment/src/scheduler/round_schedule.rs @@ -49,10 +49,8 @@ pub(super) struct CanisterRoundState { compute_allocation: AccumulatedPriority, /// Copy of the canister's `CanisterPriority::long_execution_mode`. long_execution_mode: LongExecutionMode, - /// The canister's next execution. We're interested in whether that's - /// `StartNew`, `ContinueLong`, or something else (both `None` and - /// `ContinueInstallCode` count as idle). - next_execution: NextExecution, + /// Whether the canister has a long execution. + has_long_execution: bool, } impl CanisterRoundState { @@ -63,17 +61,13 @@ impl CanisterRoundState { accumulated_priority: canister_priority.accumulated_priority + compute_allocation, compute_allocation, long_execution_mode: canister_priority.long_execution_mode, - next_execution: canister.next_execution(), + has_long_execution: canister.has_long_execution(), } } pub(super) fn canister_id(&self) -> CanisterId { self.canister_id } - - fn has_long_execution(&self) -> bool { - self.next_execution == NextExecution::ContinueLong - } } impl Ord for CanisterRoundState { @@ -84,7 +78,7 @@ impl Ord for CanisterRoundState { .long_execution_mode .cmp(&self.long_execution_mode) // 2. Long execution (long execution -> new execution) - .then(other.has_long_execution().cmp(&self.has_long_execution())) + .then(other.has_long_execution.cmp(&self.has_long_execution)) // 3. Accumulated priority, descending. .then(other.accumulated_priority.cmp(&self.accumulated_priority)) // 4. Canister ID, ascending. @@ -447,7 +441,7 @@ impl RoundSchedule { // Apply the priority credit if not in the same long execution as at the // beginning of the round. if canister_priority.priority_credit != ZERO - && (canister.next_execution() != NextExecution::ContinueLong + && (!canister.has_long_execution() || self .canisters_with_completed_messages .contains(&canister.canister_id())) @@ -678,7 +672,7 @@ impl RoundSchedule { // De-facto compute allocation includes bonus allocation let factual = rs.compute_allocation + free_capacity_per_canister; // Count long executions and sum up their compute allocation. - if rs.has_long_execution() { + if rs.has_long_execution { long_executions_compute_allocation += factual; number_of_long_executions += 1; } diff --git a/rs/execution_environment/src/scheduler/tests.rs b/rs/execution_environment/src/scheduler/tests.rs index 1e2376bac67d..6cb051bb9a5b 100644 --- a/rs/execution_environment/src/scheduler/tests.rs +++ b/rs/execution_environment/src/scheduler/tests.rs @@ -67,7 +67,7 @@ fn state_sync_clears_paused_execution_registry() { // state with the clean copy. The registry still holds the orphaned paused // execution entry. test.state_mut().put_canister_state(clean_canister); - assert!(!test.canister_state(canister).has_paused_execution()); + assert!(!test.canister_state(canister).has_long_execution()); // Execute another round. The scheduler detects that no canister has a paused // execution and calls `abandon_paused_executions()` to clear the paused diff --git a/rs/execution_environment/src/scheduler/tests/dts.rs b/rs/execution_environment/src/scheduler/tests/dts.rs index e6718785fb25..7f5852627ad1 100644 --- a/rs/execution_environment/src/scheduler/tests/dts.rs +++ b/rs/execution_environment/src/scheduler/tests/dts.rs @@ -237,8 +237,7 @@ fn dts_long_execution_aborted_after_checkpoint() { // After completion, there is no paused or aborted execution. And the priority // credit is again zero. - assert!(!test.canister_state(canister).has_paused_execution()); - assert!(!test.canister_state(canister).has_aborted_execution()); + assert!(!test.canister_state(canister).has_long_execution()); assert_eq!( test.state() .canister_priority(&canister) @@ -455,7 +454,7 @@ fn filter_after_long_executions() { test.execute_round(ExecutionRoundType::OrdinaryRound); for canister in test.state().canisters_iter() { assert_eq!(canister.system_state.canister_metrics().executed(), 2); - assert!(!canister.has_paused_execution()); + assert!(!canister.has_long_execution()); } } @@ -609,8 +608,7 @@ fn dts_resume_install_code_after_abort() { } test.execute_round(ExecutionRoundType::OrdinaryRound); - assert!(!test.canister_state(canister).has_paused_install_code()); - assert!(!test.canister_state(canister).has_aborted_install_code()); + assert!(!test.canister_state(canister).has_long_install_code()); // After 1 + 9 rounds we had a paused install code. assert_eq!( @@ -667,7 +665,6 @@ fn dts_resume_long_execution_after_abort() { assert_eq!(execution_stats(&test, canister), (1, true)); test.execute_round(ExecutionRoundType::CheckpointRound); - assert!(!test.canister_state(canister).has_paused_execution()); assert!(test.canister_state(canister).has_aborted_execution()); assert_eq!(execution_stats(&test, canister), (2, true)); diff --git a/rs/replicated_state/src/canister_state.rs b/rs/replicated_state/src/canister_state.rs index 21c7b91af1a8..d1e00638e2fc 100644 --- a/rs/replicated_state/src/canister_state.rs +++ b/rs/replicated_state/src/canister_state.rs @@ -169,6 +169,10 @@ impl CanisterState { } /// Returns what the canister is going to execute next. + /// + /// Only use this when the difference between `StartNew` and `None` is relevant. + /// Otherwise, use `next_task()` or one of the `has_*` methods, they are + /// slightly more efficient. pub fn next_execution(&self) -> NextExecution { match self.system_state.task_queue.front() { Some(ExecutionTask::Heartbeat) @@ -194,58 +198,78 @@ impl CanisterState { /// Returns true if the canister has an aborted execution. pub fn has_aborted_execution(&self) -> bool { - match self.system_state.task_queue.front() { - Some(ExecutionTask::AbortedExecution { .. }) => true, - None - | Some(ExecutionTask::Heartbeat) - | Some(ExecutionTask::GlobalTimer) - | Some(ExecutionTask::OnLowWasmMemory) - | Some(ExecutionTask::PausedExecution { .. }) - | Some(ExecutionTask::PausedInstallCode(..)) - | Some(ExecutionTask::AbortedInstallCode { .. }) => false, - } + use ExecutionTask::*; + matches!( + self.system_state.task_queue.paused_or_aborted_task(), + Some(AbortedExecution { .. }) + ) } /// Returns true if the canister has a paused execution. pub fn has_paused_execution(&self) -> bool { - match self.system_state.task_queue.front() { - Some(ExecutionTask::PausedExecution { .. }) => true, - None - | Some(ExecutionTask::Heartbeat) - | Some(ExecutionTask::GlobalTimer) - | Some(ExecutionTask::OnLowWasmMemory) - | Some(ExecutionTask::PausedInstallCode(..)) - | Some(ExecutionTask::AbortedExecution { .. }) - | Some(ExecutionTask::AbortedInstallCode { .. }) => false, - } + use ExecutionTask::*; + matches!( + self.system_state.task_queue.paused_or_aborted_task(), + Some(PausedExecution { .. }) + ) + } + + /// Returns true if the canister has a paused or aborted execution. + pub fn has_long_execution(&self) -> bool { + use ExecutionTask::*; + matches!( + self.system_state.task_queue.paused_or_aborted_task(), + Some(PausedExecution { .. }) | Some(AbortedExecution { .. }) + ) } /// Returns true if the canister has a paused install code. pub fn has_paused_install_code(&self) -> bool { - match self.system_state.task_queue.front() { - Some(ExecutionTask::PausedInstallCode(..)) => true, - None - | Some(ExecutionTask::Heartbeat) - | Some(ExecutionTask::GlobalTimer) - | Some(ExecutionTask::OnLowWasmMemory) - | Some(ExecutionTask::PausedExecution { .. }) - | Some(ExecutionTask::AbortedExecution { .. }) - | Some(ExecutionTask::AbortedInstallCode { .. }) => false, - } + use ExecutionTask::*; + matches!( + self.system_state.task_queue.paused_or_aborted_task(), + Some(PausedInstallCode(_)) + ) } /// Returns true if the canister has an aborted install code. pub fn has_aborted_install_code(&self) -> bool { - match self.system_state.task_queue.front() { - Some(ExecutionTask::AbortedInstallCode { .. }) => true, - None - | Some(ExecutionTask::Heartbeat) - | Some(ExecutionTask::GlobalTimer) - | Some(ExecutionTask::OnLowWasmMemory) - | Some(ExecutionTask::PausedExecution { .. }) - | Some(ExecutionTask::PausedInstallCode(..)) - | Some(ExecutionTask::AbortedExecution { .. }) => false, - } + use ExecutionTask::*; + matches!( + self.system_state.task_queue.paused_or_aborted_task(), + Some(AbortedInstallCode { .. }) + ) + } + + /// Returns true if the canister has a paused or aborted install code. + pub fn has_long_install_code(&self) -> bool { + use ExecutionTask::*; + matches!( + self.system_state.task_queue.paused_or_aborted_task(), + Some(ExecutionTask::PausedInstallCode { .. }) | Some(AbortedInstallCode { .. }) + ) + } + + /// Returns true if the canister has a paused or aborted execution or install + /// code. + pub fn has_long_execution_or_install_code(&self) -> bool { + use ExecutionTask::*; + matches!( + self.system_state.task_queue.paused_or_aborted_task(), + Some(ExecutionTask::PausedExecution { .. }) + | Some(PausedInstallCode(_)) + | Some(AbortedExecution { .. }) + | Some(AbortedInstallCode { .. }) + ) + } + + /// Returns true if the canister has a paused execution or paused install code. + pub fn has_paused_execution_or_install_code(&self) -> bool { + use ExecutionTask::*; + matches!( + self.system_state.task_queue.paused_or_aborted_task(), + Some(PausedExecution { .. }) | Some(PausedInstallCode(_)) + ) } /// Returns true if there is at least one message in the canister's output diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 13d11c2dd822..22f48dd6320a 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -2038,7 +2038,7 @@ impl SystemState { /// Returns the aborted or paused `Request` at the head of the task queue, if /// any. fn aborted_or_paused_request(&self) -> Option<&Request> { - match self.task_queue.front() { + match self.task_queue.paused_or_aborted_task() { Some(ExecutionTask::AbortedExecution { input: CanisterMessageOrTask::Message(CanisterMessage::Request(request)), .. diff --git a/rs/replicated_state/src/canister_state/system_state/task_queue.rs b/rs/replicated_state/src/canister_state/system_state/task_queue.rs index 135f19d58526..9fe9766c3605 100644 --- a/rs/replicated_state/src/canister_state/system_state/task_queue.rs +++ b/rs/replicated_state/src/canister_state/system_state/task_queue.rs @@ -47,6 +47,10 @@ impl TaskQueue { }) } + pub fn paused_or_aborted_task(&self) -> &Option { + &self.paused_or_aborted_task + } + pub fn has_paused_or_aborted_task(&self) -> bool { self.paused_or_aborted_task.is_some() } diff --git a/rs/replicated_state/src/metrics.rs b/rs/replicated_state/src/metrics.rs index 76b47afee312..d555d03d8538 100644 --- a/rs/replicated_state/src/metrics.rs +++ b/rs/replicated_state/src/metrics.rs @@ -342,22 +342,22 @@ impl ReplicatedStateMetrics { } CanisterStatus::Stopped => num_stopped_canisters += 1, } - match canister.next_task() { - Some(&ExecutionTask::PausedExecution { .. }) => { + match canister.system_state.task_queue.paused_or_aborted_task() { + Some(ExecutionTask::PausedExecution { .. }) => { num_paused_exec += 1; } - Some(&ExecutionTask::PausedInstallCode(_)) => { + Some(ExecutionTask::PausedInstallCode(_)) => { num_paused_install += 1; } - Some(&ExecutionTask::AbortedExecution { .. }) => { + Some(ExecutionTask::AbortedExecution { .. }) => { num_aborted_exec += 1; } - Some(&ExecutionTask::AbortedInstallCode { .. }) => { + Some(ExecutionTask::AbortedInstallCode { .. }) => { num_aborted_install += 1; } - Some(&ExecutionTask::Heartbeat) - | Some(&ExecutionTask::GlobalTimer) - | Some(&ExecutionTask::OnLowWasmMemory) + Some(ExecutionTask::Heartbeat) + | Some(ExecutionTask::GlobalTimer) + | Some(ExecutionTask::OnLowWasmMemory) | None => {} } consumed_cycles_total += canister.system_state.canister_metrics().consumed_cycles(); diff --git a/rs/test_utilities/execution_environment/src/lib.rs b/rs/test_utilities/execution_environment/src/lib.rs index bdb3da85856e..af128f4888fc 100644 --- a/rs/test_utilities/execution_environment/src/lib.rs +++ b/rs/test_utilities/execution_environment/src/lib.rs @@ -1838,9 +1838,9 @@ impl ExecutionTest { pub fn execute_message(&mut self, canister_id: CanisterId) { self.execute_slice(canister_id); self.state.as_mut().unwrap().metadata.batch_time += std::time::Duration::from_secs(1); - while self.canister_state(canister_id).next_execution() == NextExecution::ContinueLong - || self.canister_state(canister_id).next_execution() - == NextExecution::ContinueInstallCode + while self + .canister_state(canister_id) + .has_long_execution_or_install_code() { self.execute_slice(canister_id); self.state.as_mut().unwrap().metadata.batch_time += std::time::Duration::from_secs(1);