Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: [EXC-1669] Propagate hook execution status to SystemState #667

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
7442cfa
.
dragoljub-duric Jun 25, 2024
26790e0
.
dragoljub-duric Jun 25, 2024
ba9b326
.
dragoljub-duric Jun 25, 2024
6c7a6dc
fix types test
dragoljub-duric Jul 1, 2024
2c74c74
.
dragoljub-duric Jul 2, 2024
0846be8
.
dragoljub-duric Jul 2, 2024
3edf927
.
dragoljub-duric Jul 19, 2024
72535d6
fix test
dragoljub-duric Jul 22, 2024
7268824
.
dragoljub-duric Jul 23, 2024
c15f183
.
dragoljub-duric Jul 23, 2024
0388a31
fix test
dragoljub-duric Jul 23, 2024
15e4785
.
dragoljub-duric Jul 24, 2024
6e271f5
Revert accidentally deleted function.
dragoljub-duric Jul 24, 2024
c7b3039
Merge branch 'master' into EXC-1665-add-wasm-memory-threshold-field-t…
dragoljub-duric Jul 25, 2024
6dc8de8
.
dragoljub-duric Jul 29, 2024
ffece97
Merge branch 'master' into EXC-1670-implement-checking-for-hook-condi…
dragoljub-duric Jul 29, 2024
1d4d250
.
dragoljub-duric Jul 29, 2024
68aa922
Add on_low_wasm_memory_hook_status field to CanisterStateBits.
dragoljub-duric Jul 31, 2024
ebe1ccb
.
dragoljub-duric Jul 31, 2024
ae7e6ff
fix errors
dragoljub-duric Aug 6, 2024
bdc3be7
fix errors
dragoljub-duric Aug 6, 2024
7166646
fix some more errors
dragoljub-duric Aug 6, 2024
7963c80
.
dragoljub-duric Aug 7, 2024
de78992
.
dragoljub-duric Aug 7, 2024
e0b1a2f
.
dragoljub-duric Aug 7, 2024
f3a552b
.
dragoljub-duric Aug 7, 2024
dc7f4ff
.
dragoljub-duric Aug 8, 2024
0d04856
.
dragoljub-duric Aug 8, 2024
cf11649
.
dragoljub-duric Aug 9, 2024
88a44fc
fix wasm memory size missmatch
dragoljub-duric Sep 6, 2024
52197ef
Merge branch 'master' into EXC-1669-implement-checking-for-hook-condi…
dragoljub-duric Sep 9, 2024
d539caf
.
dragoljub-duric Sep 9, 2024
97e9238
.
dragoljub-duric Sep 9, 2024
e79e24c
fix execution test
dragoljub-duric Sep 10, 2024
2c084af
make field private
dragoljub-duric Sep 11, 2024
fa653f5
remove unused method
dragoljub-duric Sep 11, 2024
f56bc96
Merge branch 'master' into EXC-1669-implement-checking-for-hook-condi…
dragoljub-duric Sep 11, 2024
0492315
Add unit test for OnLowWasmMemoryHookStatus::update().
dragoljub-duric Sep 11, 2024
f5fe0fe
.
dragoljub-duric Sep 12, 2024
cd149ad
Add system_api tests
dragoljub-duric Sep 12, 2024
1ad2cb5
Clippy.
dragoljub-duric Sep 12, 2024
d3b0b1e
remove unused on_low_wasm_memory_initialization
dragoljub-duric Sep 20, 2024
1e04e21
remove unused initialization
dragoljub-duric Sep 20, 2024
6f66fe2
write derrive in canonical order
dragoljub-duric Sep 20, 2024
9e1fb92
refactor
dragoljub-duric Sep 20, 2024
b726151
refactor
dragoljub-duric Sep 20, 2024
2b1db33
Merge branch 'master' into EXC-1669-implement-checking-for-hook-condi…
dragoljub-duric Sep 20, 2024
d122dde
Fix clippy error.
dragoljub-duric Sep 20, 2024
8e337af
Fix clippy error.
dragoljub-duric Sep 20, 2024
0d51f63
Update rs/replicated_state/src/canister_state/system_state.rs
dragoljub-duric Sep 24, 2024
6a5b213
adress comments
dragoljub-duric Sep 24, 2024
f19ce55
Fix clippy.
dragoljub-duric Sep 24, 2024
1c1138d
Adress comments/
dragoljub-duric Sep 24, 2024
b2fa264
Refactor remainings.
dragoljub-duric Sep 24, 2024
6973cc4
Add comment.
dragoljub-duric Sep 24, 2024
33592b0
Refactor.
dragoljub-duric Sep 24, 2024
c97b287
Fix test.
dragoljub-duric Sep 24, 2024
29c75c1
Move update() logic to system_api.
dragoljub-duric Sep 24, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion rs/canister_sandbox/src/sandbox_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -228,6 +231,7 @@ mod tests {
RequestMetadata::new(0, Time::from_nanos_since_unix_epoch(0)),
caller,
0,
OnLowWasmMemoryHookStatus::ConditionNotSatisfied,
dragoljub-duric marked this conversation as resolved.
Show resolved Hide resolved
)
}

