feat: [DSM-103] Lower frequency storage charging

rs/execution_environment/src/scheduler.rs

@@ -808,14 +808,30 @@ impl SchedulerImpl {
     }
 
     /// Charge canisters for their resource allocation and usage. Canisters
-    /// that did not manage to pay are uninstalled.
-    /// This function is expected to be called at the end of a round.
+    /// that cannot pay are uninstalled.
+    ///
+    /// This function is expected to be called at the end of a round and
+    /// particularly after paused executions were aborted on checkpoint rounds.
     fn charge_canisters_for_resource_allocation_and_usage(
         &self,
         state: &mut ReplicatedState,
         subnet_size: usize,
         current_round: ExecutionRound,
+        current_round_type: ExecutionRoundType,
     ) {
+        // Because it is relatively expensive to make every canister mutable just to
+        // check whether 10 seconds have passed since it was last charged, only
+        // charge canisters every 50 rounds and/or on checkpoint rounds.
+        //
+        // The latter ensures that (because we skip canisters with paused
+        // executions), every canister is charged at least once per checkpoint
+        // interval.
+        if !current_round.get().is_multiple_of(50)
+            && current_round_type != ExecutionRoundType::CheckpointRound
+        {
+            return;
+        }
+
         let cost_schedule = state.get_own_cost_schedule();
         let state_time = state.time();
         let threshold_last_allocation_charge = state_time.saturating_sub(
@@ -1464,15 +1480,17 @@ impl Scheduler for SchedulerImpl {
                 round_schedule.finish_round(&mut final_state, current_round, &self.metrics);
             }
 
+            // Abort (some) paused executions.
             self.finish_round(&mut final_state, current_round_type);
 
-            // Charge canisters after (some) paused executions were aborted.
+            // Charge canisters after paused executions were aborted.
             {
                 let _timer = self.metrics.round_finalization_charge.start_timer();
                 self.charge_canisters_for_resource_allocation_and_usage(
                     &mut final_state,
                     registry_settings.subnet_size,
                     current_round,
+                    current_round_type,
                 );
             }
 

rs/execution_environment/src/scheduler/test_utilities.rs

@@ -675,6 +675,7 @@ impl SchedulerTest {
                 self.state.as_mut().unwrap(),
                 subnet_size,
                 ExecutionRound::from(0),
+                ExecutionRoundType::CheckpointRound,
             )
     }
 

rs/execution_environment/src/scheduler/tests/charging.rs

@@ -56,18 +56,19 @@ fn only_charge_for_allocation_after_specified_duration() {
     );
 
     // Don't charge because the time since the last charge is too small.
+    // Checkpoint round, to force charging for storage.
     test.set_time(initial_time + time_between_batches);
-    test.execute_round(ExecutionRoundType::OrdinaryRound);
+    test.execute_round(ExecutionRoundType::CheckpointRound);
 
     assert_eq!(
         test.canister_state(canister).system_state.balance().get(),
         initial_cycles
     );
 
     // The time of the current batch is now long enough that allocation charging
-    // should be triggered.
+    // should be triggered. Checkpoint round, to force charging for storage.
     test.set_time(initial_time + 2 * time_between_batches);
-    test.execute_round(ExecutionRoundType::OrdinaryRound);
+    test.execute_round(ExecutionRoundType::CheckpointRound);
     assert_eq!(
         test.canister_state(canister).system_state.balance().get(),
         initial_cycles - 10,
@@ -106,7 +107,7 @@ fn charging_for_message_memory_works() {
 
     // Send an ingress that triggers an inter-canister call. Because of the scheduler
     // configuration, we can only execute the ingress message but not the
-    // inter-canister message so this remain in the canister's input queue.
+    // inter-canister message so this remains in the canister's input queue.
     test.send_ingress(
         canister,
         ingress(1).call(other_side(canister, 1), on_response(1)),
@@ -220,7 +221,8 @@ fn canisters_with_insufficient_cycles_are_uninstalled() {
                 .duration_between_allocation_charges(),
     );
 
-    test.execute_round(ExecutionRoundType::OrdinaryRound);
+    // Checkpoint round, to force charging for storage.
+    test.execute_round(ExecutionRoundType::CheckpointRound);
 
     for canister in test.state().canisters_iter() {
         assert!(canister.execution_state.is_none());
@@ -455,7 +457,9 @@ fn snapshot_is_deleted_when_canister_is_out_of_cycles() {
                     .cycles_account_manager
                     .duration_between_allocation_charges(),
     );
-    test.execute_round(ExecutionRoundType::OrdinaryRound);
+    // Checkpoint round, to force charging for storage.
+    test.execute_round(ExecutionRoundType::CheckpointRound);
+
     assert_eq!(
         test.scheduler()
             .metrics
@@ -591,7 +595,9 @@ fn snapshot_is_deleted_when_uninstalled_canister_is_out_of_cycles() {
                     .cycles_account_manager
                     .duration_between_allocation_charges(),
     );
-    test.execute_round(ExecutionRoundType::OrdinaryRound);
+    // Checkpoint round, to force charging for storage.
+    test.execute_round(ExecutionRoundType::CheckpointRound);
+
     assert_eq!(
         test.scheduler()
             .metrics

rs/execution_environment/tests/canister_history.rs

@@ -635,7 +635,9 @@ fn canister_history_cleared_if_canister_out_of_cycles() {
         / compute_percent_allocated_per_second_fee.get() as u64;
     now += Duration::from_secs(seconds_to_burn_balance + 1);
     env.set_time(now);
-    env.tick();
+    // canisters are always charged for storage on checkpoint rounds
+    env.checkpointed_tick();
+
     // check canister history
     let total_num_change_entries = reference_change_entries.len();
     reference_change_entries.clear();

rs/execution_environment/tests/execution_test.rs

@@ -476,7 +476,8 @@ fn canister_has_zero_balance_when_uninstalled_due_to_low_cycles() {
     let seconds_to_burn_balance = env.cycle_balance(canister_id) as u64
         / compute_percent_allocated_per_second_fee.get() as u64;
     env.advance_time(Duration::from_secs(seconds_to_burn_balance + 1));
-    env.tick();
+    // Checkpoint round, to force charging for storage.
+    env.checkpointed_tick();
 
     // Verify the original canister still exists but it's uninstalled and has a
     // zero cycle balance.

rs/execution_environment/tests/subnet_size_test.rs

@@ -304,7 +304,8 @@ fn simulate_one_gib_per_second_cost(
     env.advance_time(duration_between_allocation_charges);
 
     let balance_before = env.cycle_balance(canister_id);
-    env.tick();
+    // Checkpoint round, to force charging for storage.
+    env.checkpointed_tick();
     let balance_after = env.cycle_balance(canister_id);
 
     // Scale the cost from a defined in config value to a 1 second duration.