From fbe26d400c9aad8fe0e245542f3bac93fea181a7 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Wed, 2 Sep 2026 09:03:10 +0000 Subject: [PATCH 1/6] feat: Add a monotonic `consumed_cycles_as_counter` to `CanisterMetrics` `CanisterMetrics::consumed_cycles` behaves like a gauge: a prepayment raises it and the matching refund lowers it again. That makes it awkward to build monitoring on top of, which is why `consumed_cycles_by_use_cases` already has a monotonic `consumed_cycles_by_use_cases_as_counters` twin, only bumped once the refund of a prepayment is known. This adds the same twin for the scalar total, plus the migration that the by-use-case counters never got, so the new counter starts out with the full history rather than from zero. The migration works because the two metrics are tied by an invariant: consumed_cycles - outstanding prepayments == consumed_cycles_as_counter Only `Instructions` and `RequestAndResponseTransmission` charges are ever refunded, and an outstanding prepayment of either is always recorded in the replicated state -- in the `Callback` of a call that has not been responded to, or in the `prepaid_execution_cycles` of an aborted execution or `install_code`. `SystemState::outstanding_prepayments` sums them up, so the counter can be derived exactly from the gauge. And since the invariant holds at all times, not just before the counter is first observed, deriving it is idempotent: no cutoff point is needed, the backfill can be redone in every round, and it is self-healing if a downgrade drops the counter. The backfill runs on checkpoint rounds only, after `abort_all_paused_executions`: a paused execution is ephemeral, so its prepayment is not part of the replicated state (except for a paused response execution, which is paid for by the callback that the task carries); aborting materializes it into the canister's task queue. Like the gauge it mirrors, the counter covers everything except HTTPS outcalls, which are only tracked at the subnet level. The subnet-level aggregate is monotonic already, so the exported `replicated_state_consumed_cycles_since_replica_started_as_counter` is simply that plus the canisters' counters. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/canister_manager.rs | 7 + rs/execution_environment/src/scheduler.rs | 59 +++ .../src/scheduler/tests/metrics.rs | 164 ++++++++- .../v1/canister_state_bits.proto | 9 + .../gen/state/state.canister_state_bits.v1.rs | 11 + .../src/canister_state/system_state.rs | 174 +++++++++ .../src/canister_state/tests.rs | 336 +++++++++++++++++- rs/replicated_state/src/metrics.rs | 28 +- rs/state_layout/src/state_layout.rs | 1 + rs/state_layout/src/state_layout/proto.rs | 10 + rs/state_layout/src/state_layout/tests.rs | 47 +++ rs/state_manager/src/checkpoint.rs | 1 + rs/state_manager/src/tip.rs | 4 + 13 files changed, 847 insertions(+), 4 deletions(-) diff --git a/rs/execution_environment/src/canister_manager.rs b/rs/execution_environment/src/canister_manager.rs index 485da60a882b..776d76c3ab17 100644 --- a/rs/execution_environment/src/canister_manager.rs +++ b/rs/execution_environment/src/canister_manager.rs @@ -1375,6 +1375,13 @@ impl CanisterManager { ); // Leftover cycles in the canister are considered `consumed`. + // + // Note that the canister's `consumed_cycles` gauge (rather than its monotonic + // `consumed_cycles_as_counter`) is what is moved into the subnet metrics + // below. The two are equal here: a canister can only be deleted while + // `Stopped` and with empty queues, so it has no callback and no execution + // left holding an outstanding prepayment (see + // `SystemState::outstanding_prepayments`). let leftover_cycles = self .cycles_account_manager .leftover_cycles_for_canister_to_deleted( diff --git a/rs/execution_environment/src/scheduler.rs b/rs/execution_environment/src/scheduler.rs index c63b673ffdcf..3f4b31c5d157 100644 --- a/rs/execution_environment/src/scheduler.rs +++ b/rs/execution_environment/src/scheduler.rs @@ -1120,6 +1120,22 @@ impl SchedulerImpl { // Abort all paused execution before the checkpoint. abort_all_paused_executions(state, &self.exec_env, cost_schedule, &self.log); + + // Backfill the monotonic `consumed_cycles_as_counter` of every + // canister from its `consumed_cycles` gauge, which predates it. + // + // Only done on checkpoint rounds, and only after the paused + // executions above have been aborted: a paused execution holds a + // prepayment that is not part of the replicated state, which would + // make the backfill overestimate the counter (see + // `SystemState::outstanding_prepayments`). Aborting materializes + // those prepayments into the canisters' task queues. + // + // Unconditional and idempotent, like + // `migrate_outcalls_cycles_to_use_cases` above: it is a no-op once a + // canister has been backfilled, and self-healing if a downgrade + // dropped the counter. + migrate_consumed_cycles_to_counters(state); } ExecutionRoundType::OrdinaryRound => { self.abort_paused_executions_above_limit(state); @@ -2154,3 +2170,46 @@ pub fn abort_all_paused_executions( abort_canister(canister, subnet_schedule, exec_env, cost_schedule, log); } } + +/// Backfills the monotonic `CanisterMetrics::consumed_cycles_as_counter` of every +/// canister from its `consumed_cycles` gauge, which predates it and thus holds the +/// full history. See `SystemState::migrated_consumed_cycles_as_counter`. +/// +/// Must only be called with no paused executions left (i.e. on a checkpoint round, +/// after `abort_all_paused_executions`); canisters that still have one are skipped, +/// as their prepayment is not part of the replicated state. +fn migrate_consumed_cycles_to_counters(state: &mut ReplicatedState) { + state.canisters_for_each_mut(|_id, canister| { + let metrics = canister.system_state.canister_metrics(); + // A canister with a paused execution is skipped, its prepayment is not part + // of the replicated state. This should not happen when called as documented, + // but it is not worth a panic. + let Some(outstanding) = canister.system_state.outstanding_prepayments() else { + debug_assert!(false, "Called with a paused execution left over"); + return; + }; + let derived = metrics.consumed_cycles() - outstanding; + match derived.cmp(&metrics.consumed_cycles_as_counter()) { + // Not backfilled yet, or a downgrade dropped the counter. Only take a + // mutable reference here, so that this stays a read-only pass once every + // canister has been backfilled (`Arc::make_mut` clones the canister). + std::cmp::Ordering::Greater => { + Arc::make_mut(canister) + .system_state + .migrate_consumed_cycles_to_counter(); + } + // The invariant that makes the backfill exact; see + // `SystemState::outstanding_prepayments`. + std::cmp::Ordering::Equal => {} + std::cmp::Ordering::Less => debug_assert!( + false, + "Canister {}: consumed cycles counter {} above the gauge {} net of the \ + {} outstanding prepayments", + canister.canister_id(), + metrics.consumed_cycles_as_counter(), + metrics.consumed_cycles(), + outstanding, + ), + } + }); +} diff --git a/rs/execution_environment/src/scheduler/tests/metrics.rs b/rs/execution_environment/src/scheduler/tests/metrics.rs index 9f4da3dccd10..04e7464ac164 100644 --- a/rs/execution_environment/src/scheduler/tests/metrics.rs +++ b/rs/execution_environment/src/scheduler/tests/metrics.rs @@ -24,7 +24,7 @@ use ic_replicated_state::metadata_state::testing::{NetworkTopologyTesting, Syste use ic_replicated_state::metrics::ReplicatedStateMetrics; use ic_replicated_state::testing::{ReplicatedStateTesting, SystemStateTesting}; use ic_test_utilities_metrics::{ - HistogramStats, MetricVec, fetch_counter_vec, fetch_gauge, fetch_gauge_vec, + HistogramStats, MetricVec, fetch_counter, fetch_counter_vec, fetch_gauge, fetch_gauge_vec, fetch_histogram_stats, fetch_histogram_vec_stats, fetch_int_gauge, fetch_int_gauge_vec, metric_vec, nonzero_values, }; @@ -1104,6 +1104,141 @@ fn threshold_signature_agreements_metric_is_updated() { assert!(sign_with_threshold_contexts.is_empty()); } +/// Asserts the invariant that ties `CanisterMetrics::consumed_cycles` to its +/// monotonic counterpart: the gauge exceeds the counter by exactly the prepayments +/// whose refund is still outstanding. Returns the latter. +fn assert_consumed_cycles_invariant( + test: &SchedulerTest, + canister_id: CanisterId, +) -> NominalCycles { + let system_state = &test.canister_state(canister_id).system_state; + let outstanding = system_state + .outstanding_prepayments() + .expect("Canister has a paused execution"); + let metrics = system_state.canister_metrics(); + assert_eq!( + metrics.consumed_cycles() - outstanding, + metrics.consumed_cycles_as_counter(), + "consumed cycles gauge {} minus the {outstanding} outstanding prepayments \ + must equal the counter {}", + metrics.consumed_cycles(), + metrics.consumed_cycles_as_counter(), + ); + outstanding +} + +/// Opens a call to an xnet canister, so that the caller is left with an outstanding +/// prepayment for the response. +fn call_xnet_canister(test: &mut SchedulerTest, canister: CanisterId) { + let xnet_canister = test.xnet_canister_id(); + test.send_ingress( + canister, + ingress(1).call(other_side(xnet_canister, 1), on_response(1)), + ); + test.execute_round(ExecutionRoundType::OrdinaryRound); +} + +#[test] +fn consumed_cycles_as_counter_matches_the_gauge_net_of_outstanding_prepayments() { + let mut test = SchedulerTestBuilder::new().build(); + let canister = test.create_canister(); + + call_xnet_canister(&mut test, canister); + + let outstanding = assert_consumed_cycles_invariant(&test, canister); + assert_ne!(outstanding, NominalCycles::zero()); + assert_ne!( + test.canister_state(canister) + .system_state + .canister_metrics() + .consumed_cycles_as_counter(), + NominalCycles::zero() + ); +} + +#[test] +fn checkpoint_round_backfills_consumed_cycles_as_counter() { + let mut test = SchedulerTestBuilder::new().build(); + let canister = test.create_canister(); + + call_xnet_canister(&mut test, canister); + let outstanding = assert_consumed_cycles_invariant(&test, canister); + assert_ne!(outstanding, NominalCycles::zero()); + + // Pretend the canister was loaded from a checkpoint predating the counter. + test.canister_state_mut(canister) + .system_state + .reset_consumed_cycles_as_counter(); + assert_eq!( + test.canister_state(canister) + .system_state + .canister_metrics() + .consumed_cycles_as_counter(), + NominalCycles::zero() + ); + + // An ordinary round does not backfill the counter, a checkpoint round does. + test.execute_round(ExecutionRoundType::OrdinaryRound); + assert_eq!( + test.canister_state(canister) + .system_state + .canister_metrics() + .consumed_cycles_as_counter(), + NominalCycles::zero() + ); + + test.execute_round(ExecutionRoundType::CheckpointRound); + assert_consumed_cycles_invariant(&test, canister); + assert_ne!( + test.canister_state(canister) + .system_state + .canister_metrics() + .consumed_cycles_as_counter(), + NominalCycles::zero() + ); +} + +/// A paused execution holds a prepayment that is not part of the replicated state, +/// so it cannot be backfilled; but a checkpoint round aborts all paused executions +/// before backfilling, materializing their prepayments into the task queues. +#[test] +fn checkpoint_round_backfills_consumed_cycles_as_counter_of_paused_canister() { + let mut test = SchedulerTestBuilder::new() + .with_scheduler_config(SchedulerConfig { + scheduler_cores: 2, + max_instructions_per_round: NumInstructions::from(100), + max_instructions_per_message: NumInstructions::from(1000), + max_instructions_per_slice: NumInstructions::from(100), + max_instructions_per_install_code_slice: NumInstructions::from(100), + ..SchedulerConfig::application_subnet() + }) + .build(); + let canister = test.create_canister(); + + test.send_ingress(canister, ingress(1000)); + test.execute_round(ExecutionRoundType::OrdinaryRound); + assert!(test.canister_state(canister).has_paused_execution()); + assert_eq!( + test.canister_state(canister) + .system_state + .outstanding_prepayments(), + None + ); + + // Pretend the canister was loaded from a checkpoint predating the counter. + test.canister_state_mut(canister) + .system_state + .reset_consumed_cycles_as_counter(); + + test.execute_round(ExecutionRoundType::CheckpointRound); + + assert!(test.canister_state(canister).has_aborted_execution()); + let outstanding = assert_consumed_cycles_invariant(&test, canister); + // The aborted execution's prepayment is outstanding, and is not part of the + // backfilled counter. + assert_ne!(outstanding, NominalCycles::zero()); +} + #[test] fn consumed_cycles_ecdsa_outcalls_are_added_to_consumed_cycles_total() { for cost_schedule in [ @@ -1434,6 +1569,33 @@ fn consumed_cycles_for_instructions_are_updated_from_valid_canisters() { } } +/// The exported total counter is the subnet-level aggregate (already monotonic) +/// plus the canisters' `consumed_cycles_as_counter`; i.e. the gauge it mirrors, net +/// of the outstanding prepayments. +#[test] +fn consumed_cycles_total_as_counter_is_exported() { + let mut test = SchedulerTestBuilder::new().build(); + let canister = test.create_canister(); + + call_xnet_canister(&mut test, canister); + let outstanding = assert_consumed_cycles_invariant(&test, canister); + assert_ne!(outstanding, NominalCycles::zero()); + + observe_state_metrics(&mut test, 0); + + let gauge = fetch_gauge( + test.metrics_registry(), + "replicated_state_consumed_cycles_since_replica_started", + ) + .unwrap(); + let counter = fetch_counter( + test.metrics_registry(), + "replicated_state_consumed_cycles_since_replica_started_as_counter", + ) + .unwrap(); + assert_eq!(gauge - outstanding.get() as f64, counter); +} + #[test] fn consumed_cycles_for_resource_allocations_are_updated_from_valid_canisters() { for cost_schedule in [ 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 78a4f5f592d7..69d868893d17 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 @@ -502,6 +502,15 @@ message CanisterStateBits { // These counters facilitate programming retrieval of metrics and performing // various aggregations on them more easily than their gauge counterparts. repeated ConsumedCyclesByUseCase consumed_cycles_by_use_cases_as_counters = 65; + // Total cycles consumed by the canister, presented as a counter: unlike + // `consumed_cycles`, which is lowered again whenever a refund is issued, this + // is only ever increased, by the actually consumed amount once the refund of a + // prepayment is known. + // + // Covers exactly the same use cases as `consumed_cycles`, i.e. everything + // except HTTPS outcalls, which are only tracked at the subnet level (and, for + // the canister, in `consumed_cycles_by_use_cases_as_counters`). + types.v1.NominalCycles consumed_cycles_as_counter = 71; CanisterHistory canister_history = 37; // Resource reservation cycles. state.queues.v1.Cycles reserved_balance = 38; 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 896b5130aaa8..f38db3f34079 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 @@ -750,6 +750,17 @@ pub struct CanisterStateBits { /// various aggregations on them more easily than their gauge counterparts. #[prost(message, repeated, tag = "65")] pub consumed_cycles_by_use_cases_as_counters: ::prost::alloc::vec::Vec, + /// Total cycles consumed by the canister, presented as a counter: unlike + /// `consumed_cycles`, which is lowered again whenever a refund is issued, this + /// is only ever increased, by the actually consumed amount once the refund of a + /// prepayment is known. + /// + /// Covers exactly the same use cases as `consumed_cycles`, i.e. everything + /// except HTTPS outcalls, which are only tracked at the subnet level (and, for + /// the canister, in `consumed_cycles_by_use_cases_as_counters`). + #[prost(message, optional, tag = "71")] + pub consumed_cycles_as_counter: + ::core::option::Option, #[prost(message, optional, tag = "37")] pub canister_history: ::core::option::Option, /// Resource reservation cycles. diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 7b30a37a4c14..b320954311ac 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -279,6 +279,7 @@ pub struct CanisterMetrics { instructions_executed: NumInstructions, load_metrics: LoadMetrics, consumed_cycles: NominalCycles, + consumed_cycles_as_counter: NominalCycles, consumed_cycles_by_use_cases: BTreeMap, consumed_cycles_by_use_cases_as_counters: BTreeMap, } @@ -290,6 +291,7 @@ impl CanisterMetrics { executed: u64, interrupted_during_execution: u64, consumed_cycles: NominalCycles, + consumed_cycles_as_counter: NominalCycles, consumed_cycles_by_use_cases: BTreeMap, consumed_cycles_by_use_cases_as_counters: BTreeMap, instructions_executed: NumInstructions, @@ -301,6 +303,7 @@ impl CanisterMetrics { executed, interrupted_during_execution, consumed_cycles, + consumed_cycles_as_counter, consumed_cycles_by_use_cases, consumed_cycles_by_use_cases_as_counters, instructions_executed, @@ -332,6 +335,20 @@ impl CanisterMetrics { self.consumed_cycles } + /// The monotonic counterpart of [`Self::consumed_cycles`]: it is only ever + /// increased, by the actually consumed amount (prepayment minus refund) once + /// the refund of a prepayment is known. + /// + /// Covers exactly the same use cases as [`Self::consumed_cycles`], i.e. + /// everything except HTTPS outcalls, which are only tracked at the subnet level + /// (and, for the canister, in [`Self::consumed_cycles_by_use_cases_as_counters`]). + /// + /// See `SystemState::outstanding_prepayments` for the invariant that ties the + /// two together. + pub fn consumed_cycles_as_counter(&self) -> NominalCycles { + self.consumed_cycles_as_counter + } + pub fn consumed_cycles_by_use_cases(&self) -> &BTreeMap { &self.consumed_cycles_by_use_cases } @@ -2209,6 +2226,7 @@ impl SystemState { | CyclesUseCase::CanisterCreation | CyclesUseCase::BurnedCycles => { *use_case_consumption_as_counter += prepayment; + self.canister_metrics.consumed_cycles_as_counter += prepayment; } CyclesUseCase::ECDSAOutcalls @@ -2225,6 +2243,7 @@ impl SystemState { *use_case_consumption -= refund; self.canister_metrics.consumed_cycles -= refund; *use_case_consumption_as_counter += prepayment - refund; + self.canister_metrics.consumed_cycles_as_counter += prepayment - refund; } } } @@ -2237,6 +2256,144 @@ impl SystemState { &mut self.canister_metrics } + /// The prepayments that have already been added to + /// [`CanisterMetrics::consumed_cycles`] but whose refund has not been observed + /// yet; i.e. the amounts that will be reported as the prepayment of a future + /// `ConsumingCycles::Refund` observation. + /// + /// Only `Instructions` and `RequestAndResponseTransmission` charges are ever + /// refunded (they are the only two `CyclesUseCaseRefundableKind`s) and an + /// outstanding prepayment of either is always recorded in the replicated state: + /// + /// * in the `Callback` of a call whose response has not been executed yet + /// (`prepayment_for_response_execution` and + /// `prepayment_for_call_transmission`); or + /// * in the `prepaid_execution_cycles` of a paused / aborted execution or + /// `install_code`. + /// + /// Together with how the two metrics are updated, this yields the invariant + /// + /// ```text + /// consumed_cycles - outstanding_prepayments() == consumed_cycles_as_counter + /// ``` + /// + /// which holds whenever no execution is in progress and is what + /// [`Self::migrate_consumed_cycles_to_counter`] relies on. + /// + /// Returns `None` if the canister has a paused execution whose prepayment is + /// not part of the replicated state: a paused execution is ephemeral, so + /// (except for a paused response execution, whose prepayments live in the + /// callback carried by the task) its prepayment is only held in memory. All + /// paused executions are aborted before a checkpoint, materializing their + /// prepayments into the state, so the caller can retry then. + pub fn outstanding_prepayments(&self) -> Option { + /// The prepayments made when the request behind `callback` was sent (see + /// `SandboxSafeSystemState::push_output_request`), to be refunded when its + /// response is executed. + fn callback_prepayments(callback: &Callback) -> NominalCycles { + // `prepayment_for_call_transmission` is zero for callbacks created before + // April 2026; the refund path falls back to + // `prepayment_for_response_transmission` for those, so mirror it here. + let transmission = if callback.prepayment_for_call_transmission.is_zero() { + callback.prepayment_for_response_transmission.nominal() + } else { + callback.prepayment_for_call_transmission.nominal() + }; + callback.prepayment_for_response_execution.nominal() + transmission + } + + let mut outstanding = NominalCycles::zero(); + + match self.task_queue.paused_or_aborted_task() { + Some(ExecutionTask::PausedExecution { input, .. }) => match input { + // A response execution prepays nothing of its own: it is paid for by + // the callback, which the task carries. + CanisterMessageOrTask::Message(CanisterMessage::Response { callback, .. }) => { + outstanding += callback_prepayments(callback) + } + // Any other paused execution holds its prepayment in memory only. + _ => return None, + }, + Some(ExecutionTask::PausedInstallCode(_)) => return None, + Some(ExecutionTask::AbortedExecution { + input, + prepaid_execution_cycles, + }) => { + // Zero for an aborted response execution, which prepays nothing of + // its own. + outstanding += prepaid_execution_cycles.nominal(); + // As for a paused response execution above. The callback was + // unregistered from the `CallContextManager` when the response was + // popped, so it is not also counted below; and nothing was refunded + // yet, because aborting discards the changes that the initial steps + // of the response execution made. + if let CanisterMessageOrTask::Message(CanisterMessage::Response { + callback, .. + }) = input + { + outstanding += callback_prepayments(callback); + } + } + Some(ExecutionTask::AbortedInstallCode { + prepaid_execution_cycles, + .. + }) => outstanding += prepaid_execution_cycles.nominal(), + Some( + task @ (ExecutionTask::Heartbeat + | ExecutionTask::GlobalTimer + | ExecutionTask::OnLowWasmMemory), + ) => debug_assert!(false, "Not a paused or aborted task: {task:?}"), + None => {} + } + + if let Some(call_context_manager) = self.call_context_manager() { + for callback in call_context_manager.callbacks().values() { + outstanding += callback_prepayments(callback); + } + } + + Some(outstanding) + } + + /// The value that [`CanisterMetrics::consumed_cycles_as_counter`] must have, + /// derived from the [`CanisterMetrics::consumed_cycles`] gauge, which predates + /// it and thus holds the full history. + /// + /// This derivation is exact, thanks to the invariant documented on + /// [`Self::outstanding_prepayments`]: the gauge differs from the counter by + /// exactly the prepayments whose refund is still outstanding, all of which are + /// recorded in the replicated state. And because the invariant holds at all + /// times (not just before the first observation of a canister), deriving the + /// counter this way is idempotent, so it is safe to redo it in every round and + /// after a downgrade has dropped the counter. + /// + /// Returns `None` if the canister has a paused execution whose prepayment is not + /// part of the replicated state (see [`Self::outstanding_prepayments`]). + pub fn migrated_consumed_cycles_as_counter(&self) -> Option { + let outstanding = self.outstanding_prepayments()?; + // `max` rather than a plain assignment: the counter must never go down, not + // even if a saturating subtraction somewhere made the gauge lag behind it. + Some( + self.canister_metrics + .consumed_cycles_as_counter + .max(self.canister_metrics.consumed_cycles - outstanding), + ) + } + + /// Backfills [`CanisterMetrics::consumed_cycles_as_counter`] with + /// [`Self::migrated_consumed_cycles_as_counter`]. Returns `false` (leaving the + /// counter untouched) if the latter cannot be derived; the caller is expected to + /// retry once paused executions have been aborted. + pub fn migrate_consumed_cycles_to_counter(&mut self) -> bool { + match self.migrated_consumed_cycles_as_counter() { + Some(counter) => { + self.canister_metrics.consumed_cycles_as_counter = counter; + true + } + None => false, + } + } + /// Clears all canister changes and their memory usage, /// but keeps the total number of changes recorded. pub fn clear_canister_history(&mut self) { @@ -2568,6 +2725,13 @@ pub mod testing { deadline: CoarseTime, ) -> (CallbackId, Arc); + /// Testing only: Registers the given callback and returns its ID. + fn with_raw_callback(&mut self, callback: Callback) -> CallbackId; + + /// Testing only: Resets `CanisterMetrics::consumed_cycles_as_counter`, e.g. to + /// simulate a canister loaded from a checkpoint predating the counter. + fn reset_consumed_cycles_as_counter(&mut self); + /// Testing only: sets the canister status. fn set_status(&mut self, status: CanisterStatus); @@ -2604,6 +2768,16 @@ pub mod testing { self.pop_input() } + fn with_raw_callback(&mut self, callback: Callback) -> CallbackId { + call_context_manager_mut(&mut self.status) + .unwrap() + .register_callback(callback) + } + + fn reset_consumed_cycles_as_counter(&mut self) { + self.canister_metrics.consumed_cycles_as_counter = NominalCycles::zero(); + } + fn set_status(&mut self, status: CanisterStatus) { self.status = status; } diff --git a/rs/replicated_state/src/canister_state/tests.rs b/rs/replicated_state/src/canister_state/tests.rs index 52228e942712..01efda9ff4f9 100644 --- a/rs/replicated_state/src/canister_state/tests.rs +++ b/rs/replicated_state/src/canister_state/tests.rs @@ -23,8 +23,9 @@ use ic_metrics::MetricsRegistry; use ic_test_utilities_types::ids::{canister_test_id, message_test_id, user_test_id}; use ic_test_utilities_types::messages::{RequestBuilder, ResponseBuilder}; use ic_types::messages::{ - CallContextId, CallbackId, CanisterCall, CanisterMessageOrTask, MAX_RESPONSE_COUNT_BYTES, - NO_DEADLINE, StopCanisterCallId, StopCanisterContext, + CallContextId, CallbackId, CanisterCall, CanisterMessageOrTask, CanisterTask, + MAX_RESPONSE_COUNT_BYTES, NO_DEADLINE, RequestMetadata, StopCanisterCallId, + StopCanisterContext, }; use ic_types::methods::{Callback, WasmClosure}; use ic_types::time::{CoarseTime, UNIX_EPOCH}; @@ -1049,6 +1050,337 @@ fn full_refund_resets_consumed_cycles() { } } +/// The monotonic `consumed_cycles_as_counter` is only bumped once the refund of a +/// prepayment is known, by the actually consumed amount; unlike the gauge, which is +/// bumped by the prepayment and lowered again by the refund. +#[test] +fn consumed_cycles_as_counter_accounts_for_refundable_use_cases_at_refund() { + fn test(cost_schedule: CanisterCyclesCostSchedule) { + let mut system_state = CanisterStateFixture::new().canister_state.system_state; + let ctx = format!( + "{:?} with {cost_schedule:?} cost schedule", + T::cycles_use_case() + ); + let prepaid = CompoundCycles::::new(Cycles::new(1000), cost_schedule); + let refund = CompoundCycles::::new(Cycles::new(100), cost_schedule); + + system_state.consume_cycles(prepaid); + assert_eq!( + system_state.canister_metrics().consumed_cycles(), + prepaid.nominal(), + "{ctx}" + ); + // Nothing on the counter yet, the refund is not known. + assert_eq!( + system_state.canister_metrics().consumed_cycles_as_counter(), + NominalCycles::zero(), + "{ctx}" + ); + + system_state.refund_cycles(prepaid, refund); + assert_eq!( + system_state.canister_metrics().consumed_cycles(), + (prepaid - refund).nominal(), + "{ctx}" + ); + assert_eq!( + system_state.canister_metrics().consumed_cycles_as_counter(), + (prepaid - refund).nominal(), + "{ctx}" + ); + } + + for cost_schedule in [ + CanisterCyclesCostSchedule::Normal, + CanisterCyclesCostSchedule::Free, + ] { + test::(cost_schedule); + test::(cost_schedule); + } +} + +/// A use case that is never refunded is accounted for on the counter right away. +#[test] +fn consumed_cycles_as_counter_accounts_for_final_use_cases_at_prepayment() { + let mut system_state = CanisterStateFixture::new().canister_state.system_state; + let charge = + CompoundCycles::::new(Cycles::new(1000), CanisterCyclesCostSchedule::Normal); + + system_state.consume_cycles(charge); + + assert_eq!( + system_state.canister_metrics().consumed_cycles(), + charge.nominal() + ); + assert_eq!( + system_state.canister_metrics().consumed_cycles_as_counter(), + charge.nominal() + ); +} + +/// A full refund lowers the gauge back to zero but must not lower the counter, which +/// was never bumped in the first place. +#[test] +fn full_refund_does_not_lower_consumed_cycles_as_counter() { + let mut system_state = CanisterStateFixture::new().canister_state.system_state; + let cost_schedule = CanisterCyclesCostSchedule::Normal; + let final_charge = CompoundCycles::::new(Cycles::new(500), cost_schedule); + let prepaid = CompoundCycles::::new(Cycles::new(1000), cost_schedule); + + system_state.consume_cycles(final_charge); + system_state.consume_cycles(prepaid); + system_state.refund_cycles(prepaid, prepaid); + + assert_eq!( + system_state.canister_metrics().consumed_cycles(), + final_charge.nominal() + ); + assert_eq!( + system_state.canister_metrics().consumed_cycles_as_counter(), + final_charge.nominal() + ); +} + +/// The prepayments of an open callback are outstanding until its response is +/// executed: `prepayment_for_response_execution` plus `prepayment_for_call_transmission`. +#[test] +fn outstanding_prepayments_of_open_callbacks() { + let mut fixture = CanisterStateFixture::new(); + let system_state = &mut fixture.canister_state.system_state; + assert_eq!( + system_state.outstanding_prepayments(), + Some(NominalCycles::zero()) + ); + + // See `SystemStateTesting::with_callback` for the prepayments of this callback. + fixture.make_callback(NO_DEADLINE); + assert_eq!( + fixture + .canister_state + .system_state + .outstanding_prepayments(), + Some(NominalCycles::new(42 + 168)) + ); + + fixture.make_callback(SOME_DEADLINE); + assert_eq!( + fixture + .canister_state + .system_state + .outstanding_prepayments(), + Some(NominalCycles::new(2 * (42 + 168))) + ); +} + +/// `prepayment_for_call_transmission` is zero for callbacks created before April +/// 2026; the refund path falls back to `prepayment_for_response_transmission` for +/// those, and so must the outstanding prepayments. +#[test] +fn outstanding_prepayments_of_legacy_callback() { + let cost_schedule = CanisterCyclesCostSchedule::Normal; + let mut fixture = CanisterStateFixture::new(); + let call_context_id = fixture + .canister_state + .system_state + .with_call_context(CallContext::new( + CallOrigin::SystemTask, + false, + false, + Cycles::zero(), + UNIX_EPOCH, + RequestMetadata::new(0, UNIX_EPOCH), + None, + )); + fixture + .canister_state + .system_state + .with_raw_callback(Callback::new( + call_context_id, + OTHER_CANISTER_ID, + Cycles::zero(), + CompoundCycles::new(Cycles::new(42), cost_schedule), + CompoundCycles::new(Cycles::new(84), cost_schedule), + CompoundCycles::new(Cycles::zero(), cost_schedule), + WasmClosure::new(0, 2), + WasmClosure::new(0, 2), + None, + NO_DEADLINE, + )); + + assert_eq!( + fixture + .canister_state + .system_state + .outstanding_prepayments(), + Some(NominalCycles::new(42 + 84)) + ); +} + +/// The prepayment of an aborted execution is outstanding until the execution is +/// retried; a paused execution's is not part of the replicated state. +#[test] +fn outstanding_prepayments_of_paused_and_aborted_executions() { + let cost_schedule = CanisterCyclesCostSchedule::Normal; + let prepaid = CompoundCycles::::new(Cycles::new(1000), cost_schedule); + let input = CanisterMessageOrTask::Task(CanisterTask::Heartbeat); + + let mut fixture = CanisterStateFixture::new(); + fixture + .canister_state + .system_state + .task_queue + .enqueue(ExecutionTask::AbortedExecution { + input: input.clone(), + prepaid_execution_cycles: prepaid, + }); + assert_eq!( + fixture + .canister_state + .system_state + .outstanding_prepayments(), + Some(prepaid.nominal()) + ); + + let mut fixture = CanisterStateFixture::new(); + fixture + .canister_state + .system_state + .task_queue + .enqueue(ExecutionTask::PausedExecution { + id: PausedExecutionId(0), + input, + }); + assert_eq!( + fixture + .canister_state + .system_state + .outstanding_prepayments(), + None + ); +} + +/// A paused or aborted response execution prepays nothing of its own: it is paid for +/// by the callback that the task carries, which is no longer registered with the +/// `CallContextManager` (so it must not be counted twice). +#[test] +fn outstanding_prepayments_of_aborted_response_execution() { + let mut fixture = CanisterStateFixture::new(); + let callback_id = fixture.make_callback(NO_DEADLINE); + let callback = fixture + .canister_state + .system_state + .call_context_manager() + .unwrap() + .callback(callback_id) + .unwrap() + .clone(); + let response = default_input_response(callback_id, NO_DEADLINE); + + let system_state = &mut fixture.canister_state.system_state; + system_state.unregister_callback(callback_id).unwrap(); + system_state + .task_queue + .enqueue(ExecutionTask::AbortedExecution { + input: CanisterMessageOrTask::Message(CanisterMessage::Response { + response: response.into(), + callback: callback.into(), + }), + prepaid_execution_cycles: CompoundCycles::new( + Cycles::zero(), + CanisterCyclesCostSchedule::Normal, + ), + }); + + assert_eq!( + system_state.outstanding_prepayments(), + Some(NominalCycles::new(42 + 168)) + ); +} + +/// Backfilling the counter from the gauge is exact, thanks to the invariant that the +/// gauge exceeds the counter by exactly the outstanding prepayments. And it is +/// idempotent, so it can be redone in every round. +#[test] +fn migrate_consumed_cycles_to_counter_is_exact_and_idempotent() { + let cost_schedule = CanisterCyclesCostSchedule::Normal; + let mut fixture = CanisterStateFixture::new(); + let system_state = &mut fixture.canister_state.system_state; + + // Some consumption with all refunds settled... + let final_charge = CompoundCycles::::new(Cycles::new(500), cost_schedule); + let prepaid = CompoundCycles::::new(Cycles::new(1000), cost_schedule); + let refund = CompoundCycles::::new(Cycles::new(100), cost_schedule); + system_state.consume_cycles(final_charge); + system_state.consume_cycles(prepaid); + system_state.refund_cycles(prepaid, refund); + let settled = final_charge.nominal() + (prepaid - refund).nominal(); + + // ...plus an outstanding prepayment for a call that has not been responded to. + let outstanding = CompoundCycles::::new(Cycles::new(42), cost_schedule).nominal() + + CompoundCycles::::new(Cycles::new(168), cost_schedule) + .nominal(); + system_state.consume_cycles(CompoundCycles::::new( + Cycles::new(42), + cost_schedule, + )); + system_state.consume_cycles(CompoundCycles::::new( + Cycles::new(168), + cost_schedule, + )); + fixture.make_callback(NO_DEADLINE); + let system_state = &mut fixture.canister_state.system_state; + + assert_eq!( + system_state.canister_metrics().consumed_cycles(), + settled + outstanding + ); + assert_eq!( + system_state.canister_metrics().consumed_cycles_as_counter(), + settled + ); + assert_eq!(system_state.outstanding_prepayments(), Some(outstanding)); + + // Pretend the canister was loaded from a checkpoint predating the counter. + system_state.reset_consumed_cycles_as_counter(); + assert!(system_state.migrate_consumed_cycles_to_counter()); + assert_eq!( + system_state.canister_metrics().consumed_cycles_as_counter(), + settled + ); + + // Redoing it changes nothing. + assert!(system_state.migrate_consumed_cycles_to_counter()); + assert_eq!( + system_state.canister_metrics().consumed_cycles_as_counter(), + settled + ); +} + +/// A canister with a paused execution cannot be backfilled, as the prepayment of the +/// paused execution is not part of the replicated state. +#[test] +fn migrate_consumed_cycles_to_counter_skips_paused_execution() { + let mut fixture = CanisterStateFixture::new(); + let system_state = &mut fixture.canister_state.system_state; + system_state.consume_cycles(CompoundCycles::::new( + Cycles::new(500), + CanisterCyclesCostSchedule::Normal, + )); + system_state + .task_queue + .enqueue(ExecutionTask::PausedExecution { + id: PausedExecutionId(0), + input: CanisterMessageOrTask::Task(CanisterTask::Heartbeat), + }); + + system_state.reset_consumed_cycles_as_counter(); + assert!(!system_state.migrate_consumed_cycles_to_counter()); + assert_eq!( + system_state.canister_metrics().consumed_cycles_as_counter(), + NominalCycles::zero() + ); +} + #[test] fn consume_cycles_exceeding_balance_reports_only_the_charged_amount() { fn test(cost_schedule: CanisterCyclesCostSchedule) { diff --git a/rs/replicated_state/src/metrics.rs b/rs/replicated_state/src/metrics.rs index 63e781dd69aa..50e895319ebb 100644 --- a/rs/replicated_state/src/metrics.rs +++ b/rs/replicated_state/src/metrics.rs @@ -14,7 +14,8 @@ use ic_types::{ }; use ic_types_cycles::{Cycles, CyclesUseCase, NominalCycles}; use prometheus::{ - CounterVec, Gauge, GaugeVec, Histogram, HistogramVec, IntCounter, IntGauge, IntGaugeVec, + Counter, CounterVec, Gauge, GaugeVec, Histogram, HistogramVec, IntCounter, IntGauge, + IntGaugeVec, }; use std::collections::BTreeMap; use std::time::Duration; @@ -55,6 +56,7 @@ pub struct ReplicatedStateMetrics { registered_canisters: IntGaugeVec, available_canister_ids: IntGauge, consumed_cycles: Gauge, + consumed_cycles_as_counter: Counter, consumed_cycles_by_use_case: GaugeVec, consumed_cycles_by_use_case_as_counters: CounterVec, input_queue_messages: IntGaugeVec, @@ -166,6 +168,15 @@ impl ReplicatedStateMetrics { "replicated_state_consumed_cycles_since_replica_started", "Number of cycles consumed", ), + consumed_cycles_as_counter: metrics_registry.register( + Counter::new( + "replicated_state_consumed_cycles_since_replica_started_as_counter", + "Number of cycles consumed, as a counter. Unlike its gauge \ + counterpart, this is only increased, by the actually consumed \ + amount once the refund of a prepayment is known.", + ) + .unwrap(), + ), consumed_cycles_by_use_case: metrics_registry.gauge_vec( "replicated_state_consumed_cycles_from_replica_start", "Number of cycles consumed by use cases.", @@ -416,6 +427,7 @@ impl ReplicatedStateMetrics { let mut consumed_cycles_total_by_use_case = BTreeMap::new(); let mut consumed_cycles_total_by_use_case_as_counters = BTreeMap::new(); + let mut consumed_cycles_by_canisters_as_counter = NominalCycles::zero(); let mut ingress_queue_message_count = 0; let mut ingress_queue_size_bytes = 0; @@ -480,6 +492,10 @@ impl ReplicatedStateMetrics { .canister_metrics() .consumed_cycles_by_use_cases(), ); + consumed_cycles_by_canisters_as_counter += canister + .system_state + .canister_metrics() + .consumed_cycles_as_counter(); // For the purpose of exporting the total counters to prometheus, filter out HTTPS // outcalls from canister level metrics as they will be added later from the subnet level metrics. // This only applies for the counter version of metrics as the gauge version only updates @@ -581,6 +597,16 @@ impl ReplicatedStateMetrics { .get() as f64, ); + // The subnet-level aggregate is monotonic already (subnet-level use cases are + // only ever charged, never refunded, and a deleted canister's consumption is + // moved into `consumed_cycles_by_deleted_canisters`), so it can be added to + // the canisters' counters as-is. + let consumed_cycles_as_counter = state.metadata.subnet_metrics.consumed_cycles_total() + + consumed_cycles_by_canisters_as_counter; + self.consumed_cycles_as_counter.reset(); + self.consumed_cycles_as_counter + .inc_by(consumed_cycles_as_counter.get() as f64); + self.observe_consumed_cycles_by_use_case(&consumed_cycles_total_by_use_case); self.observe_consumed_cycles_by_use_case_as_counters( &consumed_cycles_total_by_use_case_as_counters, diff --git a/rs/state_layout/src/state_layout.rs b/rs/state_layout/src/state_layout.rs index 7b150ed6034c..31f2d60672a1 100644 --- a/rs/state_layout/src/state_layout.rs +++ b/rs/state_layout/src/state_layout.rs @@ -197,6 +197,7 @@ pub struct CanisterStateBits { pub global_timer_nanos: Option, pub canister_version: u64, pub canister_creation_timestamp_nanos: Option, + pub consumed_cycles_as_counter: NominalCycles, pub consumed_cycles_by_use_cases: BTreeMap, pub consumed_cycles_by_use_cases_as_counters: BTreeMap, pub instructions_executed: NumInstructions, diff --git a/rs/state_layout/src/state_layout/proto.rs b/rs/state_layout/src/state_layout/proto.rs index 160e35a2872d..4526d155ec9f 100644 --- a/rs/state_layout/src/state_layout/proto.rs +++ b/rs/state_layout/src/state_layout/proto.rs @@ -34,6 +34,7 @@ impl From for pb_canister_state_bits::CanisterStateBits { interrupted_during_execution: item.interrupted_during_execution, certified_data: item.certified_data.clone(), consumed_cycles: Some((&item.consumed_cycles).into()), + consumed_cycles_as_counter: Some((&item.consumed_cycles_as_counter).into()), stable_memory_size64: item.stable_memory_size.get() as u64, heap_delta_debit: item.heap_delta_debit.get(), install_code_debit: item.install_code_debit.get(), @@ -102,6 +103,14 @@ impl TryFrom for CanisterStateBits { let consumed_cycles = try_from_option_field(value.consumed_cycles, "CanisterStateBits::consumed_cycles") .unwrap_or_default(); + // Absent in checkpoints written before the field was introduced; the + // scheduler backfills it from `consumed_cycles` (see + // `SystemState::migrate_consumed_cycles_to_counter`). + let consumed_cycles_as_counter: NominalCycles = try_from_option_field( + value.consumed_cycles_as_counter, + "CanisterStateBits::consumed_cycles_as_counter", + ) + .unwrap_or_default(); let mut controllers = BTreeSet::new(); for controller in value.controllers.into_iter() { @@ -191,6 +200,7 @@ impl TryFrom for CanisterStateBits { interrupted_during_execution: value.interrupted_during_execution, certified_data: value.certified_data, consumed_cycles, + consumed_cycles_as_counter, stable_memory_size: NumWasmPages::from(value.stable_memory_size64 as usize), heap_delta_debit: NumBytes::from(value.heap_delta_debit), install_code_debit: NumInstructions::from(value.install_code_debit), diff --git a/rs/state_layout/src/state_layout/tests.rs b/rs/state_layout/src/state_layout/tests.rs index 73cde100f919..5071640c902f 100644 --- a/rs/state_layout/src/state_layout/tests.rs +++ b/rs/state_layout/src/state_layout/tests.rs @@ -19,6 +19,7 @@ use ic_types::messages::{ }; use ic_types::methods::{Callback, WasmClosure}; use ic_types::time::{CoarseTime, UNIX_EPOCH}; +use ic_types_cycles::NominalCyclesTesting; use ic_types_cycles::{CanisterCyclesCostSchedule, CompoundCycles}; use itertools::Itertools; use proptest::prelude::*; @@ -45,6 +46,7 @@ fn default_canister_state_bits() -> CanisterStateBits { interrupted_during_execution: 0, certified_data: vec![], consumed_cycles: NominalCycles::zero(), + consumed_cycles_as_counter: NominalCycles::zero(), stable_memory_size: NumWasmPages::from(0), heap_delta_debit: NumBytes::from(0), install_code_debit: NumInstructions::from(0), @@ -134,6 +136,51 @@ fn test_encode_decode_non_empty_controllers() { assert_eq!(canister_state_bits.controllers, expected_controllers); } +#[test] +fn test_encode_decode_consumed_cycles_as_counter() { + let canister_state_bits = CanisterStateBits { + consumed_cycles: NominalCycles::new(1000), + consumed_cycles_as_counter: NominalCycles::new(900), + ..default_canister_state_bits() + }; + + let pb_bits = pb_canister_state_bits::CanisterStateBits::from(canister_state_bits); + let canister_state_bits = CanisterStateBits::try_from(pb_bits).unwrap(); + + assert_eq!( + canister_state_bits.consumed_cycles, + NominalCycles::new(1000) + ); + assert_eq!( + canister_state_bits.consumed_cycles_as_counter, + NominalCycles::new(900) + ); +} + +/// The field is absent in checkpoints written before it was introduced; the +/// scheduler backfills it from `consumed_cycles`. +#[test] +fn test_decode_missing_consumed_cycles_as_counter() { + let canister_state_bits = CanisterStateBits { + consumed_cycles: NominalCycles::new(1000), + consumed_cycles_as_counter: NominalCycles::new(900), + ..default_canister_state_bits() + }; + + let mut pb_bits = pb_canister_state_bits::CanisterStateBits::from(canister_state_bits); + pb_bits.consumed_cycles_as_counter = None; + let canister_state_bits = CanisterStateBits::try_from(pb_bits).unwrap(); + + assert_eq!( + canister_state_bits.consumed_cycles, + NominalCycles::new(1000) + ); + assert_eq!( + canister_state_bits.consumed_cycles_as_counter, + NominalCycles::zero() + ); +} + #[test] fn test_encode_decode_empty_history() { let canister_history = CanisterHistory::default(); diff --git a/rs/state_manager/src/checkpoint.rs b/rs/state_manager/src/checkpoint.rs index be6e11536dad..65e55b12017a 100644 --- a/rs/state_manager/src/checkpoint.rs +++ b/rs/state_manager/src/checkpoint.rs @@ -818,6 +818,7 @@ pub fn load_canister_state( canister_state_bits.executed, canister_state_bits.interrupted_during_execution, canister_state_bits.consumed_cycles, + canister_state_bits.consumed_cycles_as_counter, canister_state_bits.consumed_cycles_by_use_cases, canister_state_bits.consumed_cycles_by_use_cases_as_counters, canister_state_bits.instructions_executed, diff --git a/rs/state_manager/src/tip.rs b/rs/state_manager/src/tip.rs index a6de0c57380f..904e4a52847b 100644 --- a/rs/state_manager/src/tip.rs +++ b/rs/state_manager/src/tip.rs @@ -1425,6 +1425,10 @@ fn serialize_canister_protos_to_checkpoint_readwrite( .system_state .canister_metrics() .consumed_cycles(), + consumed_cycles_as_counter: canister_state + .system_state + .canister_metrics() + .consumed_cycles_as_counter(), stable_memory_size: canister_state .execution_state .as_ref() From 63d24caa7a6863aea1ddfdc63120fb16757dcb2e Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Wed, 2 Sep 2026 09:44:02 +0000 Subject: [PATCH 2/6] fix: Settle the prepayment of an `install_code` dropped by a subnet split `drop_in_progress_management_calls_after_split()` drops the canister's `AbortedInstallCode` task on subnet B, discarding the `prepaid_execution_cycles` recorded in it. The cycles are not returned to the canister, so they stay in the consumed cycles gauge -- but with the task gone there is nothing left in the replicated state to account for them, which breaks the invariant on `SystemState::outstanding_prepayments`: the split checkpoint would carry a `consumed_cycles_as_counter` lagging by the dropped prepayment until a later checkpoint round caught it up, and the `Instructions` by-use-case counter would never receive it at all. `remove_aborted_install_code_task()` now returns the prepayment and the caller settles it as a refund of zero. That leaves the balance and both gauges untouched -- nothing is actually refunded -- and only accounts for the prepayment on the monotonic counters, which is where a prepayment is recorded once its refund is known. Also documents why the exported total can drop across an online subnet split: the canisters migrating to the other subnet take their consumption with them. Retaining it here would count it on both subnets, and this total deliberately mirrors the certified `consumed_cycles_total_including_canisters`, which drops for the same reason. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/canister_state/system_state.rs | 21 ++++++- .../canister_state/system_state/task_queue.rs | 18 ++++-- .../src/canister_state/tests.rs | 62 +++++++++++++++++++ rs/replicated_state/src/metrics.rs | 8 +++ 4 files changed, 104 insertions(+), 5 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index b320954311ac..ae552df494a0 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -1673,7 +1673,22 @@ impl SystemState { // // Note that this cannot be a paused install code task, because we abort all // paused tasks before triggering the split. - self.task_queue.remove_aborted_install_code_task(); + // + // The cycles prepaid for the dropped execution are not returned to the + // canister, so settle them as fully consumed. This leaves the balance and the + // consumed cycles gauges untouched (the refund is zero) and only accounts for + // the prepayment on the monotonic counters, which is where a prepayment is + // recorded once its refund is known. Without this the prepayment would remain + // in the gauges with nothing left in the state to account for it, breaking the + // invariant on `Self::outstanding_prepayments`. + if let Some(prepaid_execution_cycles) = self.task_queue.remove_aborted_install_code_task() { + // Zero is zero under either cost schedule. + let zero_refund = CompoundCycles::::new( + Cycles::zero(), + CanisterCyclesCostSchedule::Normal, + ); + self.refund_cycles(prepaid_execution_cycles, zero_refund); + } // Roll back `Stopping` canister states to `Running` and drop all their stop // contexts (the calls corresponding to the dropped stop contexts will be @@ -2271,6 +2286,10 @@ impl SystemState { /// * in the `prepaid_execution_cycles` of a paused / aborted execution or /// `install_code`. /// + /// The one place where such a prepayment leaves the state without a refund is an + /// aborted `install_code` dropped by a subnet split; that path settles it onto + /// the counters, see `Self::drop_in_progress_management_calls_after_split`. + /// /// Together with how the two metrics are updated, this yields the invariant /// /// ```text diff --git a/rs/replicated_state/src/canister_state/system_state/task_queue.rs b/rs/replicated_state/src/canister_state/system_state/task_queue.rs index dc55df41850c..a27cd3d16465 100644 --- a/rs/replicated_state/src/canister_state/system_state/task_queue.rs +++ b/rs/replicated_state/src/canister_state/system_state/task_queue.rs @@ -4,6 +4,7 @@ use crate::ExecutionTask; use ic_management_canister_types_private::OnLowWasmMemoryHookStatus; use ic_types::CanisterId; use ic_types::NumBytes; +use ic_types_cycles::{CompoundCycles, Instructions}; use std::collections::VecDeque; /// `TaskQueue` represents the implementation of queue structure for canister tasks satisfying the following conditions: @@ -168,10 +169,19 @@ impl TaskQueue { } } - /// Removes aborted install code task. - pub fn remove_aborted_install_code_task(&mut self) { - if let Some(ExecutionTask::AbortedInstallCode { .. }) = &self.paused_or_aborted_task { - self.paused_or_aborted_task = None; + /// Removes the aborted install code task, if any, returning the execution cycles + /// that were prepaid for it. The caller is responsible for settling them. + pub fn remove_aborted_install_code_task(&mut self) -> Option> { + match &self.paused_or_aborted_task { + Some(ExecutionTask::AbortedInstallCode { + prepaid_execution_cycles, + .. + }) => { + let prepaid_execution_cycles = *prepaid_execution_cycles; + self.paused_or_aborted_task = None; + Some(prepaid_execution_cycles) + } + _ => None, } } diff --git a/rs/replicated_state/src/canister_state/tests.rs b/rs/replicated_state/src/canister_state/tests.rs index 01efda9ff4f9..f1d2de974a40 100644 --- a/rs/replicated_state/src/canister_state/tests.rs +++ b/rs/replicated_state/src/canister_state/tests.rs @@ -1807,6 +1807,68 @@ fn drops_aborted_canister_install_after_split() { assert_eq!(expected_state, canister_state); } +/// The cycles prepaid for an `install_code` that a subnet split drops are not +/// returned to the canister, so they must be settled as consumed. Otherwise the +/// prepayment would be left in the gauge with nothing in the state to account for +/// it, breaking the invariant on `SystemState::outstanding_prepayments`. +#[test] +fn settles_prepayment_of_aborted_canister_install_dropped_after_split() { + let cost_schedule = CanisterCyclesCostSchedule::Normal; + let prepaid = CompoundCycles::::new(Cycles::new(1000), cost_schedule); + let mut canister_state = CanisterStateFixture::new().canister_state; + + let system_state = &mut canister_state.system_state; + system_state.consume_cycles(prepaid); + system_state + .task_queue + .enqueue(ExecutionTask::AbortedInstallCode { + message: CanisterCall::Request(Arc::new(RequestBuilder::new().build())), + call_id: InstallCodeCallId::new(0), + prepaid_execution_cycles: prepaid, + }); + + // The prepayment is outstanding, so it is in the gauge but not on the counter. + assert_eq!( + system_state.outstanding_prepayments(), + Some(prepaid.nominal()) + ); + assert_eq!( + system_state.canister_metrics().consumed_cycles(), + prepaid.nominal() + ); + assert_eq!( + system_state.canister_metrics().consumed_cycles_as_counter(), + NominalCycles::zero() + ); + let balance_before = system_state.balance(); + + canister_state.drop_in_progress_management_calls_after_split(); + + // Nothing is refunded, so the balance and the gauge are unchanged. But the + // prepayment is no longer outstanding, so it must have moved onto the counters. + let system_state = &canister_state.system_state; + assert_eq!(system_state.balance(), balance_before); + assert_eq!( + system_state.canister_metrics().consumed_cycles(), + prepaid.nominal() + ); + assert_eq!( + system_state.outstanding_prepayments(), + Some(NominalCycles::zero()) + ); + assert_eq!( + system_state.canister_metrics().consumed_cycles_as_counter(), + prepaid.nominal() + ); + assert_eq!( + system_state + .canister_metrics() + .consumed_cycles_by_use_cases_as_counters() + .get(&CyclesUseCase::Instructions), + Some(&prepaid.nominal()) + ); +} + #[test] fn reverts_stopping_status_after_split() { let mut canister_state = CanisterStateFixture::new().canister_state; diff --git a/rs/replicated_state/src/metrics.rs b/rs/replicated_state/src/metrics.rs index 50e895319ebb..602a9cf6915f 100644 --- a/rs/replicated_state/src/metrics.rs +++ b/rs/replicated_state/src/metrics.rs @@ -601,6 +601,14 @@ impl ReplicatedStateMetrics { // only ever charged, never refunded, and a deleted canister's consumption is // moved into `consumed_cycles_by_deleted_canisters`), so it can be added to // the canisters' counters as-is. + // + // The one thing that can lower this total is an online subnet split, which + // hands the canisters migrating to the other subnet -- and with them their + // consumption -- over to that subnet. Consumers see a counter reset, which + // they handle. Retaining the migrated canisters' consumption here instead + // would count it on both subnets, and this total deliberately mirrors the + // certified `consumed_cycles_total_including_canisters`, which drops for the + // same reason. let consumed_cycles_as_counter = state.metadata.subnet_metrics.consumed_cycles_total() + consumed_cycles_by_canisters_as_counter; self.consumed_cycles_as_counter.reset(); From 44be0cbbe92d4f4243049666668c3bc05874f657 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Wed, 2 Sep 2026 09:57:26 +0000 Subject: [PATCH 3/6] fix: Refund (rather than consume) the prepayment of an `install_code` dropped by a split Amends the previous commit, which settled the prepayment of the `AbortedInstallCode` task dropped by `online_split()` as fully consumed. Refunding it in full is the better answer: the canister has nothing to show for those cycles. An aborted execution discards the slices it had already run and starts over when retried, and this one is never retried -- subnet A' rejects the corresponding call, unwinding the operation there as well. Before this PR the task was dropped with no accounting at all, silently leaving the canister charged for it. The refund is deterministic across subnet B's replicas and conserves cycles: they move back from consumed to the canister's balance. The consumed cycles metrics stay consistent either way -- both settle the prepayment, so the invariant on `SystemState::outstanding_prepayments` holds -- but a full refund lowers the gauges by the prepayment and adds nothing to the monotonic counters, which is exactly right when nothing was consumed. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/canister_state/system_state.rs | 27 ++++++++++--------- .../src/canister_state/tests.rs | 25 +++++++++-------- rs/replicated_state/tests/replicated_state.rs | 15 ++++++----- 3 files changed, 37 insertions(+), 30 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index ae552df494a0..2b2881afb41e 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -1674,20 +1674,21 @@ impl SystemState { // Note that this cannot be a paused install code task, because we abort all // paused tasks before triggering the split. // - // The cycles prepaid for the dropped execution are not returned to the - // canister, so settle them as fully consumed. This leaves the balance and the - // consumed cycles gauges untouched (the refund is zero) and only accounts for - // the prepayment on the monotonic counters, which is where a prepayment is - // recorded once its refund is known. Without this the prepayment would remain - // in the gauges with nothing left in the state to account for it, breaking the - // invariant on `Self::outstanding_prepayments`. + // Refund the execution cycles prepaid for the dropped `install_code` in full. + // The canister has nothing to show for them: an aborted execution discards the + // slices it had already run and starts over when retried, and this one is + // never retried -- subnet A' rejects the corresponding call, unwinding the + // operation there as well. + // + // This also keeps the consumed cycles metrics consistent. Dropping the task + // without refunding would leave the prepayment in the gauges with nothing left + // in the state to account for it, breaking the invariant on + // `Self::outstanding_prepayments`; and the monotonic counters, which only + // account for a prepayment once its refund is known, would never see it at + // all. A full refund lowers the gauges by the prepayment and adds nothing to + // the counters, which is exactly right: nothing was consumed. if let Some(prepaid_execution_cycles) = self.task_queue.remove_aborted_install_code_task() { - // Zero is zero under either cost schedule. - let zero_refund = CompoundCycles::::new( - Cycles::zero(), - CanisterCyclesCostSchedule::Normal, - ); - self.refund_cycles(prepaid_execution_cycles, zero_refund); + self.refund_cycles(prepaid_execution_cycles, prepaid_execution_cycles); } // Roll back `Stopping` canister states to `Running` and drop all their stop diff --git a/rs/replicated_state/src/canister_state/tests.rs b/rs/replicated_state/src/canister_state/tests.rs index f1d2de974a40..6f4b2660bd3d 100644 --- a/rs/replicated_state/src/canister_state/tests.rs +++ b/rs/replicated_state/src/canister_state/tests.rs @@ -1807,12 +1807,13 @@ fn drops_aborted_canister_install_after_split() { assert_eq!(expected_state, canister_state); } -/// The cycles prepaid for an `install_code` that a subnet split drops are not -/// returned to the canister, so they must be settled as consumed. Otherwise the -/// prepayment would be left in the gauge with nothing in the state to account for -/// it, breaking the invariant on `SystemState::outstanding_prepayments`. +/// The cycles prepaid for an `install_code` that a subnet split drops are refunded +/// in full: the canister has nothing to show for them, as the execution is never +/// retried. This also keeps the consumed cycles metrics consistent -- leaving the +/// prepayment in the gauge with nothing in the state to account for it would break +/// the invariant on `SystemState::outstanding_prepayments`. #[test] -fn settles_prepayment_of_aborted_canister_install_dropped_after_split() { +fn refunds_prepayment_of_aborted_canister_install_dropped_after_split() { let cost_schedule = CanisterCyclesCostSchedule::Normal; let prepaid = CompoundCycles::::new(Cycles::new(1000), cost_schedule); let mut canister_state = CanisterStateFixture::new().canister_state; @@ -1841,16 +1842,18 @@ fn settles_prepayment_of_aborted_canister_install_dropped_after_split() { NominalCycles::zero() ); let balance_before = system_state.balance(); + assert_eq!(balance_before, INITIAL_CYCLES - prepaid.real()); canister_state.drop_in_progress_management_calls_after_split(); - // Nothing is refunded, so the balance and the gauge are unchanged. But the - // prepayment is no longer outstanding, so it must have moved onto the counters. + // The prepayment is refunded in full, so the balance is whole again and the gauge + // is back to zero. Nothing was consumed, so the counters stay at zero too -- and + // with the prepayment no longer outstanding, the invariant still holds. let system_state = &canister_state.system_state; - assert_eq!(system_state.balance(), balance_before); + assert_eq!(system_state.balance(), balance_before + prepaid.real()); assert_eq!( system_state.canister_metrics().consumed_cycles(), - prepaid.nominal() + NominalCycles::zero() ); assert_eq!( system_state.outstanding_prepayments(), @@ -1858,14 +1861,14 @@ fn settles_prepayment_of_aborted_canister_install_dropped_after_split() { ); assert_eq!( system_state.canister_metrics().consumed_cycles_as_counter(), - prepaid.nominal() + NominalCycles::zero() ); assert_eq!( system_state .canister_metrics() .consumed_cycles_by_use_cases_as_counters() .get(&CyclesUseCase::Instructions), - Some(&prepaid.nominal()) + Some(&NominalCycles::zero()) ); } diff --git a/rs/replicated_state/tests/replicated_state.rs b/rs/replicated_state/tests/replicated_state.rs index 9a3840877009..d48ff2681f7e 100644 --- a/rs/replicated_state/tests/replicated_state.rs +++ b/rs/replicated_state/tests/replicated_state.rs @@ -1360,7 +1360,9 @@ fn online_split() { take_shapshot(CANISTER_1); take_shapshot(CANISTER_2); - // Add aborted `install_code` tasks to both canisters. + // Add aborted `install_code` tasks to both canisters, with the same prepayment. + let prepaid_install_code_cycles = + CompoundCycles::::new(Cycles::new(3), CanisterCyclesCostSchedule::Normal); let mut add_aborted_install_code_task = |canister_id| { let canister = fixture.state.canister_state_make_mut(&canister_id).unwrap(); canister @@ -1369,10 +1371,7 @@ fn online_split() { .enqueue(ExecutionTask::AbortedInstallCode { message: CanisterCall::Request(RequestBuilder::default().build().into()), call_id: InstallCodeCallId::new(3_u64), - prepaid_execution_cycles: CompoundCycles::new( - Cycles::new(3), - CanisterCyclesCostSchedule::Normal, - ), + prepaid_execution_cycles: prepaid_install_code_cycles, }); // Canister must be in the subnet schedule. fixture.state.canister_priority_mut(canister_id); @@ -1432,8 +1431,12 @@ fn online_split() { canister_state .system_state .split_input_schedules(&CANISTER_2, expected.canister_states()); - // The in-progress `install_code` task should have been silently dropped. + // The in-progress `install_code` task should have been dropped, with the cycles + // prepaid for it refunded in full. canister_state.system_state.task_queue = Default::default(); + canister_state + .system_state + .refund_cycles(prepaid_install_code_cycles, prepaid_install_code_cycles); expected.put_canister_state(canister_state_arc); // Streams, subnet queues and refunds should be empty. From 80e6f2a34897bc206761ac0e69716668328a6b6a Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Wed, 2 Sep 2026 10:18:06 +0000 Subject: [PATCH 4/6] docs: Correct the `outstanding_prepayments` documentation Two inaccuracies in the doc comment, both spotted in review: * A paused execution was listed as recording a prepayment in its `prepaid_execution_cycles`. It has no such field: only `AbortedExecution` and `AbortedInstallCode` do. Restricted the bullet to those, and moved a note about the paused cases up to right after the list, where the reader meets them. * The paragraph on prepayments leaving the state without a refund was left over from before the subnet split path was changed to issue a full refund. It no longer reads as an exception to the invariant. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/canister_state/system_state.rs | 14 ++++++++++---- .../src/canister_state/system_state/task_queue.rs | 3 ++- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 2b2881afb41e..42646754358d 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -2284,12 +2284,18 @@ impl SystemState { /// * in the `Callback` of a call whose response has not been executed yet /// (`prepayment_for_response_execution` and /// `prepayment_for_call_transmission`); or - /// * in the `prepaid_execution_cycles` of a paused / aborted execution or + /// * in the `prepaid_execution_cycles` of an aborted execution or an aborted /// `install_code`. /// - /// The one place where such a prepayment leaves the state without a refund is an - /// aborted `install_code` dropped by a subnet split; that path settles it onto - /// the counters, see `Self::drop_in_progress_management_calls_after_split`. + /// (A paused execution records no prepayment of its own: a paused response + /// execution is paid for by the callback that the task carries, and any other + /// paused execution holds its prepayment in memory only, which is why this + /// method returns `None` for it. See below.) + /// + /// The only way such a prepayment leaves the state other than through the refund + /// that its execution or response issues is an aborted `install_code` dropped by + /// a subnet split; that path refunds it in full, see + /// `Self::drop_in_progress_management_calls_after_split`. /// /// Together with how the two metrics are updated, this yields the invariant /// diff --git a/rs/replicated_state/src/canister_state/system_state/task_queue.rs b/rs/replicated_state/src/canister_state/system_state/task_queue.rs index a27cd3d16465..eb8128c8ede1 100644 --- a/rs/replicated_state/src/canister_state/system_state/task_queue.rs +++ b/rs/replicated_state/src/canister_state/system_state/task_queue.rs @@ -170,7 +170,8 @@ impl TaskQueue { } /// Removes the aborted install code task, if any, returning the execution cycles - /// that were prepaid for it. The caller is responsible for settling them. + /// that were prepaid for it. The caller is responsible for settling them, as the + /// execution they paid for will not happen. pub fn remove_aborted_install_code_task(&mut self) -> Option> { match &self.paused_or_aborted_task { Some(ExecutionTask::AbortedInstallCode { From 233937988e4f7b4f293adfb89af88d96dd0d6f51 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 3 Sep 2026 12:50:32 +0000 Subject: [PATCH 5/6] refactor: Export the monotonic consumed cycles total as the existing gauge Address review feedback: * Drop the `replicated_state_consumed_cycles_since_replica_started_as_counter` Prometheus `Counter` and export the monotonic total under the existing `replicated_state_consumed_cycles_since_replica_started` gauge (and its `replicated_state_consumed_cycles` alias) instead. A `Counter` is wrong here: the value is recomputed from the state on every observation, so a replica resuming from a checkpoint re-exports a lower value and `reset()` + `inc_by()` would report the difference as a jump by the whole checkpoint value. And because the monotonic total tracks the certified one closely, exporting it under the old name saves rewriting rules and dashboards. * Inline `SystemState::migrated_consumed_cycles_as_counter` into its only caller, `migrate_consumed_cycles_to_counter`. * Trim the comment on the refund in `drop_in_progress_management_calls_after_split` down to a single sentence in the existing first paragraph. Co-Authored-By: Claude Opus 5 (1M context) --- rs/execution_environment/src/scheduler.rs | 2 +- .../src/scheduler/tests/metrics.rs | 24 ++++---- .../src/canister_state/system_state.rs | 59 ++++++------------- .../src/metadata_state/tests.rs | 22 +++---- rs/replicated_state/src/metrics.rs | 59 +++++++------------ 5 files changed, 63 insertions(+), 103 deletions(-) diff --git a/rs/execution_environment/src/scheduler.rs b/rs/execution_environment/src/scheduler.rs index 3f4b31c5d157..8bb717128499 100644 --- a/rs/execution_environment/src/scheduler.rs +++ b/rs/execution_environment/src/scheduler.rs @@ -2173,7 +2173,7 @@ pub fn abort_all_paused_executions( /// Backfills the monotonic `CanisterMetrics::consumed_cycles_as_counter` of every /// canister from its `consumed_cycles` gauge, which predates it and thus holds the -/// full history. See `SystemState::migrated_consumed_cycles_as_counter`. +/// full history. See `SystemState::migrate_consumed_cycles_to_counter`. /// /// Must only be called with no paused executions left (i.e. on a checkpoint round, /// after `abort_all_paused_executions`); canisters that still have one are skipped, diff --git a/rs/execution_environment/src/scheduler/tests/metrics.rs b/rs/execution_environment/src/scheduler/tests/metrics.rs index 04e7464ac164..1ec98e02083d 100644 --- a/rs/execution_environment/src/scheduler/tests/metrics.rs +++ b/rs/execution_environment/src/scheduler/tests/metrics.rs @@ -24,7 +24,7 @@ use ic_replicated_state::metadata_state::testing::{NetworkTopologyTesting, Syste use ic_replicated_state::metrics::ReplicatedStateMetrics; use ic_replicated_state::testing::{ReplicatedStateTesting, SystemStateTesting}; use ic_test_utilities_metrics::{ - HistogramStats, MetricVec, fetch_counter, fetch_counter_vec, fetch_gauge, fetch_gauge_vec, + HistogramStats, MetricVec, fetch_counter_vec, fetch_gauge, fetch_gauge_vec, fetch_histogram_stats, fetch_histogram_vec_stats, fetch_int_gauge, fetch_int_gauge_vec, metric_vec, nonzero_values, }; @@ -1569,11 +1569,11 @@ fn consumed_cycles_for_instructions_are_updated_from_valid_canisters() { } } -/// The exported total counter is the subnet-level aggregate (already monotonic) -/// plus the canisters' `consumed_cycles_as_counter`; i.e. the gauge it mirrors, net -/// of the outstanding prepayments. +/// The exported total is the subnet-level aggregate (already monotonic) plus the +/// canisters' `consumed_cycles_as_counter`; i.e. the certified +/// `consumed_cycles_total_including_canisters`, net of the outstanding prepayments. #[test] -fn consumed_cycles_total_as_counter_is_exported() { +fn consumed_cycles_total_is_exported_net_of_outstanding_prepayments() { let mut test = SchedulerTestBuilder::new().build(); let canister = test.create_canister(); @@ -1583,17 +1583,17 @@ fn consumed_cycles_total_as_counter_is_exported() { observe_state_metrics(&mut test, 0); - let gauge = fetch_gauge( + let certified_total = test + .state() + .metadata + .subnet_metrics + .consumed_cycles_total_including_canisters(); + let exported = fetch_gauge( test.metrics_registry(), "replicated_state_consumed_cycles_since_replica_started", ) .unwrap(); - let counter = fetch_counter( - test.metrics_registry(), - "replicated_state_consumed_cycles_since_replica_started_as_counter", - ) - .unwrap(); - assert_eq!(gauge - outstanding.get() as f64, counter); + assert_eq!((certified_total - outstanding).get() as f64, exported); } #[test] diff --git a/rs/replicated_state/src/canister_state/system_state.rs b/rs/replicated_state/src/canister_state/system_state.rs index 42646754358d..d37143ab98a7 100644 --- a/rs/replicated_state/src/canister_state/system_state.rs +++ b/rs/replicated_state/src/canister_state/system_state.rs @@ -1669,24 +1669,11 @@ impl SystemState { /// executing on one subnet, but for which a response may only be produced by /// another subnet. pub fn drop_in_progress_management_calls_after_split(&mut self) { - // Remove aborted install code task. + // Remove aborted install code task and fully refund the prepaid execution + // cycles. // // Note that this cannot be a paused install code task, because we abort all // paused tasks before triggering the split. - // - // Refund the execution cycles prepaid for the dropped `install_code` in full. - // The canister has nothing to show for them: an aborted execution discards the - // slices it had already run and starts over when retried, and this one is - // never retried -- subnet A' rejects the corresponding call, unwinding the - // operation there as well. - // - // This also keeps the consumed cycles metrics consistent. Dropping the task - // without refunding would leave the prepayment in the gauges with nothing left - // in the state to account for it, breaking the invariant on - // `Self::outstanding_prepayments`; and the monotonic counters, which only - // account for a prepayment once its refund is known, would never see it at - // all. A full refund lowers the gauges by the prepayment and adds nothing to - // the counters, which is exactly right: nothing was consumed. if let Some(prepaid_execution_cycles) = self.task_queue.remove_aborted_install_code_task() { self.refund_cycles(prepaid_execution_cycles, prepaid_execution_cycles); } @@ -2381,9 +2368,9 @@ impl SystemState { Some(outstanding) } - /// The value that [`CanisterMetrics::consumed_cycles_as_counter`] must have, - /// derived from the [`CanisterMetrics::consumed_cycles`] gauge, which predates - /// it and thus holds the full history. + /// Backfills [`CanisterMetrics::consumed_cycles_as_counter`] from the + /// [`CanisterMetrics::consumed_cycles`] gauge, which predates it and thus holds + /// the full history. /// /// This derivation is exact, thanks to the invariant documented on /// [`Self::outstanding_prepayments`]: the gauge differs from the counter by @@ -2393,31 +2380,21 @@ impl SystemState { /// counter this way is idempotent, so it is safe to redo it in every round and /// after a downgrade has dropped the counter. /// - /// Returns `None` if the canister has a paused execution whose prepayment is not - /// part of the replicated state (see [`Self::outstanding_prepayments`]). - pub fn migrated_consumed_cycles_as_counter(&self) -> Option { - let outstanding = self.outstanding_prepayments()?; + /// Returns `false` (leaving the counter untouched) if the canister has a paused + /// execution whose prepayment is not part of the replicated state (see + /// [`Self::outstanding_prepayments`]); the caller is expected to retry once + /// paused executions have been aborted. + pub fn migrate_consumed_cycles_to_counter(&mut self) -> bool { + let Some(outstanding) = self.outstanding_prepayments() else { + return false; + }; // `max` rather than a plain assignment: the counter must never go down, not // even if a saturating subtraction somewhere made the gauge lag behind it. - Some( - self.canister_metrics - .consumed_cycles_as_counter - .max(self.canister_metrics.consumed_cycles - outstanding), - ) - } - - /// Backfills [`CanisterMetrics::consumed_cycles_as_counter`] with - /// [`Self::migrated_consumed_cycles_as_counter`]. Returns `false` (leaving the - /// counter untouched) if the latter cannot be derived; the caller is expected to - /// retry once paused executions have been aborted. - pub fn migrate_consumed_cycles_to_counter(&mut self) -> bool { - match self.migrated_consumed_cycles_as_counter() { - Some(counter) => { - self.canister_metrics.consumed_cycles_as_counter = counter; - true - } - None => false, - } + self.canister_metrics.consumed_cycles_as_counter = self + .canister_metrics + .consumed_cycles_as_counter + .max(self.canister_metrics.consumed_cycles - outstanding); + true } /// Clears all canister changes and their memory usage, diff --git a/rs/replicated_state/src/metadata_state/tests.rs b/rs/replicated_state/src/metadata_state/tests.rs index b7db8f18bed1..94fcf2e999d0 100644 --- a/rs/replicated_state/src/metadata_state/tests.rs +++ b/rs/replicated_state/src/metadata_state/tests.rs @@ -2785,13 +2785,14 @@ fn consumed_cycles_total_calculates_the_right_amount() { } /// The `replicated_state_consumed_cycles_since_replica_started` gauge is set in -/// `ReplicatedStateMetrics::observe` from -/// [`SubnetMetrics::consumed_cycles_total_including_canisters`]. This test -/// exercises every subnet-level use case that contributes to the total, so that -/// omitting any of them (as the `SchnorrOutcalls`/`VetKd`/`DroppedMessages` use -/// cases once were) would change the reported value and fail the assertion, plus -/// the canisters' part of the total. Distinct powers of two are used so that a -/// missing contribution is always detectable in the total. +/// `ReplicatedStateMetrics::observe` to [`SubnetMetrics::consumed_cycles_total`] +/// plus the canisters' monotonic consumption. This test exercises every +/// subnet-level use case that contributes to the total, so that omitting any of +/// them (as the `SchnorrOutcalls`/`VetKd`/`DroppedMessages` use cases once were) +/// would change the reported value and fail the assertion. Distinct powers of two +/// are used so that a missing contribution is always detectable in the total. (The +/// canisters' part is covered by the scheduler test +/// `consumed_cycles_total_is_exported_net_of_outstanding_prepayments`.) #[test] fn consumed_cycles_gauge_accounts_for_all_subnet_level_use_cases() { // The three use cases with a dedicated scalar field are also mirrored in the @@ -2824,14 +2825,13 @@ fn consumed_cycles_gauge_accounts_for_all_subnet_level_use_cases() { consumed_cycles_by_use_case.insert(use_case, NominalCycles::new(1024)); } - let mut subnet_metrics = SubnetMetrics { + let subnet_metrics = SubnetMetrics { consumed_cycles_by_deleted_canisters: NominalCycles::new(1), consumed_cycles_ecdsa_outcalls: NominalCycles::new(2), consumed_cycles_http_outcalls: NominalCycles::new(4), consumed_cycles_by_use_case, ..Default::default() }; - subnet_metrics.refresh_consumed_cycles(NominalCycles::new(64)); let mut state = ReplicatedState::new(subnet_test_id(1), SubnetType::Application); state.metadata.subnet_metrics = subnet_metrics; @@ -2846,7 +2846,7 @@ fn consumed_cycles_gauge_accounts_for_all_subnet_level_use_cases() { ); // Deleted canisters (1) + ECDSA (2) + HTTP (4) + Schnorr (8) + VetKd (16) - // + dropped messages (32) + the canisters' part (64) = 127. The + // + dropped messages (32) = 63; the state holds no canisters. The // canister-level use cases inserted into the map above (each worth 1024) // must not appear in the total. let gauge = fetch_gauge( @@ -2854,7 +2854,7 @@ fn consumed_cycles_gauge_accounts_for_all_subnet_level_use_cases() { "replicated_state_consumed_cycles_since_replica_started", ) .unwrap(); - assert_eq!(gauge, 127.0); + assert_eq!(gauge, 63.0); } #[test] diff --git a/rs/replicated_state/src/metrics.rs b/rs/replicated_state/src/metrics.rs index 602a9cf6915f..c79b26350930 100644 --- a/rs/replicated_state/src/metrics.rs +++ b/rs/replicated_state/src/metrics.rs @@ -14,8 +14,7 @@ use ic_types::{ }; use ic_types_cycles::{Cycles, CyclesUseCase, NominalCycles}; use prometheus::{ - Counter, CounterVec, Gauge, GaugeVec, Histogram, HistogramVec, IntCounter, IntGauge, - IntGaugeVec, + CounterVec, Gauge, GaugeVec, Histogram, HistogramVec, IntCounter, IntGauge, IntGaugeVec, }; use std::collections::BTreeMap; use std::time::Duration; @@ -56,7 +55,6 @@ pub struct ReplicatedStateMetrics { registered_canisters: IntGaugeVec, available_canister_ids: IntGauge, consumed_cycles: Gauge, - consumed_cycles_as_counter: Counter, consumed_cycles_by_use_case: GaugeVec, consumed_cycles_by_use_case_as_counters: CounterVec, input_queue_messages: IntGaugeVec, @@ -166,16 +164,9 @@ impl ReplicatedStateMetrics { ), consumed_cycles: metrics_registry.gauge( "replicated_state_consumed_cycles_since_replica_started", - "Number of cycles consumed", - ), - consumed_cycles_as_counter: metrics_registry.register( - Counter::new( - "replicated_state_consumed_cycles_since_replica_started_as_counter", - "Number of cycles consumed, as a counter. Unlike its gauge \ - counterpart, this is only increased, by the actually consumed \ - amount once the refund of a prepayment is known.", - ) - .unwrap(), + "Number of cycles consumed. Monotonic, except across replica \ + restarts and subnet splits: a prepayment is only accounted for \ + once the refund it produces is known.", ), consumed_cycles_by_use_case: metrics_registry.gauge_vec( "replicated_state_consumed_cycles_from_replica_start", @@ -586,35 +577,27 @@ impl ReplicatedStateMetrics { .get_consumed_cycles_by_use_case(), ); - // Read from the shared definition rather than re-folding, so the gauge cannot - // drift from the certified state tree. The per-use-case breakdowns below do - // still fold over the canisters, as no aggregate holds them. + // The monotonic counterpart of the certified + // `SubnetMetrics::consumed_cycles_total_including_canisters()`, from which it + // differs by exactly the prepayments whose refund is not known yet: the + // subnet-level aggregate (monotonic already, as subnet-level use cases are + // never refunded and a deleted canister's consumption is moved into + // `consumed_cycles_by_deleted_canisters`) plus the canisters' monotonic + // counters. Exported in place of the certified total, which it tracks + // closely, so that existing rules and dashboards need no change. + // + // Still a gauge, not a Prometheus `Counter`: the value is recomputed from the + // state, so a replica resuming from a checkpoint re-exports a lower value + // than it had reached before; and an online subnet split hands the migrating + // canisters' consumption over to the other subnet (retaining it here would + // count it on both). Both need the same high water mark treatment as the + // non-monotonic value did. self.consumed_cycles.set( - state - .metadata - .subnet_metrics - .consumed_cycles_total_including_canisters() + (state.metadata.subnet_metrics.consumed_cycles_total() + + consumed_cycles_by_canisters_as_counter) .get() as f64, ); - // The subnet-level aggregate is monotonic already (subnet-level use cases are - // only ever charged, never refunded, and a deleted canister's consumption is - // moved into `consumed_cycles_by_deleted_canisters`), so it can be added to - // the canisters' counters as-is. - // - // The one thing that can lower this total is an online subnet split, which - // hands the canisters migrating to the other subnet -- and with them their - // consumption -- over to that subnet. Consumers see a counter reset, which - // they handle. Retaining the migrated canisters' consumption here instead - // would count it on both subnets, and this total deliberately mirrors the - // certified `consumed_cycles_total_including_canisters`, which drops for the - // same reason. - let consumed_cycles_as_counter = state.metadata.subnet_metrics.consumed_cycles_total() - + consumed_cycles_by_canisters_as_counter; - self.consumed_cycles_as_counter.reset(); - self.consumed_cycles_as_counter - .inc_by(consumed_cycles_as_counter.get() as f64); - self.observe_consumed_cycles_by_use_case(&consumed_cycles_total_by_use_case); self.observe_consumed_cycles_by_use_case_as_counters( &consumed_cycles_total_by_use_case_as_counters, From a97772f1323e999de984916fbd1fb7e648a32d13 Mon Sep 17 00:00:00 2001 From: Martin Raszyk Date: Thu, 3 Sep 2026 13:14:23 +0000 Subject: [PATCH 6/6] docs: Update the docs stale after exporting the monotonic total `SubnetMetrics::consumed_cycles_total_including_canisters` no longer feeds the `replicated_state_consumed_cycles_since_replica_started` gauge, which now deliberately differs from it by the outstanding prepayments; and the scheduler test helper refreshes it for the assertions, not for `observe`. Co-Authored-By: Claude Opus 5 (1M context) --- .../src/scheduler/tests/metrics.rs | 9 +++++---- rs/replicated_state/src/metadata_state.rs | 10 ++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/rs/execution_environment/src/scheduler/tests/metrics.rs b/rs/execution_environment/src/scheduler/tests/metrics.rs index 1ec98e02083d..d1f39ff14785 100644 --- a/rs/execution_environment/src/scheduler/tests/metrics.rs +++ b/rs/execution_environment/src/scheduler/tests/metrics.rs @@ -45,10 +45,11 @@ use ic_types_test_utils::ids::{canister_test_id, message_test_id, subnet_test_id use more_asserts::assert_ge; use std::time::Duration; -/// Observes the state metrics at `height`, having first refreshed the derived -/// consumed-cycles total that the `replicated_state_consumed_cycles_since_replica_started` -/// gauge reads. Production refreshes it on every `commit_and_certify`, which this -/// harness never does. +/// Observes the state metrics at `height`, having first refreshed +/// `SubnetMetrics::consumed_cycles_total_including_canisters`, the derived +/// certified total that the assertions below compare the exported gauge against. +/// Production refreshes it on every `commit_and_certify`, which this harness never +/// does. fn observe_state_metrics(test: &mut SchedulerTest, height: u64) { test.state_mut().refresh_consumed_cycles(); test.state_metrics().observe( diff --git a/rs/replicated_state/src/metadata_state.rs b/rs/replicated_state/src/metadata_state.rs index 2f704f860f84..475210dc8b0c 100644 --- a/rs/replicated_state/src/metadata_state.rs +++ b/rs/replicated_state/src/metadata_state.rs @@ -647,10 +647,12 @@ impl SubnetMetrics { /// `CanisterMetrics::consumed_cycles()` over the canisters that currently /// exist, as of the end of the last committed round. /// - /// Every consumer of the full total reads it here -- the certified state tree at - /// `/subnet//metrics` (from certification version `V29`) and the - /// `replicated_state_consumed_cycles_since_replica_started` gauge -- so they - /// cannot drift apart. + /// This is the certified total: the state tree at `/subnet//metrics` + /// (from certification version `V29`) reads it here. The + /// `replicated_state_consumed_cycles_since_replica_started` gauge deliberately + /// exports the monotonic counterpart instead -- this same total, net of the + /// prepayments whose refund is not known yet -- see + /// `ReplicatedStateMetrics::observe`. pub fn consumed_cycles_total_including_canisters(&self) -> NominalCycles { self.consumed_cycles_total_including_canisters }