Expand Down
1 change: 1 addition & 0 deletions rs/embedders/src/wasm_executor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -92,6 +93,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(),
);
Expand Down
1 change: 1 addition & 0 deletions rs/embedders/tests/wasmtime_random_memory_writes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down
3 changes: 1 addition & 2 deletions rs/execution_environment/src/canister_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -340,6 +340,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";
Expand Down Expand Up @@ -429,4 +436,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;
}
38 changes: 38 additions & 0 deletions rs/protobuf/src/gen/state/state.canister_state_bits.v1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,8 @@ pub struct CanisterStateBits {
pub long_execution_mode: i32,
#[prost(uint64, optional, tag = "50")]
pub wasm_memory_threshold: ::core::option::Option<u64>,
#[prost(enumeration = "OnLowWasmMemoryHookStatus", optional, tag = "53")]
pub on_low_wasm_memory_hook_status: ::core::option::Option<i32>,
#[prost(oneof = "canister_state_bits::CanisterStatus", tags = "11, 12, 13")]
pub canister_status: ::core::option::Option<canister_state_bits::CanisterStatus>,
}
Expand Down Expand Up @@ -870,3 +872,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<Self> {
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,
}
}
}
56 changes: 56 additions & 0 deletions rs/replicated_state/src/canister_state/system_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<pb::OnLowWasmMemoryHookStatus> for OnLowWasmMemoryHookStatus {
type Error = ProxyDecodeError;

fn try_from(value: pb::OnLowWasmMemoryHookStatus) -> Result<Self, Self::Error> {
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.
Expand Down Expand Up @@ -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(),
}
}

Expand Down Expand Up @@ -770,6 +816,7 @@ impl SystemState {
next_snapshot_id: u64,
snapshots_memory_usage: NumBytes,
metrics: &dyn CheckpointLoadingMetrics,
on_low_wasm_memory_hook_status: OnLowWasmMemoryHookStatus,
dragoljub-duric marked this conversation as resolved.
Show resolved Hide resolved
) -> Self {
let system_state = Self {
controllers,
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
29 changes: 25 additions & 4 deletions rs/state_layout/src/state_layout.rs
Original file line number Diff line number Diff line change
@@ -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};
Expand All @@ -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,
Expand All @@ -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;

Expand Down Expand Up @@ -175,6 +178,7 @@ pub struct CanisterStateBits {
pub wasm_memory_limit: Option<NumBytes>,
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
Expand Down Expand Up @@ -2281,6 +2285,12 @@ impl From<CanisterStateBits> 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(),
),
}
}
}
Expand Down Expand Up @@ -2351,6 +2361,12 @@ impl TryFrom<pb_canister_state_bits::CanisterStateBits> 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(),
Expand Down Expand Up @@ -2433,6 +2449,11 @@ impl TryFrom<pb_canister_state_bits::CanisterStateBits> 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(),
})
}
}
Expand Down
7 changes: 5 additions & 2 deletions rs/state_layout/src/state_layout/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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(),
}
}

Expand Down
10 changes: 6 additions & 4 deletions rs/state_manager/src/checkpoint.rs
Original file line number Diff line number Diff line change
@@ -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;
Expand Down Expand Up @@ -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;

Expand Down Expand Up @@ -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 {
Expand Down
3 changes: 3 additions & 0 deletions rs/state_manager/src/tip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
)?;
Expand Down
Loading
Loading