Skip to content

feat: Do not route canister messages to a cooling down subnet #11129

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
mraszyk wants to merge 6 commits into
base: master
Choose a base branch
from
Open

feat: Do not route canister messages to a cooling down subnet #11129

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
771eaff
feat(messaging): Do not route canister messages to a cooling down subnet
mraszyk Aug 13, 2026
27c91d3
docs: Trim the cooling down comments
mraszyk Aug 13, 2026
2bf7eba
refactor(messaging): Count cooling down skipped queues in a separate …
mraszyk Aug 14, 2026
f32024a
Update rs/replicated_state/src/metadata_state.rs
mraszyk Aug 14, 2026
e2b8160
test(messaging): Scope the cooling down tests to a module
mraszyk Aug 14, 2026
7bd6d6e
docs(messaging): Explain the explicit callback ID
mraszyk Aug 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions rs/messaging/src/routing/stream_builder.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use ic_types::messages::{
MAX_INTER_CANISTER_PAYLOAD_IN_BYTES, MAX_REJECT_MESSAGE_LEN_BYTES, NO_DEADLINE, Payload,
RejectContext, Request, RequestOrResponse, Response, StreamMessage,
};
use ic_types::{CountBytes, SubnetId};
use ic_types::{CanisterId, CountBytes, SubnetId};
use ic_types_cycles::{CompoundCycles, Cycles};
#[cfg(test)]
use mockall::automock;
Expand Down Expand Up @@ -42,6 +42,9 @@ struct StreamBuilderMetrics {
pub routed_payload_sizes: Histogram,
/// Misrouted messages currently in streams, by remote subnet.
pub stream_misrouted_messages: IntGaugeVec,
/// Canister output queues skipped because their destination subnet was
/// cooling down.
pub cooling_down_skipped_queues: IntCounter,
/// Critical error for payloads above the maximum supported size.
pub critical_error_payload_too_large: IntCounter,
/// Critical error for responses dropped due to destination not found.
Expand All @@ -63,6 +66,7 @@ const METRIC_SIGNALS_END: &str = "mr_signals_end";
const METRIC_ROUTED_MESSAGES: &str = "mr_routed_message_count";
const METRIC_ROUTED_PAYLOAD_SIZES: &str = "mr_routed_payload_size_bytes";
const METRIC_STREAM_MISROUTED_MESSAGES: &str = "mr_stream_misrouted_messages";
const METRIC_COOLING_DOWN_SKIPPED_QUEUES: &str = "mr_cooling_down_skipped_queues";

const LABEL_TYPE: &str = "type";
const LABEL_STATUS: &str = "status";
Expand Down Expand Up @@ -126,6 +130,12 @@ impl StreamBuilderMetrics {
"Count of misrouted messages in streams, by remote subnet. Only populated for subnets currently involved in a canister migration.",
&[LABEL_REMOTE],
);
let cooling_down_skipped_queues = metrics_registry.int_counter(
METRIC_COOLING_DOWN_SKIPPED_QUEUES,
"Canister output queues skipped because their destination subnet was cooling down. \
Counted once per queue per round, so the same queue is counted repeatedly for as \
long as the destination subnet keeps cooling down.",
);
let critical_error_payload_too_large =
metrics_registry.error_counter(CRITICAL_ERROR_PAYLOAD_TOO_LARGE);
let critical_error_response_destination_not_found =
Expand Down Expand Up @@ -167,6 +177,7 @@ impl StreamBuilderMetrics {
routed_messages,
routed_payload_sizes,
stream_misrouted_messages,
cooling_down_skipped_queues,
critical_error_payload_too_large,
critical_error_response_destination_not_found,
critical_error_induct_response_failed,
Expand Down Expand Up @@ -442,6 +453,10 @@ impl StreamBuilderImpl {
let refund_limit = self.max_stream_messages / 2;
self.route_refunds(&mut state, refund_limit, &network_topology, &mut streams);

// No canister can have the subnet's own principal as its canister ID, so this
// identifies the messages taken from the subnet's own output queues.
let own_subnet_as_canister_id = CanisterId::from(self.subnet_id);

let mut requests_to_reject = Vec::new();
let mut oversized_requests = Vec::new();
let mut engine_requests_to_reject: Vec<Arc<Request>> = Vec::new();
Expand All @@ -458,11 +473,24 @@ impl StreamBuilderImpl {
// Cheap to clone, `RequestOrResponse` wraps `Arcs`.
let msg = msg.clone();

let is_from_subnet_queues = msg.sender() == own_subnet_as_canister_id;

@alin-at-dfinity alin-at-dfinity Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_subnet_response?

It might also make the intent clearer: we must still route/deliver subnet responses, just not other messages.

@mraszyk mraszyk Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is_subnet_response?

That would be correct because subnet output queues can only contain responses. But I planned to leave subnet output queues for a follow-up PR.

we must still route/deliver subnet responses, just not other messages.

I now put this into the design doc: messages in subnet output queues (responses as the mgmt canister never makes requests itself) on subnets that are not “cooling down” are not routed to streams to subnets that are “cooling down” (i.e., they stay in the subnet output queues)—this is enough because the (source) subnet that is “cooling down” is not being routed any new input subnet messages and thus it stops “bombarding” the target subnet which is also “cooling down” by its subnet output queue messages eventually;

I.e., we should still not route responses from subnets that are not cooling down. Otherwise, canisters on the subnet that is cooling down could keep calling the mgmt canister on a remote subnet that is not cooling down and if those responses are not held back, then the stream to the cooling down subnet never fully empties.

@alin-at-dfinity alin-at-dfinity Aug 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, my bad. I totally forgot that this was only an issue for the cooling down subnet itself and its loopback stream.

But then what's with the if !is_from_subnet_queues && is_cooling_down(&dst_subnet_id) below? Why are we letting messages from subnet queues through? Or is this change intended to be strictly about "messages from canister output queues", with "messages from subnet output queues" intended to be handled in the follow-up PR?

If so, I suppose I found it somewhat confusing that you chose to explicitly deal with subnet queues in this PR. If you were going to split it into 2 PRs, I would have found it a lot more natural to ignore subnet queues in this PR altogether. I.e. simply say if is_cooling_down(&dst_subnet_id) { exclude_queue() }. Then, in the follow-up refine it to "except for routing subnet responses into the loopback stream"

@mraszyk mraszyk Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or is this change intended to be strictly about "messages from canister output queues", with "messages from subnet output queues" intended to be handled in the follow-up PR?

Exactly, this PR should not change behavior for subnet output queues at all.

this was only an issue for the cooling down subnet itself and its loopback stream

not sure what you mean by this; if you refer to the case when we need to route subnet responses, then we need to do so unless source is not cooling down and target is cooling down, i.e., whenever source is cooling down or target is not cooling down


match network_topology.route(msg.receiver().get()) {
// Destination subnet found.
Some(dst_subnet_id) => {
let dst_stream_entry = streams.entry(dst_subnet_id);
let is_loopback_stream = self.subnet_id == dst_subnet_id;

// A cooling down destination subnet must not be sent any messages
// from canister output queues. Retain the message (along with
// everything behind it in the same queue) until the destination stops
// cooling down, rather than rejecting or dropping it.
if !is_from_subnet_queues && network_topology.is_cooling_down(&dst_subnet_id) {
Comment thread
alin-at-dfinity marked this conversation as resolved.
self.metrics.cooling_down_skipped_queues.inc();
output_iter.exclude_queue();
continue;
}

let dst_stream_entry = streams.entry(dst_subnet_id);
if !is_loopback_stream
&& is_at_limit(
&dst_stream_entry,
Expand Down
Loading
Loading