Skip to content

Commit 35c7c10

Browse files
committed
integration: improve request timeout test
The test has long been marked with #[ignore] because it only worked on remote Scylla instances, as local ones were too fast to reliably trigger timeouts even with 1-ms timeouts set. The test has now been rewritten to use scylla-proxy to introduce artificial delays in request processing, allowing it to work reliably on local Scylla instances as well. Also, I have expanded the test to cover batch statements in addition to simple queries and prepared statements. Finally, I have split the test into two cases: one testing per-statement timeouts, and another testing session-level timeouts, optionally overridden by per-statement long timeouts.
1 parent 2dbbf0d commit 35c7c10

File tree

1 file changed

+130
-61
lines changed

1 file changed

+130
-61
lines changed
Lines changed: 130 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,75 +1,144 @@
1-
use std::time::Duration;
1+
use std::{sync::Arc, time::Duration};
22

33
use assert_matches::assert_matches;
44
use scylla::{
5-
client::execution_profile::ExecutionProfile, errors::ExecutionError, statement::Statement,
5+
client::{execution_profile::ExecutionProfile, session_builder::SessionBuilder},
6+
errors::ExecutionError,
7+
statement::{
8+
Statement,
9+
batch::{Batch, BatchType},
10+
},
11+
};
12+
use scylla_proxy::{
13+
Condition, ProxyError, Reaction as _, RequestOpcode, RequestReaction, RequestRule, WorkerError,
614
};
715

8-
use crate::utils::{create_new_session_builder, setup_tracing};
16+
use crate::utils::{setup_tracing, test_with_3_node_cluster};
917

