Skip to content

Commit

Permalink
feat(MR): [MR-638] Include best-effort calls in xnet_compatibility
Browse files Browse the repository at this point in the history
…test

Make both guaranteed response and best-effort calls in the subnet upgrade-downgrade `xnet_compatibility` test.
  • Loading branch information
alin-at-dfinity committed Jan 23, 2025
1 parent 87dc81c commit 2074a50
Show file tree
Hide file tree
Showing 9 changed files with 46 additions and 26 deletions.
4 changes: 2 additions & 2 deletions rs/messaging/tests/queue_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ use ic_types::{
Cycles,
};
use maplit::btreemap;
use std::collections::BTreeSet;
use std::sync::Arc;
use std::{collections::BTreeSet, vec};
use xnet_test::{Metrics, StartArgs};

const MAX_TICKS: u64 = 100;
Expand Down Expand Up @@ -103,7 +103,7 @@ impl SubnetPairProxy {
network_topology,
canister_to_subnet_rate,
request_payload_size_bytes: payload_size_bytes,
request_timeout_seconds: 0,
call_timeouts_seconds: vec![None],
response_payload_size_bytes: payload_size_bytes,
})
}
Expand Down
2 changes: 1 addition & 1 deletion rs/rust_canisters/xnet_test/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ pub struct StartArgs {
pub network_topology: NetworkTopology,
pub canister_to_subnet_rate: u64,
pub request_payload_size_bytes: u64,
pub request_timeout_seconds: u32,
pub call_timeouts_seconds: Vec<Option<u32>>,
pub response_payload_size_bytes: u64,
}

