Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 1 addition & 2 deletions rs/bitcoin/ckbtc/minter/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1501,8 +1501,7 @@ impl CkBtcSetup {
}

pub fn print_minter_canister_logs(&self) {
let log = self.env.canister_log(self.minter_id);
let mut records = log.records().iter().collect::<Vec<_>>();
let mut records = self.env.canister_log_records(self.minter_id);
records.sort_by_key(|a| a.idx);
for entry in records {
println!(
Expand Down
19 changes: 2 additions & 17 deletions rs/config/src/execution_environment.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,23 +15,8 @@ const REPLICATED_INTER_CANISTER_LOG_FETCH_FEATURE: FlagStatus = FlagStatus::Disa

const FLEXIBLE_HTTP_REQUESTS_FEATURE: FlagStatus = FlagStatus::Disabled;

// TODO(DSM-105): remove after the feature is enabled by default.
pub const LOG_MEMORY_STORE_FEATURE_ENABLED: bool = true;
pub const LOG_MEMORY_STORE_FEATURE: FlagStatus = if LOG_MEMORY_STORE_FEATURE_ENABLED {
FlagStatus::Enabled
} else {
FlagStatus::Disabled
};
pub const TEST_DEFAULT_LOG_MEMORY_LIMIT: u64 = if LOG_MEMORY_STORE_FEATURE_ENABLED {
4 * KIB
} else {
0
};
pub const TEST_DEFAULT_LOG_MEMORY_USAGE: u64 = if LOG_MEMORY_STORE_FEATURE_ENABLED {
4 * KIB + 4 * KIB + TEST_DEFAULT_LOG_MEMORY_LIMIT // header, index table, data region
} else {
0
};
pub const TEST_DEFAULT_LOG_MEMORY_LIMIT: u64 = 4 * KIB;
pub const TEST_DEFAULT_LOG_MEMORY_USAGE: u64 = 4 * KIB + 4 * KIB + TEST_DEFAULT_LOG_MEMORY_LIMIT; // header, index table, data region

/// This specifies the threshold in bytes at which the subnet memory usage is
/// considered to be high. If this value is greater or equal to the subnet
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -393,16 +393,10 @@ impl SystemStateModifications {
) -> HypervisorResult<RequestMetadataStats> {
// Append delta logs.
if !self.canister_log.is_empty() {
// TODO(DSM-11): Move this into append_delta_log() once there is only one of it.
metrics.observe_delta_log_size(self.canister_log.bytes_used());
}
system_state
.log_memory_store
.append_delta_log(&mut self.canister_log.clone());
// Keep the legacy `canister_log` store up to date so that checkpoints
// remain readable by replicas that predate the log memory store.
system_state
.canister_log
.append_delta_log(&mut self.canister_log);

// Verify total cycle change is not positive and update cycles balance.
Expand Down
4 changes: 1 addition & 3 deletions rs/ethereum/cketh/test_utils/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -698,9 +698,7 @@ impl CkEthSetup {
}

pub fn minter_canister_logs(&self) -> Vec<CanisterLog> {
let log = self.env.canister_log(self.minter_id);

let mut records = log.records().iter().collect::<Vec<_>>();
let mut records = self.env.canister_log_records(self.minter_id);
records.sort_by_key(|a| a.idx);
records
.into_iter()
Expand Down
21 changes: 7 additions & 14 deletions rs/execution_environment/src/canister_manager/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@ use ic_config::embedders::DEFAULT_CREATE_EXECUTION_STATE_BASE_COST;
use ic_config::{
execution_environment::{
CANISTER_GUARANTEED_CALLBACK_QUOTA, Config, DEFAULT_WASM_MEMORY_LIMIT,
LOG_MEMORY_STORE_FEATURE_ENABLED, MAX_ENVIRONMENT_VARIABLE_NAME_LENGTH,
MAX_ENVIRONMENT_VARIABLE_VALUE_LENGTH, MAX_ENVIRONMENT_VARIABLES,
MAX_NUMBER_OF_SNAPSHOTS_PER_CANISTER, SUBNET_CALLBACK_SOFT_LIMIT,
SUBNET_MEMORY_RESERVATION, TEST_DEFAULT_LOG_MEMORY_USAGE,
MAX_ENVIRONMENT_VARIABLE_NAME_LENGTH, MAX_ENVIRONMENT_VARIABLE_VALUE_LENGTH,
MAX_ENVIRONMENT_VARIABLES, MAX_NUMBER_OF_SNAPSHOTS_PER_CANISTER,
SUBNET_CALLBACK_SOFT_LIMIT, SUBNET_MEMORY_RESERVATION, TEST_DEFAULT_LOG_MEMORY_USAGE,
},
flag_status::FlagStatus,
subnet_config::{CANISTER_CREATION_FEE, SchedulerConfig},
Expand Down Expand Up @@ -536,11 +535,8 @@ fn install_canister_fails_if_memory_capacity_exceeded() {

// Try installing canister2, should fail due to insufficient memory capacity on the subnet.
let err = test.install_canister(canister2, wasm).unwrap_err();
let msg = if LOG_MEMORY_STORE_FEATURE_ENABLED {
"Canister requested 10.00 MiB of memory but only 9.99 MiB are available in the subnet."
} else {
"Canister requested 10.00 MiB of memory but only 10.00 MiB are available in the subnet."
};
let msg =
"Canister requested 10.00 MiB of memory but only 9.99 MiB are available in the subnet.";
err.assert_contains(ErrorCode::SubnetOversubscribed, msg);
assert_eq!(
test.canister_state(canister2).system_state.balance(),
Expand Down Expand Up @@ -2294,11 +2290,8 @@ fn upgrading_canister_fails_if_memory_capacity_exceeded() {

// Try upgrading the canister, should fail because there is not enough memory capacity
// on the subnet.
let msg = if LOG_MEMORY_STORE_FEATURE_ENABLED {
"Canister requested 10.00 MiB of memory but only 9.99 MiB are available in the subnet."
} else {
"Canister requested 10.00 MiB of memory but only 10.00 MiB are available in the subnet."
};
let msg =
"Canister requested 10.00 MiB of memory but only 9.99 MiB are available in the subnet.";
test.upgrade_canister(canister2, wasm)
.unwrap_err()
.assert_contains(ErrorCode::SubnetOversubscribed, msg);
Expand Down
10 changes: 5 additions & 5 deletions rs/execution_environment/src/execution/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ pub(crate) fn execute_install(
original,
round,
err,
helper.take_canister_log(),
helper.clone_log_memory_store(),
);
}

Expand All @@ -109,7 +109,7 @@ pub(crate) fn execute_install(
original,
round,
err,
helper.take_canister_log(),
helper.clone_log_memory_store(),
);
}
};
Expand All @@ -136,7 +136,7 @@ pub(crate) fn execute_install(
original,
round,
err,
helper.take_canister_log(),
helper.clone_log_memory_store(),
);
}
helper.clear_certified_data();
Expand Down Expand Up @@ -250,7 +250,7 @@ fn install_stage_2a_process_start_result(
original,
round,
err,
helper.take_canister_log(),
helper.clone_log_memory_store(),
);
}

Expand Down Expand Up @@ -362,7 +362,7 @@ fn install_stage_3_process_init_result(
original,
round,
err,
helper.take_canister_log(),
helper.clone_log_memory_store(),
);
}
helper.finish(clean_canister, original, round, round_limits)
Expand Down
46 changes: 21 additions & 25 deletions rs/execution_environment/src/execution/install_code.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,7 @@ use ic_replicated_state::canister_state::system_state::{
use ic_replicated_state::metadata_state::subnet_call_context_manager::InstallCodeCallId;
use ic_replicated_state::{CanisterState, ExecutionState, num_bytes_try_from};
use ic_sys::PAGE_SIZE;
use ic_types::{
CanisterLog, CanisterTimer, MemoryAllocation, NumInstructions, Time, messages::CanisterCall,
};
use ic_types::{CanisterTimer, MemoryAllocation, NumInstructions, Time, messages::CanisterCall};
use ic_types_cycles::{CompoundCycles, Cycles, CyclesUseCase, Instructions};
use ic_wasm_types::WasmHash;

Expand Down Expand Up @@ -249,20 +247,19 @@ impl InstallCodeHelper {
paused: PausedInstallCodeHelper,
original: &OriginalContext,
round: &RoundContext,
) -> Result<
Self,
(
CanisterManagerError,
NumInstructions,
(CanisterLog, LogMemoryStore),
),
> {
) -> Result<Self, (CanisterManagerError, NumInstructions, LogMemoryStore)> {
let mut helper = Self::new(clean_canister, original);
let paused_instructions_left = paused.instructions_left;
for state_change in paused.steps.into_iter() {
helper
.replay_step(state_change, original, round)
.map_err(|err| (err, paused_instructions_left, helper.take_canister_log()))?;
.map_err(|err| {
(
err,
paused_instructions_left,
helper.clone_log_memory_store(),
)
})?;
}
debug_assert_eq!(paused_instructions_left, helper.instructions_left());
Ok(helper)
Expand Down Expand Up @@ -349,7 +346,7 @@ impl InstallCodeHelper {
original,
round,
CanisterManagerError::Hypervisor(self.canister.canister_id(), err),
self.take_canister_log(),
self.clone_log_memory_store(),
);
}
}
Expand Down Expand Up @@ -393,7 +390,7 @@ impl InstallCodeHelper {
original,
round,
err,
self.take_canister_log(),
self.clone_log_memory_store(),
);
}
}
Expand Down Expand Up @@ -422,7 +419,7 @@ impl InstallCodeHelper {
original,
round,
err,
self.take_canister_log(),
self.clone_log_memory_store(),
);
}
}
Expand Down Expand Up @@ -466,7 +463,7 @@ impl InstallCodeHelper {
original,
round,
err,
self.take_canister_log(),
self.clone_log_memory_store(),
);
}
}
Expand Down Expand Up @@ -617,13 +614,12 @@ impl InstallCodeHelper {
}
}

