From 78afb37ae0ce542b35361ffcb8ae6a10de651f4e Mon Sep 17 00:00:00 2001 From: Dragoljub Djuric Date: Tue, 24 Sep 2024 22:31:49 +0200 Subject: [PATCH] feat: [EXC-1669] Propagate hook execution status to SystemState (#667) This PR is part of the OnLowWasmMemoryHook effort. Here we implement checking for hook condition and propagating hook status to SystemState. Condition for executing hook, as agreed in interface spec [discussion](https://github.com/dfinity/interface-spec/pull/286#discussion_r1666870058) is: 1. In the case with memory_allocation `min(memory_allocation - used_stable_memory, wasm_memory_limit) - used_wasm_memory` 2. Without memory allocation `wasm_memory_limit - used_wasm_memory` Hook status can be one of: 1. Condition is not satisfied 3. Ready for execution 4. Executed The hook condition is checked whenever additional execution (stable or Wasm) memory allocation is requested. After this change, we will have all the required information of SystemState and it will be necessary only to schedule hook execution in the following round. This PR will be followed with: 1. [EXC-1724](https://dfinity.atlassian.net/browse/EXC-1724) In which we will refactor `SystemState::task_queue` to be new struct with additional field `on_low_wasm_memory_hook_status`, to ensure that whenever available first task that will be poped form task queue is `OnLowWasmMemoryHook` (excluding paused executions). For that reason `SystemState::on_low_wasm_memory_hook status` is private, and we use `set` and `get`, so it can be easier encapsulated in the future `TaskQueue` struct. 2. [EXC-1725](https://dfinity.atlassian.net/browse/EXC-1725) Schedule and execute `OnLowWasmMemoryHook` [EXC-1724]: https://dfinity.atlassian.net/browse/EXC-1724?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ [EXC-1725]: https://dfinity.atlassian.net/browse/EXC-1725?atlOrigin=eyJpIjoiNWRkNTljNzYxNjVmNDY3MDlhMDU5Y2ZhYzA5YTRkZjUiLCJwIjoiZ2l0aHViLWNvbS1KU1cifQ --------- Co-authored-by: Alin Sinpalean <58422065+alin-at-dfinity@users.noreply.github.com> --- rs/canister_sandbox/src/sandbox_server.rs | 6 +- rs/embedders/src/wasm_executor.rs | 1 + .../wasmtime_embedder_tests.rs | 2 + .../tests/wasmtime_random_memory_writes.rs | 1 + .../src/canister_manager.rs | 3 +- .../v1/canister_state_bits.proto | 8 + .../gen/state/state.canister_state_bits.v1.rs | 38 +++ .../src/canister_state/system_state.rs | 56 ++++ rs/state_layout/src/state_layout.rs | 29 +- rs/state_layout/src/state_layout/tests.rs | 7 +- rs/state_manager/src/checkpoint.rs | 10 +- rs/state_manager/src/tip.rs | 3 + rs/system_api/src/lib.rs | 123 +++++++-- .../src/sandbox_safe_system_state.rs | 202 +++++++++++++- rs/system_api/tests/common/mod.rs | 2 + rs/system_api/tests/system_api.rs | 253 +++++++++++++++++- rs/test_utilities/embedders/src/lib.rs | 2 + rs/test_utilities/state/src/lib.rs | 24 +- 18 files changed, 728 insertions(+), 42 deletions(-) diff --git a/rs/canister_sandbox/src/sandbox_server.rs b/rs/canister_sandbox/src/sandbox_server.rs index 76738376dcd..a5fc7adbbac 100644 --- a/rs/canister_sandbox/src/sandbox_server.rs +++ b/rs/canister_sandbox/src/sandbox_server.rs @@ -153,7 +153,10 @@ mod tests { use ic_limits::SMALL_APP_SUBNET_MAX_SIZE; use ic_logger::replica_logger::no_op_logger; use ic_registry_subnet_type::SubnetType; - use ic_replicated_state::{Global, NumWasmPages, PageIndex, PageMap}; + use ic_replicated_state::{ + canister_state::system_state::OnLowWasmMemoryHookStatus, Global, NumWasmPages, PageIndex, + PageMap, + }; use ic_system_api::{ sandbox_safe_system_state::{CanisterStatusView, SandboxSafeSystemState}, ApiType, ExecutionParameters, InstructionLimits, @@ -228,6 +231,7 @@ mod tests { RequestMetadata::new(0, Time::from_nanos_since_unix_epoch(0)), caller, 0, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, ) } diff --git a/rs/embedders/src/wasm_executor.rs b/rs/embedders/src/wasm_executor.rs index 33d09773a4d..7aff93b8d7d 100644 --- a/rs/embedders/src/wasm_executor.rs +++ b/rs/embedders/src/wasm_executor.rs @@ -597,6 +597,7 @@ pub fn process( embedder.config().feature_flags.canister_backtrace, embedder.config().max_sum_exported_function_name_lengths, stable_memory.clone(), + wasm_memory.size, out_of_instructions_handler, logger, ); diff --git a/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs b/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs index 8a93f032baf..74d98ee3432 100644 --- a/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs +++ b/rs/embedders/src/wasmtime_embedder/wasmtime_embedder_tests.rs @@ -24,6 +24,7 @@ use ic_types::{ }; use ic_wasm_types::BinaryEncodedWasm; +use ic_replicated_state::NumWasmPages; use lazy_static::lazy_static; use wasmtime::{Engine, Module, Store, StoreLimits, Val}; @@ -91,6 +92,7 @@ fn test_wasmtime_system_api() { EmbeddersConfig::default().feature_flags.canister_backtrace, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), + NumWasmPages::from(0), Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ); diff --git a/rs/embedders/tests/wasmtime_random_memory_writes.rs b/rs/embedders/tests/wasmtime_random_memory_writes.rs index 3c4a38eb31c..1eb9729ce1e 100644 --- a/rs/embedders/tests/wasmtime_random_memory_writes.rs +++ b/rs/embedders/tests/wasmtime_random_memory_writes.rs @@ -111,6 +111,7 @@ fn test_api_for_update( EmbeddersConfig::default().feature_flags.canister_backtrace, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), + NumWasmPages::from(0), Rc::new(DefaultOutOfInstructionsHandler::new(instruction_limit)), log, ) diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index a6f47fa084f..a96ff3dc5af 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -28,14 +28,13 @@ use ic_management_canister_types::{ }; use ic_registry_provisional_whitelist::ProvisionalWhitelist; use ic_registry_subnet_type::SubnetType; -use ic_replicated_state::canister_state::system_state::ReservationError; use ic_replicated_state::{ canister_snapshots::{CanisterSnapshot, CanisterSnapshotError}, canister_state::{ execution_state::Memory, system_state::{ wasm_chunk_store::{self, WasmChunkStore}, - CyclesUseCase, + CyclesUseCase, ReservationError, }, NextExecution, }, diff --git a/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto b/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto index 75def4f95eb..36a8afbae41 100644 --- a/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto +++ b/rs/protobuf/def/state/canister_state_bits/v1/canister_state_bits.proto @@ -341,6 +341,13 @@ enum LongExecutionMode { LONG_EXECUTION_MODE_PRIORITIZED = 2; } +enum OnLowWasmMemoryHookStatus { + ON_LOW_WASM_MEMORY_HOOK_STATUS_UNSPECIFIED = 0; + ON_LOW_WASM_MEMORY_HOOK_STATUS_CONDITION_NOT_SATISFIED = 1; + ON_LOW_WASM_MEMORY_HOOK_STATUS_READY = 2; + ON_LOW_WASM_MEMORY_HOOK_STATUS_EXECUTED = 3; +} + message CanisterStateBits { reserved 1; reserved "controller"; @@ -430,4 +437,5 @@ message CanisterStateBits { int64 priority_credit = 48; LongExecutionMode long_execution_mode = 49; optional uint64 wasm_memory_threshold = 50; + optional OnLowWasmMemoryHookStatus on_low_wasm_memory_hook_status = 53; } diff --git a/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs b/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs index e978703b739..75d14eec2b8 100644 --- a/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs +++ b/rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs @@ -700,6 +700,8 @@ pub struct CanisterStateBits { pub long_execution_mode: i32, #[prost(uint64, optional, tag = "50")] pub wasm_memory_threshold: ::core::option::Option, + #[prost(enumeration = "OnLowWasmMemoryHookStatus", optional, tag = "53")] + pub on_low_wasm_memory_hook_status: ::core::option::Option, #[prost(oneof = "canister_state_bits::CanisterStatus", tags = "11, 12, 13")] pub canister_status: ::core::option::Option, } @@ -872,3 +874,39 @@ impl LongExecutionMode { } } } +#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord, ::prost::Enumeration)] +#[repr(i32)] +pub enum OnLowWasmMemoryHookStatus { + Unspecified = 0, + ConditionNotSatisfied = 1, + Ready = 2, + Executed = 3, +} +impl OnLowWasmMemoryHookStatus { + /// String value of the enum field names used in the ProtoBuf definition. + /// + /// The values are not transformed in any way and thus are considered stable + /// (if the ProtoBuf definition does not change) and safe for programmatic use. + pub fn as_str_name(&self) -> &'static str { + match self { + OnLowWasmMemoryHookStatus::Unspecified => "ON_LOW_WASM_MEMORY_HOOK_STATUS_UNSPECIFIED", + OnLowWasmMemoryHookStatus::ConditionNotSatisfied => { + "ON_LOW_WASM_MEMORY_HOOK_STATUS_CONDITION_NOT_SATISFIED" + } + OnLowWasmMemoryHookStatus::Ready => "ON_LOW_WASM_MEMORY_HOOK_STATUS_READY", + OnLowWasmMemoryHookStatus::Executed => "ON_LOW_WASM_MEMORY_HOOK_STATUS_EXECUTED", + } + } + /// Creates an enum from field names used in the ProtoBuf definition. + pub fn from_str_name(value: &str) -> ::core::option::Option { + match value { + "ON_LOW_WASM_MEMORY_HOOK_STATUS_UNSPECIFIED" => Some(Self::Unspecified), + "ON_LOW_WASM_MEMORY_HOOK_STATUS_CONDITION_NOT_SATISFIED" => { + Some(Self::ConditionNotSatisfied) + } + "ON_LOW_WASM_MEMORY_HOOK_STATUS_READY" => Some(Self::Ready), + "ON_LOW_WASM_MEMORY_HOOK_STATUS_EXECUTED" => Some(Self::Executed), + _ => None, + } + } +} diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 896ebfebdfa..541ac4d0b3f 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -371,6 +371,51 @@ pub struct SystemState { /// This amount contributes to the total `memory_usage` of the canister as /// reported by `CanisterState::memory_usage`. pub snapshots_memory_usage: NumBytes, + + /// Status of low_on_wasm_memory hook execution. + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, +} + +/// A wrapper around the different statuses of `OnLowWasmMemory` hook execution. +#[derive(Clone, Copy, Eq, PartialEq, Debug, Default, Deserialize, Serialize)] +pub enum OnLowWasmMemoryHookStatus { + #[default] + ConditionNotSatisfied, + Ready, + Executed, +} + +impl From<&OnLowWasmMemoryHookStatus> for pb::OnLowWasmMemoryHookStatus { + fn from(item: &OnLowWasmMemoryHookStatus) -> Self { + use OnLowWasmMemoryHookStatus::*; + + match *item { + ConditionNotSatisfied => Self::ConditionNotSatisfied, + Ready => Self::Ready, + Executed => Self::Executed, + } + } +} + +impl TryFrom for OnLowWasmMemoryHookStatus { + type Error = ProxyDecodeError; + + fn try_from(value: pb::OnLowWasmMemoryHookStatus) -> Result { + match value { + pb::OnLowWasmMemoryHookStatus::Unspecified => Err(ProxyDecodeError::ValueOutOfRange { + typ: "OnLowWasmMemoryHookStatus", + err: format!( + "Unexpected value of status of on low wasm memory hook: {:?}", + value + ), + }), + pb::OnLowWasmMemoryHookStatus::ConditionNotSatisfied => { + Ok(OnLowWasmMemoryHookStatus::ConditionNotSatisfied) + } + pb::OnLowWasmMemoryHookStatus::Ready => Ok(OnLowWasmMemoryHookStatus::Ready), + pb::OnLowWasmMemoryHookStatus::Executed => Ok(OnLowWasmMemoryHookStatus::Executed), + } + } } /// A wrapper around the different canister statuses. @@ -740,6 +785,7 @@ impl SystemState { wasm_memory_limit: None, next_snapshot_id: 0, snapshots_memory_usage: NumBytes::from(0), + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus::default(), } } @@ -770,6 +816,7 @@ impl SystemState { next_snapshot_id: u64, snapshots_memory_usage: NumBytes, metrics: &dyn CheckpointLoadingMetrics, + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, ) -> Self { let system_state = Self { controllers, @@ -798,6 +845,7 @@ impl SystemState { wasm_memory_limit, next_snapshot_id, snapshots_memory_usage, + on_low_wasm_memory_hook_status, }; system_state.check_invariants().unwrap_or_else(|msg| { metrics.observe_broken_soft_invariant(msg); @@ -1557,6 +1605,14 @@ impl SystemState { _ => None, } } + + pub fn set_on_low_wasm_memory_hook_status(&mut self, status: OnLowWasmMemoryHookStatus) { + self.on_low_wasm_memory_hook_status = status; + } + + pub fn get_on_low_wasm_memory_hook_status(&self) -> OnLowWasmMemoryHookStatus { + self.on_low_wasm_memory_hook_status + } } /// Implements memory limits verification for pushing a canister-to-canister diff --git a/rs/state_layout/src/state_layout.rs b/rs/state_layout/src/state_layout.rs index b4ce3c04365..f515df0d2db 100644 --- a/rs/state_layout/src/state_layout.rs +++ b/rs/state_layout/src/state_layout.rs @@ -1,6 +1,3 @@ -use crate::error::LayoutError; -use crate::utils::do_copy; - use ic_base_types::{NumBytes, NumSeconds}; use ic_config::flag_status::FlagStatus; use ic_logger::{error, info, warn, ReplicaLogger}; @@ -17,7 +14,10 @@ use ic_protobuf::{ use ic_replicated_state::{ canister_state::{ execution_state::{NextScheduledMethod, WasmMetadata}, - system_state::{wasm_chunk_store::WasmChunkStoreMetadata, CanisterHistory, CyclesUseCase}, + system_state::{ + wasm_chunk_store::WasmChunkStoreMetadata, CanisterHistory, CyclesUseCase, + OnLowWasmMemoryHookStatus, + }, }, page_map::{Shard, StorageLayout, StorageResult}, CallContextManager, CanisterStatus, ExecutionTask, ExportedFunctions, Global, NumWasmPages, @@ -42,6 +42,9 @@ use std::sync::atomic::{AtomicBool, Ordering}; use std::sync::{Arc, Mutex}; use std::time::Instant; +use crate::error::LayoutError; +use crate::utils::do_copy; + #[cfg(test)] mod tests; @@ -175,6 +178,7 @@ pub struct CanisterStateBits { pub wasm_memory_limit: Option, pub next_snapshot_id: u64, pub snapshots_memory_usage: NumBytes, + pub on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, } /// This struct contains bits of the `CanisterSnapshot` that are not already @@ -2281,6 +2285,12 @@ impl From for pb_canister_state_bits::CanisterStateBits { wasm_memory_limit: item.wasm_memory_limit.map(|v| v.get()), next_snapshot_id: item.next_snapshot_id, snapshots_memory_usage: item.snapshots_memory_usage.get(), + on_low_wasm_memory_hook_status: Some( + pb_canister_state_bits::OnLowWasmMemoryHookStatus::from( + &item.on_low_wasm_memory_hook_status, + ) + .into(), + ), } } } @@ -2351,6 +2361,12 @@ impl TryFrom for CanisterStateBits { ); } + let on_low_wasm_memory_hook_status: Option< + pb_canister_state_bits::OnLowWasmMemoryHookStatus, + > = value.on_low_wasm_memory_hook_status.map(|v| { + pb_canister_state_bits::OnLowWasmMemoryHookStatus::try_from(v).unwrap_or_default() + }); + Ok(Self { controllers, last_full_execution_round: value.last_full_execution_round.into(), @@ -2433,6 +2449,11 @@ impl TryFrom for CanisterStateBits { wasm_memory_limit: value.wasm_memory_limit.map(NumBytes::from), next_snapshot_id: value.next_snapshot_id, snapshots_memory_usage: NumBytes::from(value.snapshots_memory_usage), + on_low_wasm_memory_hook_status: try_from_option_field( + on_low_wasm_memory_hook_status, + "CanisterStateBits::on_low_wasm_memory_hook_status", + ) + .unwrap_or_default(), }) } } diff --git a/rs/state_layout/src/state_layout/tests.rs b/rs/state_layout/src/state_layout/tests.rs index c9bd140a6e0..be6983b5a43 100644 --- a/rs/state_layout/src/state_layout/tests.rs +++ b/rs/state_layout/src/state_layout/tests.rs @@ -4,8 +4,10 @@ use ic_management_canister_types::{ CanisterChange, CanisterChangeDetails, CanisterChangeOrigin, CanisterInstallMode, IC_00, }; use ic_replicated_state::{ - canister_state::system_state::CanisterHistory, - metadata_state::subnet_call_context_manager::InstallCodeCallId, page_map::Shard, NumWasmPages, + canister_state::system_state::{CanisterHistory, OnLowWasmMemoryHookStatus}, + metadata_state::subnet_call_context_manager::InstallCodeCallId, + page_map::Shard, + NumWasmPages, }; use ic_test_utilities_logger::with_test_replica_logger; use ic_test_utilities_tmpdir::tmpdir; @@ -58,6 +60,7 @@ fn default_canister_state_bits() -> CanisterStateBits { wasm_memory_limit: None, next_snapshot_id: 0, snapshots_memory_usage: NumBytes::from(0), + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus::default(), } } diff --git a/rs/state_manager/src/checkpoint.rs b/rs/state_manager/src/checkpoint.rs index fc43e67e8d0..50b67ff43f3 100644 --- a/rs/state_manager/src/checkpoint.rs +++ b/rs/state_manager/src/checkpoint.rs @@ -1,7 +1,3 @@ -use crate::{ - CheckpointError, CheckpointMetrics, HasDowngrade, PageMapType, TipRequest, - CRITICAL_ERROR_CHECKPOINT_SOFT_INVARIANT_BROKEN, NUMBER_OF_CHECKPOINT_THREADS, -}; use crossbeam_channel::{unbounded, Sender}; use ic_base_types::{subnet_id_try_from_protobuf, CanisterId, SnapshotId}; use ic_config::flag_status::FlagStatus; @@ -29,6 +25,11 @@ use std::convert::TryFrom; use std::sync::Arc; use std::time::{Duration, Instant}; +use crate::{ + CheckpointError, CheckpointMetrics, HasDowngrade, PageMapType, TipRequest, + CRITICAL_ERROR_CHECKPOINT_SOFT_INVARIANT_BROKEN, NUMBER_OF_CHECKPOINT_THREADS, +}; + #[cfg(test)] mod tests; @@ -450,6 +451,7 @@ pub fn load_canister_state( canister_state_bits.next_snapshot_id, canister_state_bits.snapshots_memory_usage, metrics, + canister_state_bits.on_low_wasm_memory_hook_status, ); let canister_state = CanisterState { diff --git a/rs/state_manager/src/tip.rs b/rs/state_manager/src/tip.rs index c9a6022ec24..5c9bf330384 100644 --- a/rs/state_manager/src/tip.rs +++ b/rs/state_manager/src/tip.rs @@ -1077,6 +1077,9 @@ fn serialize_canister_to_tip( wasm_memory_limit: canister_state.system_state.wasm_memory_limit, next_snapshot_id: canister_state.system_state.next_snapshot_id, snapshots_memory_usage: canister_state.system_state.snapshots_memory_usage, + on_low_wasm_memory_hook_status: canister_state + .system_state + .get_on_low_wasm_memory_hook_status(), } .into(), )?; diff --git a/rs/system_api/src/lib.rs b/rs/system_api/src/lib.rs index 17051fe2341..382c7956ff3 100644 --- a/rs/system_api/src/lib.rs +++ b/rs/system_api/src/lib.rs @@ -1,9 +1,3 @@ -pub mod cycles_balance_change; -mod request_in_prep; -mod routing; -pub mod sandbox_safe_system_state; -mod stable_memory; - use ic_base_types::PrincipalIdBlobParseError; use ic_config::embedders::StableMemoryPageLimit; use ic_config::flag_status::FlagStatus; @@ -41,6 +35,12 @@ use std::{ rc::Rc, }; +pub mod cycles_balance_change; +mod request_in_prep; +mod routing; +pub mod sandbox_safe_system_state; +mod stable_memory; + pub const MULTIPLIER_MAX_SIZE_LOCAL_SUBNET: u64 = 5; const MAX_NON_REPLICATED_QUERY_REPLY_SIZE: NumBytes = NumBytes::new(3 << 20); const CERTIFIED_DATA_MAX_LENGTH: usize = 32; @@ -684,6 +684,12 @@ impl std::fmt::Display for ApiType { } } +#[derive(Debug, PartialEq, Eq)] +enum ExecutionMemoryType { + WasmMemory, + StableMemory, +} + /// A struct to gather the relevant fields that correspond to a canister's /// memory consumption. struct MemoryUsage { @@ -696,6 +702,12 @@ struct MemoryUsage { /// The current amount of execution memory that the canister is using. current_usage: NumBytes, + /// The current amount of stable memory that the canister is using. + stable_memory_usage: NumBytes, + + /// The current amount of Wasm memory that the canister is using. + wasm_memory_usage: NumBytes, + /// The current amount of message memory that the canister is using. current_message_usage: NumBytes, @@ -721,6 +733,8 @@ impl MemoryUsage { limit: NumBytes, wasm_memory_limit: Option, current_usage: NumBytes, + stable_memory_usage: NumBytes, + wasm_memory_usage: NumBytes, current_message_usage: NumBytes, subnet_available_memory: SubnetAvailableMemory, memory_allocation: MemoryAllocation, @@ -738,10 +752,13 @@ impl MemoryUsage { limit ); } + Self { limit, wasm_memory_limit, current_usage, + stable_memory_usage, + wasm_memory_usage, current_message_usage, subnet_available_memory, allocated_execution_memory: NumBytes::from(0), @@ -812,6 +829,7 @@ impl MemoryUsage { api_type: &ApiType, sandbox_safe_system_state: &mut SandboxSafeSystemState, subnet_memory_saturation: &ResourceSaturation, + execution_memory_type: ExecutionMemoryType, ) -> HypervisorResult<()> { let (new_usage, overflow) = self .current_usage @@ -851,6 +869,16 @@ impl MemoryUsage { ); self.current_usage = NumBytes::from(new_usage); self.allocated_execution_memory += execution_bytes; + + self.add_execution_memory(execution_bytes, execution_memory_type)?; + + sandbox_safe_system_state.update_on_low_wasm_memory_hook_status( + None, + self.wasm_memory_limit, + self.stable_memory_usage, + self.wasm_memory_usage, + ); + Ok(()) } Err(_err) => Err(HypervisorError::OutOfMemory), @@ -869,11 +897,34 @@ impl MemoryUsage { return Err(HypervisorError::OutOfMemory); } self.current_usage = NumBytes::from(new_usage); + self.add_execution_memory(execution_bytes, execution_memory_type)?; + + sandbox_safe_system_state.update_on_low_wasm_memory_hook_status( + Some(reserved_bytes), + self.wasm_memory_limit, + self.stable_memory_usage, + self.wasm_memory_usage, + ); Ok(()) } } } + fn add_execution_memory( + &mut self, + execution_bytes: NumBytes, + execution_memory_type: ExecutionMemoryType, + ) -> Result<(), HypervisorError> { + match execution_memory_type { + ExecutionMemoryType::WasmMemory => { + add_memory(&mut self.wasm_memory_usage, execution_bytes) + } + ExecutionMemoryType::StableMemory => { + add_memory(&mut self.stable_memory_usage, execution_bytes) + } + } + } + /// Tries to allocate the requested amount of message memory. /// /// Returns `Err(HypervisorError::OutOfMemory)` and leaves `self` unchanged @@ -941,6 +992,20 @@ impl MemoryUsage { } } +fn add_memory( + memory_size: &mut NumBytes, + additional_memory: NumBytes, +) -> Result<(), HypervisorError> { + let (new_usage, overflow) = memory_size.get().overflowing_add(additional_memory.get()); + + if overflow { + return Err(HypervisorError::OutOfMemory); + } + + *memory_size = NumBytes::new(new_usage); + Ok(()) +} + /// Struct that implements the SystemApi trait. This trait enables a canister to /// have mediated access to its system state. pub struct SystemApiImpl { @@ -1011,15 +1076,31 @@ impl SystemApiImpl { canister_backtrace: FlagStatus, max_sum_exported_function_name_lengths: usize, stable_memory: Memory, + wasm_memory_size: NumWasmPages, out_of_instructions_handler: Rc, log: ReplicaLogger, ) -> Self { + let stable_memory_usage = stable_memory + .size + .get() + .checked_mul(WASM_PAGE_SIZE_IN_BYTES) + .map(|v| NumBytes::new(v as u64)) + .expect("Stable memory size is larger than maximal allowed."); + + let wasm_memory_usage = wasm_memory_size + .get() + .checked_mul(WASM_PAGE_SIZE_IN_BYTES) + .map(|v| NumBytes::new(v as u64)) + .expect("Wasm memory size is larger than maximal allowed."); + let memory_usage = MemoryUsage::new( log.clone(), sandbox_safe_system_state.canister_id, execution_parameters.canister_memory_limit, execution_parameters.wasm_memory_limit, canister_current_memory_usage, + stable_memory_usage, + wasm_memory_usage, canister_current_message_memory_usage, subnet_available_memory, execution_parameters.memory_allocation, @@ -1919,9 +2000,9 @@ impl SystemApi for SystemApiImpl { ResponseStatus::NotRepliedYet => { if size as u64 > max_reply_size.get() { let string = format!( - "ic0.msg_reject: application payload size ({}) cannot be larger than {}.", - size, max_reply_size - ); + "ic0.msg_reject: application payload size ({}) cannot be larger than {}.", + size, max_reply_size + ); return Err(UserContractViolation { error: string, suggestion: "Try truncating the error messages that are too long." @@ -2677,6 +2758,7 @@ impl SystemApi for SystemApiImpl { &self.api_type, &mut self.sandbox_safe_system_state, &self.execution_parameters.subnet_memory_saturation, + ExecutionMemoryType::WasmMemory, ) { Ok(()) => Ok(()), Err(err @ HypervisorError::InsufficientCyclesInMemoryGrow { .. }) => { @@ -2729,6 +2811,7 @@ impl SystemApi for SystemApiImpl { &self.api_type, &mut self.sandbox_safe_system_state, &self.execution_parameters.subnet_memory_saturation, + ExecutionMemoryType::StableMemory, ) { Ok(()) => Ok(StableGrowOutcome::Success), Err(err @ HypervisorError::InsufficientCyclesInMemoryGrow { .. }) => { @@ -2947,13 +3030,13 @@ impl SystemApi for SystemApiImpl { if overflow || upper_bound > data_certificate.len() { return Err(ToolchainContractViolation { error: format!( - "ic0_data_certificate_copy failed because offset + size is out \ + "ic0_data_certificate_copy failed because offset + size is out \ of bounds. Found offset = {} and size = {} while offset + size \ must be <= {}", - offset, - size, - data_certificate.len() - ), + offset, + size, + data_certificate.len() + ), }); } @@ -3218,9 +3301,9 @@ impl SystemApi for SystemApiImpl { } | ApiType::NonReplicatedQuery { query_kind: - NonReplicatedQueryKind::Stateful { - outgoing_request, .. - }, + NonReplicatedQueryKind::Stateful { + outgoing_request, .. + }, .. } | ApiType::SystemTask { @@ -3232,13 +3315,13 @@ impl SystemApi for SystemApiImpl { | ApiType::RejectCallback { outgoing_request, .. } => match outgoing_request { - None => Err(HypervisorError::ToolchainContractViolation{ + None => Err(HypervisorError::ToolchainContractViolation { error: "ic0.call_with_best_effort_response called when no call is under construction." - .to_string(), + .to_string(), }), Some(request) => { if request.is_timeout_set() { - Err(HypervisorError::ToolchainContractViolation{ + Err(HypervisorError::ToolchainContractViolation { error: "ic0_call_with_best_effort_response failed because a timeout is already set.".to_string(), }) } else { diff --git a/rs/system_api/src/sandbox_safe_system_state.rs b/rs/system_api/src/sandbox_safe_system_state.rs index 37db36e6d8b..3e2c51dbc39 100644 --- a/rs/system_api/src/sandbox_safe_system_state.rs +++ b/rs/system_api/src/sandbox_safe_system_state.rs @@ -17,7 +17,10 @@ use ic_management_canister_types::{ use ic_nns_constants::CYCLES_MINTING_CANISTER_ID; use ic_registry_subnet_type::SubnetType; use ic_replicated_state::{ - canister_state::{system_state::CyclesUseCase, DEFAULT_QUEUE_CAPACITY}, + canister_state::{ + system_state::{CyclesUseCase, OnLowWasmMemoryHookStatus}, + DEFAULT_QUEUE_CAPACITY, + }, CallOrigin, CanisterStatus, NetworkTopology, SystemState, }; use ic_types::{ @@ -73,6 +76,7 @@ pub struct SystemStateChanges { requests: Vec, pub(super) new_global_timer: Option, canister_log: CanisterLog, + pub on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, } impl Default for SystemStateChanges { @@ -88,6 +92,7 @@ impl Default for SystemStateChanges { requests: vec![], new_global_timer: None, canister_log: Default::default(), + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus::ConditionNotSatisfied, } } } @@ -301,6 +306,8 @@ impl SystemStateChanges { self.validate_cycle_change(system_state.canister_id == CYCLES_MINTING_CANISTER_ID)?; self.apply_balance_changes(system_state); + system_state.set_on_low_wasm_memory_hook_status(self.on_low_wasm_memory_hook_status); + // Verify we don't accept more cycles than are available from call // context and update the call context balance. if let Some((context_id, call_context_balance_taken)) = self.call_context_balance_taken { @@ -610,6 +617,7 @@ impl SandboxSafeSystemState { request_metadata: RequestMetadata, caller: Option, next_canister_log_record_idx: u64, + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, ) -> Self { Self { canister_id, @@ -626,6 +634,7 @@ impl SandboxSafeSystemState { canister_log: CanisterLog::new_with_next_index(next_canister_log_record_idx), call_context_balance_taken: call_context_id .map(|call_context_id| (call_context_id, Cycles::zero())), + on_low_wasm_memory_hook_status, ..SystemStateChanges::default() }, initial_cycles_balance, @@ -727,6 +736,7 @@ impl SandboxSafeSystemState { request_metadata, caller, system_state.canister_log.next_idx(), + system_state.get_on_low_wasm_memory_hook_status(), ) } @@ -1210,6 +1220,59 @@ impl SandboxSafeSystemState { } } + /// Updates status `OnLowWasmMemoryHook` if the following condition is satisfied: + /// + /// 1. In the case of `memory_allocation` + /// `wasm_memory_threshold >= min(memory_allocation - used_stable_memory, wasm_memory_limit) - used_wasm_memory` + /// 2. Without memory allocation + /// `wasm_memory_threshold >= wasm_memory_limit - used_wasm_memory` + /// + /// Note: if `wasm_memory_limit` is not set, its default value is 4 GiB. + pub fn update_on_low_wasm_memory_hook_status( + &mut self, + memory_allocation: Option, + wasm_memory_limit: Option, + used_stable_memory: NumBytes, + used_wasm_memory: NumBytes, + ) { + // If wasm memory limit is not set, the default is 4 GiB. Wasm memory + // limit is ignored for query methods, response callback handlers, + // global timers, heartbeats, and canister pre_upgrade. + let wasm_memory_limit = + wasm_memory_limit.unwrap_or_else(|| NumBytes::new(4 * 1024 * 1024 * 1024)); + + // If the canister has memory allocation, then it maximum allowed Wasm memory + // can be calculated as min(memory_allocation - used_stable_memory, wasm_memory_limit). + let wasm_capacity = memory_allocation.map_or_else( + || wasm_memory_limit, + |memory_allocation| { + debug_assert!( + used_stable_memory <= memory_allocation, + "Used stable memory: {:?} is larger than memory allocation: {:?}.", + used_stable_memory, + memory_allocation + ); + std::cmp::min(memory_allocation - used_stable_memory, wasm_memory_limit) + }, + ); + + let on_low_wasm_memory_hook_status = + &mut self.system_state_changes.on_low_wasm_memory_hook_status; + + // Conceptually we can think that the remaining Wasm memory is + // equal to `wasm_capacity - used_wasm_memory` and that should + // be compared with `wasm_memory_threshold` when checking for + // the condition for the hook. However, since `wasm_memory_limit` + // is ignored in some executions as stated above it is possible + // that `used_wasm_memory` is greater than `wasm_capacity` to + // avoid overflowing subtraction we adopted inequality. + if wasm_capacity >= used_wasm_memory + self.wasm_memory_threshold { + *on_low_wasm_memory_hook_status = OnLowWasmMemoryHookStatus::ConditionNotSatisfied; + } else if *on_low_wasm_memory_hook_status != OnLowWasmMemoryHookStatus::Executed { + *on_low_wasm_memory_hook_status = OnLowWasmMemoryHookStatus::Ready; + } + } + // Returns `true` if storage cycles need to be reserved for the given // API type when growing memory. fn should_reserve_storage_cycles(&self, api_type: &ApiType) -> bool { @@ -1281,7 +1344,10 @@ mod tests { use ic_cycles_account_manager::CyclesAccountManager; use ic_limits::SMALL_APP_SUBNET_MAX_SIZE; use ic_registry_subnet_type::SubnetType; - use ic_replicated_state::{canister_state::system_state::CyclesUseCase, SystemState}; + use ic_replicated_state::{ + canister_state::system_state::{CyclesUseCase, OnLowWasmMemoryHookStatus}, + SystemState, + }; use ic_test_utilities_types::ids::{canister_test_id, subnet_test_id, user_test_id}; use ic_types::{ messages::{RequestMetadata, NO_DEADLINE}, @@ -1391,6 +1457,7 @@ mod tests { RequestMetadata::new(0, Time::from_nanos_since_unix_epoch(0)), None, 0, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, ); sandbox_state.msg_deadline() } @@ -1405,4 +1472,135 @@ mod tests { // Otherwise the correct `deadline` is returned. assert_eq!(helper_msg_deadline(Some(deadline)), deadline); } + + fn helper_create_state_for_hook_status( + start_status: OnLowWasmMemoryHookStatus, + wasm_memory_threshold: u64, + ) -> SandboxSafeSystemState { + SandboxSafeSystemState::new_internal( + canister_test_id(0), + CanisterStatusView::Running, + NumSeconds::from(3600), + MemoryAllocation::BestEffort, + NumBytes::new(wasm_memory_threshold), + ComputeAllocation::default(), + Cycles::new(1_000_000), + Cycles::zero(), + None, + None, + None, + None, + CyclesAccountManager::new( + NumInstructions::from(1_000_000_000), + SubnetType::Application, + subnet_test_id(0), + CyclesAccountManagerConfig::application_subnet(), + ), + Some(0), + BTreeMap::new(), + 0, + BTreeSet::new(), + SMALL_APP_SUBNET_MAX_SIZE, + SchedulerConfig::application_subnet().dirty_page_overhead, + CanisterTimer::Inactive, + 0, + BTreeSet::new(), + RequestMetadata::new(0, Time::from_nanos_since_unix_epoch(0)), + None, + 0, + start_status, + ) + } + + const GIB: u64 = 1 << 30; + + fn helper_is_condition_satisfied_for_on_low_wasm_memory_hook( + wasm_memory_threshold: u64, + memory_allocation: Option, + wasm_memory_limit: Option, + used_stable_memory: u64, + used_wasm_memory: u64, + ) -> bool { + let wasm_memory_limit = wasm_memory_limit.unwrap_or(4 * GIB); + + let wasm_capacity = memory_allocation.map_or(wasm_memory_limit, |memory_allocation| { + std::cmp::min(memory_allocation - used_stable_memory, wasm_memory_limit) + }); + + wasm_capacity < used_wasm_memory + wasm_memory_threshold + } + + fn helper_test_on_low_wasm_memory_hook( + start_status: OnLowWasmMemoryHookStatus, + status_if_condition_satisfied: OnLowWasmMemoryHookStatus, + ) { + for wasm_memory_threshold in [0, GIB, 2 * GIB, 3 * GIB, 4 * GIB] { + for memory_allocation in [None, Some(GIB), Some(2 * GIB), Some(3 * GIB), Some(4 * GIB)] + { + for wasm_memory_limit in + [None, Some(GIB), Some(2 * GIB), Some(3 * GIB), Some(4 * GIB)] + { + for used_stable_memory in [0, GIB] { + for used_wasm_memory in [0, GIB, 2 * GIB, 3 * GIB, 4 * GIB] { + let mut state = helper_create_state_for_hook_status( + start_status, + wasm_memory_threshold, + ); + + assert_eq!( + state.system_state_changes.on_low_wasm_memory_hook_status, + start_status + ); + + state.update_on_low_wasm_memory_hook_status( + memory_allocation.map(|m| m.into()), + wasm_memory_limit.map(|m| m.into()), + used_stable_memory.into(), + used_wasm_memory.into(), + ); + + assert_eq!( + state.system_state_changes.on_low_wasm_memory_hook_status, + if helper_is_condition_satisfied_for_on_low_wasm_memory_hook( + wasm_memory_threshold, + memory_allocation, + wasm_memory_limit, + used_stable_memory, + used_wasm_memory + ) { + status_if_condition_satisfied + } else { + OnLowWasmMemoryHookStatus::ConditionNotSatisfied + } + ); + } + } + } + } + } + } + + #[test] + fn test_on_low_wasm_memory_hook_start_status_condition_not_satisfied() { + helper_test_on_low_wasm_memory_hook( + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::Ready, + ); + } + + #[test] + fn test_on_low_wasm_memory_hook_start_status_ready() { + helper_test_on_low_wasm_memory_hook( + OnLowWasmMemoryHookStatus::Ready, + OnLowWasmMemoryHookStatus::Ready, + ); + } + + #[test] + fn test_on_low_wasm_memory_hook_start_status_executed() { + helper_test_on_low_wasm_memory_hook( + OnLowWasmMemoryHookStatus::Executed, + OnLowWasmMemoryHookStatus::Executed, + ); + } } diff --git a/rs/system_api/tests/common/mod.rs b/rs/system_api/tests/common/mod.rs index cbb9bf8d3ad..24d7bcc3eac 100644 --- a/rs/system_api/tests/common/mod.rs +++ b/rs/system_api/tests/common/mod.rs @@ -11,6 +11,7 @@ use ic_management_canister_types::IC_00; use ic_nns_constants::CYCLES_MINTING_CANISTER_ID; use ic_registry_routing_table::{CanisterIdRange, RoutingTable}; use ic_registry_subnet_type::SubnetType; +use ic_replicated_state::NumWasmPages; use ic_replicated_state::{CallOrigin, Memory, NetworkTopology, SubnetTopology, SystemState}; use ic_system_api::{ sandbox_safe_system_state::SandboxSafeSystemState, ApiType, DefaultOutOfInstructionsHandler, @@ -208,6 +209,7 @@ pub fn get_system_api( EmbeddersConfig::default().feature_flags.canister_backtrace, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), + NumWasmPages::from(0), Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ) diff --git a/rs/system_api/tests/system_api.rs b/rs/system_api/tests/system_api.rs index 1b54099ae24..65b47f58a9f 100644 --- a/rs/system_api/tests/system_api.rs +++ b/rs/system_api/tests/system_api.rs @@ -1,4 +1,4 @@ -use ic_base_types::{NumSeconds, PrincipalIdBlobParseError}; +use ic_base_types::{NumBytes, NumSeconds, PrincipalIdBlobParseError}; use ic_config::{ embedders::Config as EmbeddersConfig, flag_status::FlagStatus, subnet_config::SchedulerConfig, }; @@ -6,13 +6,16 @@ use ic_cycles_account_manager::CyclesAccountManager; use ic_error_types::RejectCode; use ic_interfaces::execution_environment::{ CanisterOutOfCyclesError, ExecutionMode, HypervisorError, HypervisorResult, - PerformanceCounterType, SubnetAvailableMemory, SystemApi, SystemApiCallId, TrapCode, + PerformanceCounterType, StableMemoryApi, SubnetAvailableMemory, SystemApi, SystemApiCallId, + TrapCode, }; use ic_limits::SMALL_APP_SUBNET_MAX_SIZE; use ic_logger::replica_logger::no_op_logger; use ic_registry_subnet_type::SubnetType; +use ic_replicated_state::NumWasmPages; use ic_replicated_state::{ - testing::CanisterQueuesTesting, CallOrigin, Memory, NetworkTopology, SystemState, + canister_state::system_state::OnLowWasmMemoryHookStatus, testing::CanisterQueuesTesting, + CallOrigin, Memory, NetworkTopology, SystemState, }; use ic_system_api::{ sandbox_safe_system_state::SandboxSafeSystemState, ApiType, DefaultOutOfInstructionsHandler, @@ -1349,6 +1352,7 @@ fn growing_wasm_memory_updates_subnet_available_memory() { EmbeddersConfig::default().feature_flags.canister_backtrace, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), + NumWasmPages::from(0), Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ); @@ -1370,6 +1374,247 @@ fn growing_wasm_memory_updates_subnet_available_memory() { ); } +const GIB: i64 = 1 << 30; + +fn helper_test_on_low_wasm_memory( + wasm_memory_threshold: NumBytes, + wasm_memory_limit: Option, + memory_allocation: Option, + grow_memory_size: i64, + grow_wasm_memory: bool, + start_status: OnLowWasmMemoryHookStatus, + expected_status: OnLowWasmMemoryHookStatus, +) { + let wasm_page_size = 64 << 10; + let subnet_available_memory_bytes = 20 * GIB; + let subnet_available_memory = SubnetAvailableMemory::new(subnet_available_memory_bytes, 0, 0); + + let mut state_builder = SystemStateBuilder::default() + .wasm_memory_threshold(wasm_memory_threshold) + .wasm_memory_limit(wasm_memory_limit) + .on_low_wasm_memory_hook_status(start_status) + .initial_cycles(Cycles::from(10_000_000_000_000_000u128)); + + if let Some(memory_allocation) = memory_allocation { + state_builder = state_builder.memory_allocation(memory_allocation); + }; + + let mut system_state = state_builder.build(); + + let api_type = ApiTypeBuilder::build_update_api(); + let mut execution_parameters = execution_parameters(api_type.execution_mode()); + execution_parameters.memory_allocation = system_state.memory_allocation; + execution_parameters.wasm_memory_limit = system_state.wasm_memory_limit; + + let sandbox_safe_system_state = SandboxSafeSystemState::new( + &system_state, + CyclesAccountManagerBuilder::new().build(), + &NetworkTopology::default(), + SchedulerConfig::application_subnet().dirty_page_overhead, + execution_parameters.compute_allocation, + RequestMetadata::new(0, UNIX_EPOCH), + api_type.caller(), + api_type.call_context_id(), + ); + + let mut api = SystemApiImpl::new( + api_type, + sandbox_safe_system_state, + CANISTER_CURRENT_MEMORY_USAGE, + CANISTER_CURRENT_MESSAGE_MEMORY_USAGE, + execution_parameters, + subnet_available_memory, + EmbeddersConfig::default() + .feature_flags + .wasm_native_stable_memory, + EmbeddersConfig::default().feature_flags.canister_backtrace, + EmbeddersConfig::default().max_sum_exported_function_name_lengths, + Memory::new_for_testing(), + NumWasmPages::from(0), + Rc::new(DefaultOutOfInstructionsHandler::default()), + no_op_logger(), + ); + + let additional_wasm_pages = (grow_memory_size as u64).div_ceil(wasm_page_size as u64); + + if grow_wasm_memory { + api.try_grow_wasm_memory(0, additional_wasm_pages).unwrap(); + } else { + api.try_grow_stable_memory(0, additional_wasm_pages, StableMemoryApi::Stable64) + .unwrap(); + } + + let system_state_changes = api.into_system_state_changes(); + system_state_changes + .apply_changes( + UNIX_EPOCH, + &mut system_state, + &default_network_topology(), + subnet_test_id(1), + &no_op_logger(), + ) + .unwrap(); + + assert_eq!( + system_state.get_on_low_wasm_memory_hook_status(), + expected_status + ); +} + +#[test] +fn test_on_low_wasm_memory_grow_wasm_memory_all_status_changes() { + let wasm_memory_threshold = NumBytes::new(GIB as u64); + let wasm_memory_limit = Some(NumBytes::new(3 * GIB as u64)); + let memory_allocation = None; + // `max_allowed_wasm_memory` = `wasm_memory_limit` - `wasm_memory_threshold` + let max_allowed_wasm_memory = 2 * GIB; + let grow_wasm_memory = true; + + // Hook condition is not satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + ); + + // Hook condition is satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory + 1, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::Ready, + ); + + // Hook condition is not satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::Ready, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + ); + + // Hook condition is satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory + 1, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::Ready, + OnLowWasmMemoryHookStatus::Ready, + ); + + // Hook condition is not satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::Executed, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + ); + + // Hook condition is satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory + 1, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::Executed, + OnLowWasmMemoryHookStatus::Executed, + ); +} + +#[test] +fn test_on_low_wasm_memory_grow_stable_memory() { + // When memory_allocation is provided, hook condition can be triggered if: + // memory_allocation - used_stable_memory - used_wasm_memory < wasm_memory_threshold + // Hence growing stable memory can trigger hook condition. + let wasm_memory_threshold = NumBytes::new(GIB as u64); + let wasm_memory_limit = None; + let memory_allocation = Some(NumBytes::new(3 * GIB as u64)); + let max_allowed_memory_size = 2 * GIB; + let grow_wasm_memory = false; + + // Hook condition is not satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_memory_size, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + ); + + // Hook condition is satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_memory_size + 1, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::Ready, + ); + + // Without `memory_allocation`, hook condition is not satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + None, + max_allowed_memory_size + 1, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + ); +} + +#[test] +fn test_on_low_wasm_memory_without_memory_limitn() { + // When memory limit is not set, the default Wasm memory limit is 4 GIB. + let wasm_memory_threshold = NumBytes::new(GIB as u64); + // `max_allowed_wasm_memory` = `wasm_memory_limit` - `wasm_memory_threshold` + let max_allowed_wasm_memory = 3 * GIB; + let wasm_memory_limit = None; + let memory_allocation = None; + let grow_wasm_memory = true; + + // Hook condition is not satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + ); + + // Hook condition is satisfied. + helper_test_on_low_wasm_memory( + wasm_memory_threshold, + wasm_memory_limit, + memory_allocation, + max_allowed_wasm_memory + 1, + grow_wasm_memory, + OnLowWasmMemoryHookStatus::ConditionNotSatisfied, + OnLowWasmMemoryHookStatus::Ready, + ); +} + #[test] fn push_output_request_respects_memory_limits() { let subnet_available_memory_bytes = 1 << 30; @@ -1422,6 +1667,7 @@ fn push_output_request_respects_memory_limits() { EmbeddersConfig::default().feature_flags.canister_backtrace, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), + NumWasmPages::from(0), Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ); @@ -1534,6 +1780,7 @@ fn push_output_request_oversized_request_memory_limits() { EmbeddersConfig::default().feature_flags.canister_backtrace, EmbeddersConfig::default().max_sum_exported_function_name_lengths, Memory::new_for_testing(), + NumWasmPages::from(0), Rc::new(DefaultOutOfInstructionsHandler::default()), no_op_logger(), ); diff --git a/rs/test_utilities/embedders/src/lib.rs b/rs/test_utilities/embedders/src/lib.rs index ad634c1b9e6..efbd4c704df 100644 --- a/rs/test_utilities/embedders/src/lib.rs +++ b/rs/test_utilities/embedders/src/lib.rs @@ -9,6 +9,7 @@ use ic_interfaces::execution_environment::{ }; use ic_logger::replica_logger::no_op_logger; use ic_registry_subnet_type::SubnetType; +use ic_replicated_state::NumWasmPages; use ic_replicated_state::{Global, Memory, NetworkTopology, PageMap}; use ic_system_api::{ sandbox_safe_system_state::SandboxSafeSystemState, ExecutionParameters, InstructionLimits, @@ -167,6 +168,7 @@ impl WasmtimeInstanceBuilder { embedder.config().feature_flags.canister_backtrace, embedder.config().max_sum_exported_function_name_lengths, Memory::new_for_testing(), + NumWasmPages::from(0), Rc::new(ic_system_api::DefaultOutOfInstructionsHandler::new( self.num_instructions, )), diff --git a/rs/test_utilities/state/src/lib.rs b/rs/test_utilities/state/src/lib.rs index 54412cedfba..36dc0c766ff 100644 --- a/rs/test_utilities/state/src/lib.rs +++ b/rs/test_utilities/state/src/lib.rs @@ -12,13 +12,15 @@ use ic_replicated_state::{ execution_state::{ CustomSection, CustomSectionType, NextScheduledMethod, WasmBinary, WasmMetadata, }, - system_state::CyclesUseCase, + system_state::{CyclesUseCase, OnLowWasmMemoryHookStatus}, testing::new_canister_output_queues_for_test, }, - metadata_state::subnet_call_context_manager::{ - BitcoinGetSuccessorsContext, BitcoinSendTransactionInternalContext, SubnetCallContext, + metadata_state::{ + subnet_call_context_manager::{ + BitcoinGetSuccessorsContext, BitcoinSendTransactionInternalContext, SubnetCallContext, + }, + Stream, SubnetMetrics, }, - metadata_state::{Stream, SubnetMetrics}, page_map::PageMap, testing::{CanisterQueuesTesting, ReplicatedStateTesting, SystemStateTesting}, CallContext, CallOrigin, CanisterState, CanisterStatus, ExecutionState, ExportedFunctions, @@ -482,6 +484,20 @@ impl SystemStateBuilder { self } + pub fn wasm_memory_limit(mut self, wasm_memory_limit: Option) -> Self { + self.system_state.wasm_memory_limit = wasm_memory_limit; + self + } + + pub fn on_low_wasm_memory_hook_status( + mut self, + on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus, + ) -> Self { + self.system_state + .set_on_low_wasm_memory_hook_status(on_low_wasm_memory_hook_status); + self + } + pub fn freeze_threshold(mut self, threshold: NumSeconds) -> Self { self.system_state.freeze_threshold = threshold; self