Expand Down
26 changes: 19 additions & 7 deletions rs/rust_canisters/xnet_test/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,8 @@ thread_local! {
/// Pad requests to this size (in bytes) if smaller.
static REQUEST_PAYLOAD_SIZE: RefCell<u64> = const { RefCell::new(1024) };

/// Timeout to set on requests. Zero for guaranteed response calls.
static REQUEST_TIMEOUT_SECONDS: RefCell<u32> = const { RefCell::new(0) };
/// Timeouts to set on calls. `None` for guaranteed response calls.
static CALL_TIMEOUTS_SECONDS: RefCell<Vec<Option<u32>>> = const { RefCell::new(Vec::new()) };

/// Pad responses to this size (in bytes) if smaller.
static RESPONSE_PAYLOAD_SIZE: RefCell<u64> = const { RefCell::new(1024) };
Expand Down Expand Up @@ -148,12 +148,19 @@ fn log(message: &str) {
/// requests to other canisters.
#[update]
fn start(start_args: StartArgs) -> String {
// Default to guaranteed response calls only.
let call_timeouts_seconds = if start_args.call_timeouts_seconds.is_empty() {
vec![None]
} else {
start_args.call_timeouts_seconds
};

NETWORK_TOPOLOGY.with(move |canisters| {
*canisters.borrow_mut() = start_args.network_topology;
});
PER_SUBNET_RATE.with(|r| *r.borrow_mut() = start_args.canister_to_subnet_rate);
REQUEST_PAYLOAD_SIZE.with(|r| *r.borrow_mut() = start_args.request_payload_size_bytes);
REQUEST_TIMEOUT_SECONDS.with(|r| *r.borrow_mut() = start_args.request_timeout_seconds);
CALL_TIMEOUTS_SECONDS.with(|r| *r.borrow_mut() = call_timeouts_seconds);
RESPONSE_PAYLOAD_SIZE.with(|r| *r.borrow_mut() = start_args.response_payload_size_bytes);

RUNNING.with(|r| *r.borrow_mut() = true);
Expand All @@ -176,7 +183,7 @@ async fn fanout() {

let network_topology =
NETWORK_TOPOLOGY.with(|network_topology| network_topology.borrow().clone());
let timeout_seconds = REQUEST_TIMEOUT_SECONDS.with(|p| *p.borrow());
let timeouts_seconds = CALL_TIMEOUTS_SECONDS.with(|p| p.borrow().clone());
let payload_size = REQUEST_PAYLOAD_SIZE.with(|p| *p.borrow()) as usize;

let mut futures = vec![];
Expand All @@ -202,11 +209,16 @@ async fn fanout() {
padding: vec![0; payload_size.saturating_sub(16)],
};

// Cycle over the timeouts.
let timeout_seconds = timeouts_seconds
.get(seq_no as usize % timeouts_seconds.len())
.unwrap();

let call = Call::new(canister, "handle_request").with_arg(payload);
let call = if timeout_seconds == 0 {
call.with_guaranteed_response()
let call = if let Some(timeout_seconds) = timeout_seconds {
call.change_timeout(*timeout_seconds)
} else {
call.change_timeout(timeout_seconds)
call.with_guaranteed_response()
};
futures.push(call.call::<Reply>());
METRICS.with(move |m| m.borrow_mut().calls_attempted += 1);
Expand Down
4 changes: 2 additions & 2 deletions rs/tests/message_routing/common/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use xnet_test::StartArgs;
pub async fn start_all_canisters(
canisters: &[Vec<Canister<'_>>],
request_payload_size_bytes: u64,
request_timeout_seconds: u32,
call_timeouts_seconds: &[Option<u32>],
response_payload_size_bytes: u64,
canister_to_subnet_rate: u64,
) {
Expand All @@ -36,7 +36,7 @@ pub async fn start_all_canisters(
network_topology: topology.clone(),
canister_to_subnet_rate,
request_payload_size_bytes,
request_timeout_seconds,
call_timeouts_seconds: call_timeouts_seconds.to_vec(),
response_payload_size_bytes,
};
futures.push(async move {
Expand Down
2 changes: 1 addition & 1 deletion rs/tests/message_routing/global_reboot_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,7 @@ pub fn start_all_canisters(
network_topology: topology.clone(),
canister_to_subnet_rate,
request_payload_size_bytes: payload_size_bytes,
request_timeout_seconds: 0,
call_timeouts_seconds: vec![None],
response_payload_size_bytes: payload_size_bytes,
};
let _: String = canister
Expand Down
12 changes: 7 additions & 5 deletions rs/tests/message_routing/xnet/slo_test_lib/xnet_slo_test_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use ic_system_test_driver::util::{block_on, runtime_from_url};
use slog::info;
use std::fmt::Display;
use std::time::Duration;
use std::vec;
use systest_message_routing_common::{install_canisters, parallel_async, start_all_canisters};
use xnet_test::Metrics;

Expand All @@ -59,7 +60,8 @@ pub struct Config {
nodes_per_subnet: usize,
runtime: Duration,
request_payload_size_bytes: u64,
request_timeout_seconds: u32,
/// Possible call timeouts. `None` for guaranteed response.
call_timeouts_seconds: Vec<Option<u32>>,
response_payload_size_bytes: u64,
send_rate_threshold: f64,
error_percentage_threshold: f64,
Expand Down Expand Up @@ -110,7 +112,7 @@ impl Config {
nodes_per_subnet,
runtime,
request_payload_size_bytes: PAYLOAD_SIZE_BYTES,
request_timeout_seconds: 0,
call_timeouts_seconds: vec![None],
response_payload_size_bytes: PAYLOAD_SIZE_BYTES,
send_rate_threshold,
error_percentage_threshold,
Expand All @@ -134,9 +136,9 @@ impl Config {
config
}

pub fn with_best_effort_response(self, timeout_seconds: u32) -> Self {
pub fn with_call_timeouts(self, timeouts_seconds: &[Option<u32>]) -> Self {
let mut config = self.clone();
config.request_timeout_seconds = timeout_seconds;
config.call_timeouts_seconds = timeouts_seconds.to_vec();
config
}

Expand Down Expand Up @@ -267,7 +269,7 @@ pub async fn deploy_and_start<'a, 'b>(
start_all_canisters(
&canisters,
config.request_payload_size_bytes,
config.request_timeout_seconds,
&config.call_timeouts_seconds,
config.response_payload_size_bytes,
config.canister_to_subnet_rate as u64,
)
Expand Down
9 changes: 7 additions & 2 deletions rs/tests/message_routing/xnet/xnet_compatibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ use std::time::Duration;

const PER_TASK_TIMEOUT: Duration = Duration::from_secs(10 * 60);
const OVERALL_TIMEOUT: Duration = Duration::from_secs(15 * 60);
const CALL_TIMEOUT_SECONDS: u32 = 300;

const DKG_INTERVAL: u64 = 9;
const NODES_PER_SUBNET: usize = 1;
Expand Down Expand Up @@ -146,7 +147,10 @@ pub async fn test_async(env: TestEnv) {
.map(|(_, _, node)| node)
.map(|node| runtime_from_url(node.get_public_url(), node.effective_canister_id()));

let xnet_config = xnet_slo_test_lib::Config::new(2, 1, Duration::from_secs(30), 10);
// Make both guaranteed response and best-effort calls.
let call_timeouts = [None, Some(CALL_TIMEOUT_SECONDS)];
let xnet_config = xnet_slo_test_lib::Config::new(2, 1, Duration::from_secs(30), 10)
.with_call_timeouts(&call_timeouts);
let long_xnet_config = xnet_slo_test_lib::Config::new_with_custom_thresholds(
2,
1,
Expand All @@ -163,7 +167,8 @@ pub async fn test_async(env: TestEnv) {
// with error thresholds.
75.0,
40,
);
)
.with_call_timeouts(&call_timeouts);

let mainnet_version = read_dependency_to_string("mainnet_nns_subnet_revision.txt").unwrap();

Expand Down
9 changes: 5 additions & 4 deletions rs/tests/message_routing/xnet/xnet_malicious_slices.rs
Original file line number Diff line number Diff line change
Expand Up @@ -153,10 +153,11 @@ pub async fn test_async(env: TestEnv, config: Config) {
// Start all canisters (via update `start` call).
info!(logger, "Calling start() on all canisters...");
start_all_canisters(
&canisters, 1024, // send messages with 1024 byte payloads
0, // guaranteed response calls
1024, // same response size
10, // each canister sends 10 RPS
&canisters,
1024, // send messages with 1024 byte payloads
&[None], // guaranteed response calls
1024, // same response size
10, // each canister sends 10 RPS
)
.await;
info!(logger, "Starting chatter: 10 messages/round * 1024 bytes",);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,14 +8,14 @@ const SUBNETS: usize = 3;
const NODES_PER_SUBNET: usize = 4;
const RUNTIME: Duration = Duration::from_secs(60);
const REQUEST_RATE: usize = 100;
const RESPONSE_TIMEOUT_SECONDS: u32 = 300;
const CALL_TIMEOUT_SECONDS: u32 = 300;

const PER_TASK_TIMEOUT: Duration = Duration::from_secs(15 * 60);
const OVERALL_TIMEOUT: Duration = Duration::from_secs(25 * 60);

fn main() -> Result<()> {
let config = Config::new(SUBNETS, NODES_PER_SUBNET, RUNTIME, REQUEST_RATE)
.with_best_effort_response(RESPONSE_TIMEOUT_SECONDS);
.with_call_timeouts(&[Some(CALL_TIMEOUT_SECONDS)]);
let test = config.clone().test();
SystemTestGroup::new()
.with_setup(config.build())
Expand Down

0 comments on commit 2074a50

Please sign in to comment.