/// Takes the canister log.
pub(crate) fn take_canister_log(&mut self) -> (CanisterLog, LogMemoryStore) {
// TODO(DSM-105): Remove duplication when the migration is fully done.
(
self.canister.system_state.canister_log.take(),
self.canister.system_state.log_memory_store.clone(),
)
/// Returns a clone of the canister's log memory store.
///
/// Cloning is cheap as the log memory store is backed by a persistent
/// `PageMap` whose clone creates an independent snapshot cheaply.
pub(crate) fn clone_log_memory_store(&self) -> LogMemoryStore {
self.canister.system_state.log_memory_store.clone()
}

/// Checks the result of Wasm execution and applies the state changes.
Expand Down Expand Up @@ -859,11 +855,11 @@ pub(crate) fn finish_err(
original: OriginalContext,
round: RoundContext,
err: CanisterManagerError,
new_canister_log: (CanisterLog, LogMemoryStore),
new_log_memory_store: LogMemoryStore,
) -> DtsInstallCodeResult {
let mut new_canister = clean_canister;

new_canister.set_log(new_canister_log);
new_canister.set_log(new_log_memory_store);
new_canister
.system_state
.apply_ingress_induction_cycles_debit(
Expand Down
16 changes: 8 additions & 8 deletions rs/execution_environment/src/execution/upgrade.rs
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ pub(crate) fn execute_upgrade(
original,
round,
err,
helper.take_canister_log(),
helper.clone_log_memory_store(),
);
}

Expand All @@ -128,7 +128,7 @@ pub(crate) fn execute_upgrade(
original,
round,
(canister_id, HypervisorError::WasmModuleNotFound).into(),
helper.take_canister_log(),
helper.clone_log_memory_store(),
);
}
};
Expand Down Expand Up @@ -237,7 +237,7 @@ fn upgrade_stage_1_process_pre_upgrade_result(
original,
round,
err,
helper.take_canister_log(),
helper.clone_log_memory_store(),
);
}