10-
#[ignore = "works on remote Scylla instances only (local ones are too fast)"]
18+
#[cfg_attr(scylla_cloud_tests, ignore)]
1119
#[tokio::test]
1220
async fn test_request_timeout() {
1321
setup_tracing();
1422

15-
let fast_timeouting_profile_handle = ExecutionProfile::builder()
16-
.request_timeout(Some(Duration::from_millis(1)))
17-
.build()
18-
.into_handle();
19-
20-
{
21-
let session = create_new_session_builder().build().await.unwrap();
22-
23-
let mut query: Statement = Statement::new("SELECT * FROM system_schema.tables");
24-
query.set_request_timeout(Some(Duration::from_millis(1)));
25-
match session.query_unpaged(query, &[]).await {
26-
Ok(_) => panic!("the query should have failed due to a client-side timeout"),
27-
Err(e) => assert_matches!(e, ExecutionError::RequestTimeout(_)),
23+
let res = test_with_3_node_cluster(
24+
scylla_proxy::ShardAwareness::QueryNode,
25+
|proxy_uris, translation_map, mut running_proxy| async move {
26+
let short_timeout = Duration::from_millis(1);
27+
let very_long_timeout = Duration::from_secs(10000);
28+
let query_str = "SELECT host_id FROM system.local WHERE key='local'";
29+
30+
let create_session_builder = || {
31+
SessionBuilder::new()
32+
.known_node(proxy_uris[0].as_str())
33+
.address_translator(Arc::new(translation_map.clone()))
34+
};
35+
36+
running_proxy.running_nodes.iter_mut().for_each(|node| {
37+
node.change_request_rules(Some(vec![
38+
RequestRule(
39+
Condition::any([
40+
Condition::RequestOpcode(RequestOpcode::Query),
41+
Condition::RequestOpcode(RequestOpcode::Execute),
42+
Condition::RequestOpcode(RequestOpcode::Batch),
43+
])
44+
.and(Condition::not(Condition::ConnectionRegisteredAnyEvent)),
45+
RequestReaction::delay(10 * short_timeout)
46+
)
47+
]));
48+
});
49+
50+
let session = create_session_builder()
51+
.build()
52+
.await
53+
.unwrap();
54+
55+
// Case 1: per-statement timeouts.
56+
{
57+
let mut query: Statement = Statement::new(query_str);
58+
query.set_request_timeout(Some(short_timeout));
59+
match session.query_unpaged(query, &[]).await {
60+
Ok(_) => panic!("the query should have failed due to a client-side timeout"),
61+
Err(e) => assert_matches!(e, ExecutionError::RequestTimeout(_)),
62+
}
63+
64+
let mut batch: Batch = Batch::new(BatchType::Logged);
65+
batch.set_request_timeout(Some(short_timeout));
66+
match session.batch(&batch, &[][..] as &[()]).await {
67+
Ok(_) => panic!("the batch should have failed due to a client-side timeout"),
68+
Err(e) => assert_matches!(e, ExecutionError::RequestTimeout(_)),
69+
}
70+
71+
let mut prepared = session
72+
.prepare(query_str)
73+
.await
74+
.unwrap();
75+
76+
prepared.set_request_timeout(Some(short_timeout));
77+
match session.execute_unpaged(&prepared, &[]).await {
78+
Ok(_) => {
79+
panic!("the prepared query should have failed due to a client-side timeout")
80+
}
81+
Err(e) => assert_matches!(e, ExecutionError::RequestTimeout(_)),
82+
};
83+
}
84+
85+
// Case 2: tight session-level timeout, overridden by per-statement long timeouts.
86+
{
87+
let fast_timeouting_profile = ExecutionProfile::builder()
88+
.request_timeout(Some(short_timeout))
89+
.build();
90+
91+
// Although this `clone()` looks suspicious, it is necessary to get an owned handle to the profile
92+
// in order to call `map_to_another_profile()`, which requires a mutable reference.
93+
// The profile handle itself is just a reference-counted pointer, so cloning it is cheap,
94+
// and the remapping affects the session too.
95+
session.get_default_execution_profile_handle().clone().map_to_another_profile(fast_timeouting_profile);
96+
97+
let mut query = Statement::new(query_str);
98+
99+
match session.query_unpaged(query.clone(), &[]).await {
100+
Ok(_) => panic!("the query should have failed due to a client-side timeout"),
101+
Err(e) => assert_matches!(e, ExecutionError::RequestTimeout(_)),
102+
};
103+
104+
query.set_request_timeout(Some(very_long_timeout));
105+
106+
session.query_unpaged(query, &[]).await.expect(
107+
"the query should have not failed, because no client-side timeout was specified",
108+
);
109+
110+
let mut batch: Batch = Batch::new(BatchType::Logged);
111+
match session.batch(&batch, &[][..] as &[()]).await {
112+
Ok(_) => panic!("the batch should have failed due to a client-side timeout"),
113+
Err(e) => assert_matches!(e, ExecutionError::RequestTimeout(_)),
114+
}
115+
116+
batch.set_request_timeout(Some(very_long_timeout));
117+
session.batch(&batch, &[][..] as &[()]).await.expect("the batch should have not failed, because no client-side timeout was specified");
118+
119+
let mut prepared = session
120+
.prepare(query_str)
121+
.await
122+
.unwrap();
123+
124+
match session.execute_unpaged(&prepared, &[]).await {
125+
Ok(_) => {
126+
panic!("the prepared query should have failed due to a client-side timeout")
127+
}
128+
Err(e) => assert_matches!(e, ExecutionError::RequestTimeout(_)),
129+
};
130+
131+
prepared.set_request_timeout(Some(very_long_timeout));
132+
133+
session.execute_unpaged(&prepared, &[]).await.expect("the prepared query should have not failed, because no client-side timeout was specified");
134+
}
135+
running_proxy
28136
}
137+
).await;
29138

30-
let mut prepared = session
31-
.prepare("SELECT * FROM system_schema.tables")
32-
.await
33-
.unwrap();
34-
35-
prepared.set_request_timeout(Some(Duration::from_millis(1)));
36-
match session.execute_unpaged(&prepared, &[]).await {
37-
Ok(_) => panic!("the prepared query should have failed due to a client-side timeout"),
38-
Err(e) => assert_matches!(e, ExecutionError::RequestTimeout(_)),
39-
};
40-
}
41-
{
42-
let timeouting_session = create_new_session_builder()
43-
.default_execution_profile_handle(fast_timeouting_profile_handle)
44-
.build()
45-
.await
46-
.unwrap();
47-
48-
let mut query = Statement::new("SELECT * FROM system_schema.tables");
49-
50-
match timeouting_session.query_unpaged(query.clone(), &[]).await {
51-
Ok(_) => panic!("the query should have failed due to a client-side timeout"),
52-
Err(e) => assert_matches!(e, ExecutionError::RequestTimeout(_)),
53-
};
54-
55-
query.set_request_timeout(Some(Duration::from_secs(10000)));
56-
57-
timeouting_session.query_unpaged(query, &[]).await.expect(
58-
"the query should have not failed, because no client-side timeout was specified",
59-
);
60-
61-
let mut prepared = timeouting_session
62-
.prepare("SELECT * FROM system_schema.tables")
63-
.await
64-
.unwrap();
65-
66-
match timeouting_session.execute_unpaged(&prepared, &[]).await {
67-
Ok(_) => panic!("the prepared query should have failed due to a client-side timeout"),
68-
Err(e) => assert_matches!(e, ExecutionError::RequestTimeout(_)),
69-
};
70-
71-
prepared.set_request_timeout(Some(Duration::from_secs(10000)));
72-
73-
timeouting_session.execute_unpaged(&prepared, &[]).await.expect("the prepared query should have not failed, because no client-side timeout was specified");
139+
match res {
140+
Ok(()) => (),
141+
Err(ProxyError::Worker(WorkerError::DriverDisconnected(_))) => (),
142+
Err(err) => panic!("{}", err),
74143
}
75144
}

0 commit comments

Comments
 (0)