Skip to content

Commit 99b7d3a

Browse files
mraszykclaude
andcommitted
Merge branch 'master' into subnet-metrics-endpoint
Master landed #11335, the upstream version of this branch's consumed-cycles refactor, which makes `SubnetMetrics::consumed_cycles_total_including_canisters` a private field behind a getter written only by `SubnetMetrics::refresh_consumed_cycles`. Every conflict is between that and this branch's own variant of the same change, so master's version wins throughout; the `subnet_metrics` endpoint and its tests now read the total through the getter. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2 parents f622601 + c2cebaf commit 99b7d3a

53 files changed

Lines changed: 965 additions & 443 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

ic-os/components/setupos/check-ntp.sh

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,13 @@ function set_hwclock_utc() {
3939

4040
main() {
4141
log_start "$(basename $0)"
42-
check_ntp
42+
if check_cmdline_var ic.setupos.run_checks; then
43+
check_ntp
44+
else
45+
# NTP is not available in test environments; test VMs take their clock
46+
# from the hypervisor.
47+
echo "* NTP synchronization check skipped by request via kernel command line"
48+
fi
4349
set_hwclock_utc
4450
log_end "$(basename $0)"
4551
}

rs/canister_sandbox/src/replica_controller/sandboxed_execution_controller.rs

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ use ic_metrics::buckets::{decimal_buckets_with_zero, exponential_buckets};
2727
use ic_replicated_state::canister_state::execution_state::{
2828
SandboxMemory, SandboxMemoryHandle, SandboxMemoryOwner, WasmBinary, WasmExecutionMode,
2929
};
30-
use ic_replicated_state::metrics::instructions_buckets;
3130
use ic_replicated_state::{
3231
EmbedderCache, ExecutionState, ExportedFunctions, Memory, PageMap, ReplicatedState,
3332
page_map::allocated_pages_count,
@@ -157,7 +156,6 @@ struct SandboxedExecutionMetrics {
157156
mprotect_count: HistogramVec,
158157
copy_page_count: HistogramVec,
159158
sigsegv_handler_duration: HistogramVec,
160-
dmt_projected_message_cost: HistogramVec,
161159
}
162160

163161
impl SandboxedExecutionMetrics {
@@ -411,12 +409,6 @@ impl SandboxedExecutionMetrics {
411409
decimal_buckets_with_zero(-4, 1),
412410
&["api_type", "memory_type"],
413411
),
414-
dmt_projected_message_cost: metrics_registry.histogram_vec(
415-
"sandboxed_execution_dmt_projected_message_cost",
416-
"Cost of a message when the DMT charges for all page accesses.",
417-
instructions_buckets(), /* same as scheduler_instructions_consumed_per_message for comparison */
418-
&["api_type", "memory_type"],
419-
),
420412
}
421413
}
422414

@@ -502,10 +494,6 @@ impl SandboxedExecutionMetrics {
502494
.with_label_values(&[api_type_label, "stable"])
503495
.observe(instance_stats.stable_sigsegv_handler_duration.as_secs_f64());
504496

505-
self.dmt_projected_message_cost
506-
.with_label_values(&[api_type_label, "both"])
507-
.observe(instance_stats.dmt_projected_message_cost as f64);
508-
509497
self.allocated_pages.set(allocated_pages_count() as i64);
510498
}
511499
}
@@ -1709,7 +1697,7 @@ impl SandboxedExecutionController {
17091697
});
17101698
return WasmExecutionResult::Paused(slice, paused);
17111699
}
1712-
CompletionResult::Finished(mut exec_output) => {
1700+
CompletionResult::Finished(exec_output) => {
17131701
let execution_status = match exec_output.wasm.wasm_result.clone() {
17141702
Ok(Some(WasmResult::Reply(_))) => "Success",
17151703
Ok(Some(WasmResult::Reject(_))) => "Reject",
@@ -1721,19 +1709,6 @@ impl SandboxedExecutionController {
17211709
execution_status,
17221710
execution_state.wasm_execution_mode.as_str(),
17231711
);
1724-
// Temporary metric: How much will the message cost when we charge via DMT.
1725-
let instructions_used = message_instruction_limit
1726-
.saturating_sub(&exec_output.wasm.num_instructions_left);
1727-
let stable_writes = exec_output.wasm.instance_stats.stable_dirty_pages;
1728-
let stable_reads = exec_output.wasm.instance_stats.stable_accessed_pages;
1729-
let heap_writes = exec_output.wasm.instance_stats.wasm_dirty_pages;
1730-
let heap_reads = exec_output.wasm.instance_stats.wasm_accessed_pages - heap_writes;
1731-
// we currently charge 1000 for stable and heap writes (plus 3000 for copy overhead, which remains separate), and 0 for everything else.
1732-
let additional_cost =
1733-
(stable_writes + heap_writes) * 4000 + (stable_reads + heap_reads) * 5000;
1734-
exec_output.wasm.instance_stats.dmt_projected_message_cost =
1735-
instructions_used.get() as usize + additional_cost;
1736-
17371712
self.metrics
17381713
.observe_instance_stats(&exec_output.wasm.instance_stats, api_type_label);
17391714
exec_output

rs/canonical_state/src/encoding/tests/compatibility.rs

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -307,8 +307,9 @@ fn canonical_encoding_stream_header_v26() {
307307
/// Starting with `V29`, the `Instructions` (80B) and
308308
/// `RequestAndResponseTransmission` (20B) use case entries are no longer added
309309
/// on top of the deleted canisters scalar (fixing the double counting), while
310-
/// the cycles consumed by non-deleted canisters (50B, passed to
311-
/// `encode_subnet_metrics`) are added instead. Hence the expected value becomes
310+
/// the cycles consumed by non-deleted canisters (50B, folded into the stored
311+
/// aggregate by `SubnetMetrics::refresh_consumed_cycles`) are added instead.
312+
/// Hence the expected value becomes
312313
/// 0 (deleted) + 50B (HTTP) + 100B (ECDSA) + 50B (canisters) = 200B
313314
/// (`1B 0000002E90EDD000`).
314315
///
@@ -342,10 +343,7 @@ fn canonical_encoding_subnet_metrics() {
342343
metrics.threshold_signature_agreements =
343344
BTreeMap::from([(schnorr_key_id, 15), (ecdsa_key_id, 16)]);
344345

345-
// From `V29` on the reported total is this stored aggregate: the subnet-level
346-
// total plus the part consumed by the canisters that still exist.
347-
metrics.consumed_cycles_total_including_canisters =
348-
metrics.consumed_cycles_total() + NominalCycles::new(50_000_000_000);
346+
metrics.refresh_consumed_cycles(NominalCycles::new(50_000_000_000));
349347

350348
let expected = if certification_version >= CertificationVersion::V29 {
351349
"A4 00 05 01 1A 00 50 00 00 02 A2 00 1B 00 00 00 2E 90 ED D0 00 01 00 03 19 10 68"

rs/canonical_state/src/encoding/types.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -738,10 +738,11 @@ impl
738738
// by deleted canisters and does not account for non-deleted canisters.
739739
//
740740
// Starting with `V29`, the reported total is the stored
741-
// `SubnetMetrics::consumed_cycles_total_including_canisters`, which no longer
742-
// double counts deleted canisters and does account for the existing ones.
741+
// `SubnetMetrics::consumed_cycles_total_including_canisters`, which no
742+
// longer double counts deleted canisters and does account for the existing
743+
// ones.
743744
let consumed_cycles_total = if certification_version >= CertificationVersion::V29 {
744-
metrics.consumed_cycles_total_including_canisters
745+
metrics.consumed_cycles_total_including_canisters()
745746
} else {
746747
metrics.consumed_cycles_total_v28()
747748
};

rs/canonical_state/src/lazy_tree_conversion.rs

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1144,11 +1144,7 @@ fn subnets_as_tree<'a>(
11441144
subnet_id == &own_subnet_id,
11451145
"metrics",
11461146
// Starting with `V29`, the reported total also includes
1147-
// the cycles consumed by all non-deleted canisters, read
1148-
// from the stored
1149-
// `SubnetMetrics::consumed_cycles_total_including_canisters`
1150-
// (refreshed by
1151-
// `ReplicatedState::refresh_consumed_cycles`).
1147+
// the cycles consumed by all non-deleted canisters.
11521148
blob(move || encode_subnet_metrics(metrics, certification_version)),
11531149
)
11541150
.with_tree_if(

rs/canonical_state/src/traversal.rs

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1199,7 +1199,9 @@ mod tests {
11991199
#[test]
12001200
fn test_traverse_subnet_metrics_includes_canister_consumed_cycles_at_v29() {
12011201
use crate::encoding::encode_subnet_metrics;
1202-
use ic_types_cycles::{CompoundCycles, Instructions, NominalCycles};
1202+
use ic_types_cycles::{
1203+
CompoundCycles, CyclesUseCase, Instructions, NominalCycles, NominalCyclesTesting,
1204+
};
12031205

12041206
let own_subnet_id = subnet_test_id(1);
12051207
let mut state = ReplicatedState::new(own_subnet_id, SubnetType::Application);
@@ -1227,6 +1229,13 @@ mod tests {
12271229
);
12281230
});
12291231

1232+
// Non-zero subnet-level consumption, so that the reported total covers both
1233+
// parts: the subnet-level aggregate and the canisters' part below.
1234+
let subnet_metrics = &mut state.metadata.subnet_metrics;
1235+
subnet_metrics.observe_consumed_cycles_by_deleted_canisters(NominalCycles::new(1_000));
1236+
subnet_metrics
1237+
.observe_consumed_cycles_with_use_case(CyclesUseCase::VetKd, NominalCycles::new(4));
1238+
12301239
// Add a non-deleted canister that has consumed some cycles.
12311240
let mut canister_state = new_canister_state(
12321241
canister_test_id(2),
@@ -1252,19 +1261,18 @@ mod tests {
12521261
state
12531262
.metadata
12541263
.subnet_metrics
1255-
.consumed_cycles_total_including_canisters,
1264+
.consumed_cycles_total_including_canisters(),
12561265
NominalCycles::zero()
12571266
);
12581267

1259-
// The refresh publishes the fold into `SubnetMetrics`. This subnet has no
1260-
// subnet-level consumption, so the canisters' part is the whole total.
12611268
let subnet_level = state.metadata.subnet_metrics.consumed_cycles_total();
1269+
assert!(subnet_level > NominalCycles::zero());
12621270
state.refresh_consumed_cycles();
12631271
assert_eq!(
12641272
state
12651273
.metadata
12661274
.subnet_metrics
1267-
.consumed_cycles_total_including_canisters,
1275+
.consumed_cycles_total_including_canisters(),
12681276
subnet_level + consumed_by_canisters
12691277
);
12701278

@@ -1293,8 +1301,7 @@ mod tests {
12931301

12941302
// The canister's consumed cycles are included only starting with V29.
12951303
let mut metrics_without_canisters = state.metadata.subnet_metrics.clone();
1296-
metrics_without_canisters.consumed_cycles_total_including_canisters =
1297-
NominalCycles::zero();
1304+
metrics_without_canisters.refresh_consumed_cycles(NominalCycles::zero());
12981305
let without_canisters =
12991306
encode_subnet_metrics(&metrics_without_canisters, certification_version);
13001307
if certification_version >= CertificationVersion::V29 {

rs/config/src/embedders.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -222,11 +222,12 @@ pub struct Config {
222222
/// overridden at runtime by the registry's `maximum_state_delta`.
223223
pub default_subnet_heap_delta_capacity: NumBytes,
224224

225-
/// Dirty page overhead. The number of instructions to charge for each dirty
226-
/// page created by a write to stable memory. The default value should be
225+
/// The number of instructions to charge for every OS page of heap or stable
226+
/// memory that a message touches: once when the page is first accessed and
227+
/// once more when it is first written to. The default value should be
227228
/// replaced with the correct value at runtime when the hypervisor is
228229
/// created.
229-
pub dirty_page_overhead: NumInstructions,
230+
pub page_overhead: NumInstructions,
230231

231232
/// If this flag is enabled, then execution of a slice will produce a log
232233
/// entry with the number of executed instructions and the duration.
@@ -281,7 +282,7 @@ impl Config {
281282
max_sandbox_count: DEFAULT_MAX_SANDBOX_COUNT,
282283
max_sandbox_idle_time: DEFAULT_MAX_SANDBOX_IDLE_TIME,
283284
default_subnet_heap_delta_capacity: SUBNET_HEAP_DELTA_CAPACITY,
284-
dirty_page_overhead: NumInstructions::new(0),
285+
page_overhead: NumInstructions::new(0),
285286
trace_execution: FlagStatus::Disabled,
286287
max_dirty_pages_without_optimization: DEFAULT_MAX_DIRTY_PAGES_WITHOUT_OPTIMIZATION,
287288
dirty_page_copy_overhead: DIRTY_PAGE_COPY_OVERHEAD,

rs/config/src/subnet_config.rs

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,8 @@ pub const DEFAULT_REFERENCE_SUBNET_SIZE: usize = 13;
138138
/// Reference subnet size for SEV-enabled application subnets.
139139
pub const SEV_REFERENCE_SUBNET_SIZE: usize = 7;
140140

141-
/// Costs for each newly created dirty page in stable memory.
142-
pub const DEFAULT_DIRTY_PAGE_OVERHEAD: NumInstructions = NumInstructions::new(5_000);
141+
/// Cost of touching a single OS page of canister memory.
142+
pub const DEFAULT_PAGE_OVERHEAD: NumInstructions = NumInstructions::new(5_000);
143143

144144
/// Accumulated priority reset interval, rounds.
145145
///
@@ -276,8 +276,10 @@ pub struct SchedulerConfig {
276276
/// rounds until they are back under the allowed rate.
277277
pub install_code_rate_limit: NumInstructions,
278278

279-
/// Cost for each newly created dirty page in stable memory.
280-
pub dirty_page_overhead: NumInstructions,
279+
/// The number of instructions to charge for every OS page of heap or stable
280+
/// memory that a message touches: once when the page is first accessed and
281+
/// once more when it is first written to.
282+
pub page_overhead: NumInstructions,
281283

282284
/// Accumulated priority reset interval, rounds.
283285
pub accumulated_priority_reset_interval: ExecutionRound,
@@ -318,7 +320,7 @@ impl SchedulerConfig {
318320
MAX_MESSAGE_DURATION_BEFORE_WARN_IN_SECONDS,
319321
heap_delta_rate_limit: NumBytes::from(75 * 1024 * 1024),
320322
install_code_rate_limit: MAX_INSTRUCTIONS_PER_SLICE,
321-
dirty_page_overhead: DEFAULT_DIRTY_PAGE_OVERHEAD,
323+
page_overhead: DEFAULT_PAGE_OVERHEAD,
322324
accumulated_priority_reset_interval: ACCUMULATED_PRIORITY_RESET_INTERVAL,
323325
upload_wasm_chunk_instructions: DEFAULT_UPLOAD_CHUNK_INSTRUCTIONS,
324326
canister_snapshot_baseline_instructions:
@@ -366,7 +368,7 @@ impl SchedulerConfig {
366368
// This limit should be high enough (1000T) to effectively disable
367369
// rate-limiting for the system subnets.
368370
install_code_rate_limit: NumInstructions::from(1_000_000_000_000_000),
369-
dirty_page_overhead: DEFAULT_DIRTY_PAGE_OVERHEAD,
371+
page_overhead: DEFAULT_PAGE_OVERHEAD,
370372
accumulated_priority_reset_interval: ACCUMULATED_PRIORITY_RESET_INTERVAL,
371373
upload_wasm_chunk_instructions: NumInstructions::from(0),
372374
canister_snapshot_baseline_instructions: NumInstructions::from(0),

rs/consensus/idkg/src/signer.rs

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -573,10 +573,7 @@ impl ThresholdSignerImpl {
573573
let reconstruction_threshold = match inputs {
574574
ThresholdSigInputs::Ecdsa(inputs) => inputs.reconstruction_threshold().get() as usize,
575575
ThresholdSigInputs::Schnorr(inputs) => inputs.reconstruction_threshold().get() as usize,
576-
// VetKd's API does not expose the number of shares needed for reconstruction directly.
577-
// As this code path is an optimization, we conservatively assume that we do not have
578-
// enough shares if the inputs are for VetKd.
579-
// The worst thing that can happen is to validate a few extra shares.
576+
// Validate all VetKD shares, to allow for failures during share combination.
580577
ThresholdSigInputs::VetKd(_inputs) => return false,
581578
};
582579

rs/consensus/utils/src/chain_key.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -80,6 +80,7 @@ pub fn build_signature_inputs<'a>(
8080
debug_assert_eq!(context.derivation_path.len(), 1);
8181
const EMPTY_VEC_REF: &Vec<u8> = &vec![];
8282
let inputs = ThresholdSigInputs::VetKd(VetKdArgs {
83+
request_id,
8384
context: VetKdDerivationContextRef {
8485
caller: context.request.sender.get_ref(),
8586
context: context.derivation_path.first().unwrap_or(EMPTY_VEC_REF),

0 commit comments

Comments
 (0)