Expand Down Expand Up @@ -275,7 +275,7 @@ fn upgrade_stage_2_and_3a_create_execution_state_and_call_start(
original,
round,
err,
helper.take_canister_log(),
helper.clone_log_memory_store(),
);
}
};
Expand Down Expand Up @@ -308,7 +308,7 @@ fn upgrade_stage_2_and_3a_create_execution_state_and_call_start(
original,
round,
err,
helper.take_canister_log(),
helper.clone_log_memory_store(),
);
}
};
Expand All @@ -326,7 +326,7 @@ fn upgrade_stage_2_and_3a_create_execution_state_and_call_start(
original,
round,
err,
helper.take_canister_log(),
helper.clone_log_memory_store(),
);
}

Expand Down Expand Up @@ -439,7 +439,7 @@ fn upgrade_stage_3b_process_start_result(
original,
round,
err,
helper.take_canister_log(),
helper.clone_log_memory_store(),
);
}

Expand Down Expand Up @@ -554,7 +554,7 @@ fn upgrade_stage_4b_process_post_upgrade_result(
original,
round,
err,
helper.take_canister_log(),
helper.clone_log_memory_store(),
);
}
helper.finish(clean_canister, original, round, round_limits)
Expand Down
13 changes: 6 additions & 7 deletions rs/execution_environment/tests/backtraces.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ fn assert_error(
.execute_ingress_as(CONTROLLER, canister_id, method, Encode!(&()).unwrap())
.unwrap_err();
result.assert_matches(code, &format!("{message}.*{backtrace}.*"));
let logs = env.canister_log(canister_id);
let last_error = std::str::from_utf8(&logs.records().back().as_ref().unwrap().content).unwrap();
let logs = env.canister_log_records(canister_id);
let last_error = std::str::from_utf8(&logs.last().as_ref().unwrap().content).unwrap();
let backtrace_regex = RegexBuilder::new(&format!(".*{backtrace}.*"))
.dot_matches_new_line(true)
.build()
Expand Down Expand Up @@ -129,8 +129,8 @@ fn no_backtrace_without_name_section() {
"Result message: {} cointains unexpected 'Backtrace'",
result.description(),
);
let logs = env.canister_log(canister_id);
for log in logs.records() {
let logs = env.canister_log_records(canister_id);
for log in &logs {
let log = std::str::from_utf8(&log.content).unwrap();
assert!(
!log.contains("Backtrace"),
Expand Down Expand Up @@ -236,9 +236,8 @@ mod visibility {
);
}
// The backtrace should still be in the logs.
let logs = env.canister_log(canister_id);
let last_error =
std::str::from_utf8(&logs.records().back().as_ref().unwrap().content).unwrap();
let logs = env.canister_log_records(canister_id);
let last_error = std::str::from_utf8(&logs.last().as_ref().unwrap().content).unwrap();
assert!(
backtrace_regex.is_match(last_error),
"Last log:\n----------\n{last_error}\n----------\ndoesn't contain backtrace: {backtrace}"
Expand Down
Loading
Loading