Skip to content

Commit 6224cc4

Browse files
authored
fix: EXC-1775: Replace delete_snapshots functionality with remove (#2395)
Eliminate code duplication by replacing `CanisterSnapshots::delete_snapshots` functionality with `CanisterSnapshots::remove`.
1 parent 9346d96 commit 6224cc4

File tree

2 files changed

+41
-5
lines changed

2 files changed

+41
-5
lines changed

rs/execution_environment/tests/execution_test.rs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -849,6 +849,45 @@ fn canister_snapshot_metrics_are_observed() {
849849
assert_eq!(gauge, 1.0);
850850
}
851851

852+
#[test]
853+
fn canister_snapshot_metrics_are_consistent_after_canister_deletion() {
854+
let env = StateMachineBuilder::new()
855+
.with_subnet_type(SubnetType::Application)
856+
.build();
857+
858+
let canister_id = create_universal_canister_with_cycles(
859+
&env,
860+
Some(CanisterSettingsArgsBuilder::new().build()),
861+
INITIAL_CYCLES_BALANCE,
862+
);
863+
864+
env.take_canister_snapshot(TakeCanisterSnapshotArgs::new(canister_id, None))
865+
.unwrap();
866+
867+
let count = fetch_gauge(env.metrics_registry(), "scheduler_num_canister_snapshots").unwrap();
868+
assert_eq!(count, 1.0);
869+
let memory_usage = fetch_gauge(
870+
env.metrics_registry(),
871+
"scheduler_canister_snapshots_memory_usage_bytes",
872+
)
873+
.unwrap();
874+
assert_gt!(memory_usage, 0.0);
875+
876+
env.stop_canister(canister_id)
877+
.expect("Error stopping canister.");
878+
env.delete_canister(canister_id)
879+
.expect("Error deleting canister.");
880+
881+
let count = fetch_gauge(env.metrics_registry(), "scheduler_num_canister_snapshots").unwrap();
882+
assert_eq!(count, 0.0);
883+
let memory_usage = fetch_gauge(
884+
env.metrics_registry(),
885+
"scheduler_canister_snapshots_memory_usage_bytes",
886+
)
887+
.unwrap();
888+
assert_eq!(memory_usage, 0.0);
889+
}
890+
852891
fn assert_replied(result: Result<WasmResult, UserError>) {
853892
match result {
854893
Ok(wasm_result) => match wasm_result {

rs/replicated_state/src/canister_snapshots.rs

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -126,12 +126,9 @@ impl CanisterSnapshots {
126126
/// Additionally, new items are added to the `unflushed_changes`,
127127
/// representing the deleted backups since the last flush to the disk.
128128
pub fn delete_snapshots(&mut self, canister_id: CanisterId) {
129-
if let Some(snapshot_ids) = self.snapshot_ids.remove(&canister_id) {
129+
if let Some(snapshot_ids) = self.snapshot_ids.get(&canister_id).cloned() {
130130
for snapshot_id in snapshot_ids {
131-
debug_assert!(self.snapshots.contains_key(&snapshot_id));
132-
self.snapshots.remove(&snapshot_id).unwrap();
133-
self.unflushed_changes
134-
.push(SnapshotOperation::Delete(snapshot_id));
131+
self.remove(snapshot_id);
135132
}
136133
}
137134
}

0 commit comments

Comments
 (0)