Skip to content

Commit 72f7edf

Browse files
authored
chore: drop critical error for unexpected infinite loops (#10623)
In every loop iteration, we do `output_iter.exclude_queue()` or `output_iter.next().unwrap()` so the loop indeed always terminates and not asserting that it does so requires additional error handling downstream (in a follow-up PR) whose complexity is not worth the benefit of this safe-guard.
1 parent caa8170 commit 72f7edf

2 files changed

Lines changed: 0 additions & 23 deletions

File tree

rs/messaging/src/routing/stream_builder.rs

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -42,8 +42,6 @@ struct StreamBuilderMetrics {
4242
pub routed_payload_sizes: Histogram,
4343
/// Misrouted messages currently in streams, by remote subnet.
4444
pub stream_misrouted_messages: IntGaugeVec,
45-
/// Critical error counter for detected infinite loops while routing.
46-
pub critical_error_infinite_loops: IntCounter,
4745
/// Critical error for payloads above the maximum supported size.
4846
pub critical_error_payload_too_large: IntCounter,
4947
/// Critical error for responses dropped due to destination not found.
@@ -78,7 +76,6 @@ const LABEL_VALUE_STATUS_CANISTER_NOT_FOUND: &str = "canister_not_found";
7876
const LABEL_VALUE_STATUS_PAYLOAD_TOO_LARGE: &str = "payload_too_large";
7977
const LABEL_VALUE_STATUS_ENGINE_NOT_ALLOWED: &str = "engine_not_allowed";
8078

81-
const CRITICAL_ERROR_INFINITE_LOOP: &str = "mr_stream_builder_infinite_loop";
8279
const CRITICAL_ERROR_PAYLOAD_TOO_LARGE: &str = "mr_stream_builder_payload_too_large";
8380
const CRITICAL_ERROR_RESPONSE_DESTINATION_NOT_FOUND: &str =
8481
"mr_stream_builder_response_destination_not_found";
@@ -129,8 +126,6 @@ impl StreamBuilderMetrics {
129126
"Count of misrouted messages in streams, by remote subnet. Only populated for subnets currently involved in a canister migration.",
130127
&[LABEL_REMOTE],
131128
);
132-
let critical_error_infinite_loops =
133-
metrics_registry.error_counter(CRITICAL_ERROR_INFINITE_LOOP);
134129
let critical_error_payload_too_large =
135130
metrics_registry.error_counter(CRITICAL_ERROR_PAYLOAD_TOO_LARGE);
136131
let critical_error_response_destination_not_found =
@@ -172,7 +167,6 @@ impl StreamBuilderMetrics {
172167
routed_messages,
173168
routed_payload_sizes,
174169
stream_misrouted_messages,
175-
critical_error_infinite_loops,
176170
critical_error_payload_too_large,
177171
critical_error_response_destination_not_found,
178172
critical_error_induct_response_failed,
@@ -454,29 +448,13 @@ impl StreamBuilderImpl {
454448
let mut engine_response_dropped_cycles = Cycles::zero();
455449

456450
let mut output_iter = state.output_into_iter();
457-
let mut last_output_size = usize::MAX;
458451

459452
// Route all messages into the appropriate stream or generate reject Responses
460453
// when unable to (no route to canister). When a stream's byte size reaches or
461454
// exceeds `target_stream_size_bytes`, any matching queues are skipped.
462455
while let Some(msg) = output_iter.peek() {
463456
// Cheap to clone, `RequestOrResponse` wraps `Arcs`.
464457
let msg = msg.clone();
465-
// Safeguard to guarantee that iteration always terminates. Will always loop at
466-
// least once, if messages are available.
467-
let output_size = output_iter.size();
468-
debug_assert!(output_size < last_output_size);
469-
if output_size >= last_output_size {
470-
error!(
471-
self.log,
472-
"{}: Infinite loop detected in StreamBuilder::build_streams @{}.",
473-
CRITICAL_ERROR_INFINITE_LOOP,
474-
output_size
475-
);
476-
self.metrics.critical_error_infinite_loops.inc();
477-
break;
478-
}
479-
last_output_size = output_size;
480458

481459
match network_topology.route(msg.receiver().get()) {
482460
// Destination subnet found.

rs/messaging/src/routing/stream_builder/tests.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2063,7 +2063,6 @@ fn assert_eq_critical_errors(
20632063
) {
20642064
assert_eq!(
20652065
nonzero_values(metric_vec(&[
2066-
(&[("error", &CRITICAL_ERROR_INFINITE_LOOP)], 0),
20672066
(
20682067
&[("error", &CRITICAL_ERROR_PAYLOAD_TOO_LARGE)],
20692068
payload_too_large

0 commit comments

Comments
 (0)