Skip to content

Commit f7a56fa

Browse files
committed
feat(router): Subgraph Timeout Configuration
1 parent 0438acf commit f7a56fa

File tree

8 files changed

+200
-42
lines changed

8 files changed

+200
-42
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

lib/executor/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@ hyper-util = { version = "0.1.16", features = [
4444
"http2",
4545
"tokio",
4646
] }
47+
humantime = "2.3.0"
4748
bytes = "1.10.1"
4849
itoa = "1.0.15"
4950
ryu = "1.0.20"

lib/executor/src/execution/plan.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -708,6 +708,7 @@ impl<'exec, 'req> Executor<'exec, 'req> {
708708
representations,
709709
headers: headers_map,
710710
extensions: None,
711+
client_request: self.client_request,
711712
};
712713

713714
if let Some(jwt_forwarding_plan) = &self.jwt_forwarding_plan {
@@ -722,7 +723,7 @@ impl<'exec, 'req> Executor<'exec, 'req> {
722723
subgraph_name: node.service_name.clone(),
723724
response: self
724725
.executors
725-
.execute(&node.service_name, subgraph_request, self.client_request)
726+
.execute(&node.service_name, subgraph_request)
726727
.await
727728
.into(),
728729
}))

lib/executor/src/executors/common.rs

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,13 @@ use bytes::Bytes;
55
use http::HeaderMap;
66
use sonic_rs::Value;
77

8+
use crate::execution::client_request_details::ClientRequestDetails;
9+
810
#[async_trait]
911
pub trait SubgraphExecutor {
10-
async fn execute<'a>(
12+
async fn execute<'exec, 'req>(
1113
&self,
12-
execution_request: HttpExecutionRequest<'a>,
14+
execution_request: HttpExecutionRequest<'exec, 'req>,
1315
) -> HttpExecutionResponse;
1416

1517
fn to_boxed_arc<'a>(self) -> Arc<Box<dyn SubgraphExecutor + Send + Sync + 'a>>
@@ -26,18 +28,19 @@ pub type SubgraphExecutorBoxedArc = Arc<Box<SubgraphExecutorType>>;
2628

2729
pub type SubgraphRequestExtensions = HashMap<String, Value>;
2830

29-
pub struct HttpExecutionRequest<'a> {
30-
pub query: &'a str,
31+
pub struct HttpExecutionRequest<'exec, 'req> {
32+
pub query: &'exec str,
3133
pub dedupe: bool,
32-
pub operation_name: Option<&'a str>,
34+
pub operation_name: Option<&'exec str>,
3335
// TODO: variables could be stringified before even executing the request
34-
pub variables: Option<HashMap<&'a str, &'a sonic_rs::Value>>,
36+
pub variables: Option<HashMap<&'exec str, &'exec sonic_rs::Value>>,
3537
pub headers: HeaderMap,
3638
pub representations: Option<Vec<u8>>,
3739
pub extensions: Option<SubgraphRequestExtensions>,
40+
pub client_request: &'exec ClientRequestDetails<'exec, 'req>,
3841
}
3942

40-
impl HttpExecutionRequest<'_> {
43+
impl HttpExecutionRequest<'_, '_> {
4144
pub fn add_request_extensions_field(&mut self, key: String, value: Value) {
4245
self.extensions
4346
.get_or_insert_with(HashMap::new)

lib/executor/src/executors/error.rs

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ pub enum SubgraphExecutorError {
2020
RequestFailure(String, String),
2121
#[error("Failed to serialize variable \"{0}\": {1}")]
2222
VariablesSerializationFailure(String, String),
23+
#[error("Failed to resolve VRL expression for timeout. Runtime error: {0}")]
24+
TimeoutExpressionResolutionFailure(String),
25+
#[error("Request to subgraph \"{0}\" timed out after {1} milliseconds")]
26+
RequestTimeout(String, u64),
2327
}
2428

2529
impl From<SubgraphExecutorError> for GraphQLError {
@@ -61,6 +65,10 @@ impl SubgraphExecutorError {
6165
SubgraphExecutorError::VariablesSerializationFailure(_, _) => {
6266
"SUBGRAPH_VARIABLES_SERIALIZATION_FAILURE"
6367
}
68+
SubgraphExecutorError::TimeoutExpressionResolutionFailure(_) => {
69+
"SUBGRAPH_TIMEOUT_EXPRESSION_RESOLUTION_FAILURE"
70+
}
71+
SubgraphExecutorError::RequestTimeout(_, _) => "SUBGRAPH_REQUEST_TIMEOUT",
6472
}
6573
}
6674
}

lib/executor/src/executors/http.rs

Lines changed: 78 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,11 @@
1+
use std::collections::BTreeMap;
12
use std::sync::Arc;
23

4+
use crate::execution::client_request_details::ClientRequestDetails;
35
use crate::executors::common::HttpExecutionResponse;
46
use crate::executors::dedupe::{request_fingerprint, ABuildHasher, SharedResponse};
57
use dashmap::DashMap;
6-
use hive_router_config::HiveRouterConfig;
8+
use hive_router_config::traffic_shaping::DurationOrExpression;
79
use tokio::sync::OnceCell;
810

911
use async_trait::async_trait;
@@ -18,6 +20,7 @@ use hyper_tls::HttpsConnector;
1820
use hyper_util::client::legacy::{connect::HttpConnector, Client};
1921
use tokio::sync::Semaphore;
2022
use tracing::debug;
23+
use vrl::core::Value as VrlValue;
2124

2225
use crate::executors::common::HttpExecutionRequest;
2326
use crate::executors::error::SubgraphExecutorError;
@@ -35,8 +38,9 @@ pub struct HTTPSubgraphExecutor {
3538
pub http_client: Arc<Client<HttpsConnector<HttpConnector>, Full<Bytes>>>,
3639
pub header_map: HeaderMap,
3740
pub semaphore: Arc<Semaphore>,
38-
pub config: Arc<HiveRouterConfig>,
41+
pub dedupe_enabled: bool,
3942
pub in_flight_requests: Arc<DashMap<u64, Arc<OnceCell<SharedResponse>>, ABuildHasher>>,
43+
pub timeout: DurationOrExpression,
4044
}
4145

4246
const FIRST_VARIABLE_STR: &[u8] = b",\"variables\":{";
@@ -50,8 +54,9 @@ impl HTTPSubgraphExecutor {
5054
endpoint: http::Uri,
5155
http_client: Arc<HttpClient>,
5256
semaphore: Arc<Semaphore>,
53-
config: Arc<HiveRouterConfig>,
57+
dedupe_enabled: bool,
5458
in_flight_requests: Arc<DashMap<u64, Arc<OnceCell<SharedResponse>>, ABuildHasher>>,
59+
timeout: DurationOrExpression,
5560
) -> Self {
5661
let mut header_map = HeaderMap::new();
5762
header_map.insert(
@@ -69,14 +74,15 @@ impl HTTPSubgraphExecutor {
6974
http_client,
7075
header_map,
7176
semaphore,
72-
config,
77+
dedupe_enabled,
7378
in_flight_requests,
79+
timeout,
7480
}
7581
}
7682

77-
fn build_request_body<'a>(
83+
fn build_request_body(
7884
&self,
79-
execution_request: &HttpExecutionRequest<'a>,
85+
execution_request: &HttpExecutionRequest<'_, '_>,
8086
) -> Result<Vec<u8>, SubgraphExecutorError> {
8187
let mut body = Vec::with_capacity(4096);
8288
body.put(FIRST_QUOTE_STR);
@@ -137,6 +143,7 @@ impl HTTPSubgraphExecutor {
137143
&self,
138144
body: Vec<u8>,
139145
headers: HeaderMap,
146+
client_request: &ClientRequestDetails<'_, '_>,
140147
) -> Result<SharedResponse, SubgraphExecutorError> {
141148
let mut req = hyper::Request::builder()
142149
.method(http::Method::POST)
@@ -151,9 +158,62 @@ impl HTTPSubgraphExecutor {
151158

152159
debug!("making http request to {}", self.endpoint.to_string());
153160

154-
let res = self.http_client.request(req).await.map_err(|e| {
155-
SubgraphExecutorError::RequestFailure(self.endpoint.to_string(), e.to_string())
156-
})?;
161+
let timeout = match &self.timeout {
162+
DurationOrExpression::Duration(dur) => *dur,
163+
DurationOrExpression::Expression(expr) => {
164+
let value =
165+
VrlValue::Object(BTreeMap::from([("request".into(), client_request.into())]));
166+
let result = expr.execute(value).map_err(|err| {
167+
SubgraphExecutorError::TimeoutExpressionResolutionFailure(err.to_string())
168+
})?;
169+
match result {
170+
VrlValue::Integer(i) if i >= 0 => std::time::Duration::from_millis(i as u64),
171+
VrlValue::Float(f) => {
172+
let f = f.into_inner();
173+
if f >= 0.0 {
174+
std::time::Duration::from_millis(f as u64)
175+
} else {
176+
return Err(SubgraphExecutorError::TimeoutExpressionResolutionFailure(
177+
"Timeout expression resolved to a negative float".to_string(),
178+
));
179+
}
180+
}
181+
VrlValue::Bytes(b) => {
182+
let str: String = String::from_utf8(b.to_vec()).map_err(|e| {
183+
SubgraphExecutorError::TimeoutExpressionResolutionFailure(format!(
184+
"Failed to parse duration string from bytes: {}",
185+
e
186+
))
187+
})?;
188+
let parsed = humantime::parse_duration(&str).map_err(|e| {
189+
SubgraphExecutorError::TimeoutExpressionResolutionFailure(format!(
190+
"Failed to parse duration string '{}': {}",
191+
str, e
192+
))
193+
})?;
194+
parsed
195+
}
196+
_ => {
197+
return Err(SubgraphExecutorError::TimeoutExpressionResolutionFailure(
198+
"Timeout expression did not resolve to a non-negative integer or float"
199+
.to_string(),
200+
));
201+
}
202+
}
203+
}
204+
};
205+
206+
let res = tokio::time::timeout(timeout, self.http_client.request(req))
207+
.await
208+
.map_err(|_| {
209+
SubgraphExecutorError::RequestTimeout(
210+
self.endpoint.to_string(),
211+
timeout.as_millis() as u64,
212+
)
213+
})?
214+
.map_err(|e| {
215+
SubgraphExecutorError::RequestFailure(self.endpoint.to_string(), e.to_string())
216+
})?;
157217

158218
debug!(
159219
"http request to {} completed, status: {}",
@@ -210,9 +270,9 @@ impl HTTPSubgraphExecutor {
210270
#[async_trait]
211271
impl SubgraphExecutor for HTTPSubgraphExecutor {
212272
#[tracing::instrument(skip_all, fields(subgraph_name = self.subgraph_name))]
213-
async fn execute<'a>(
273+
async fn execute<'exec, 'req>(
214274
&self,
215-
execution_request: HttpExecutionRequest<'a>,
275+
execution_request: HttpExecutionRequest<'exec, 'req>,
216276
) -> HttpExecutionResponse {
217277
let body = match self.build_request_body(&execution_request) {
218278
Ok(body) => body,
@@ -230,11 +290,14 @@ impl SubgraphExecutor for HTTPSubgraphExecutor {
230290
headers.insert(key, value.clone());
231291
});
232292

233-
if !self.config.traffic_shaping.dedupe_enabled || !execution_request.dedupe {
293+
if !self.dedupe_enabled || !execution_request.dedupe {
234294
// This unwrap is safe because the semaphore is never closed during the application's lifecycle.
235295
// `acquire()` only fails if the semaphore is closed, so this will always return `Ok`.
236296
let _permit = self.semaphore.acquire().await.unwrap();
237-
return match self._send_request(body, headers).await {
297+
return match self
298+
._send_request(body, headers, execution_request.client_request)
299+
.await
300+
{
238301
Ok(shared_response) => HttpExecutionResponse {
239302
body: shared_response.body,
240303
headers: shared_response.headers,
@@ -266,7 +329,8 @@ impl SubgraphExecutor for HTTPSubgraphExecutor {
266329
// This unwrap is safe because the semaphore is never closed during the application's lifecycle.
267330
// `acquire()` only fails if the semaphore is closed, so this will always return `Ok`.
268331
let _permit = self.semaphore.acquire().await.unwrap();
269-
self._send_request(body, headers).await
332+
self._send_request(body, headers, execution_request.client_request)
333+
.await
270334
};
271335
// It's important to remove the entry from the map before returning the result.
272336
// This ensures that once the OnceCell is set, no future requests can join it.

lib/executor/src/executors/map.rs

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ impl SubgraphExecutorMap {
5959
let client: HttpClient = Client::builder(TokioExecutor::new())
6060
.pool_timer(TokioTimer::new())
6161
.pool_idle_timeout(Duration::from_secs(
62-
config.traffic_shaping.pool_idle_timeout_seconds,
62+
config.traffic_shaping.all.pool_idle_timeout_seconds,
6363
))
6464
.pool_max_idle_per_host(config.traffic_shaping.max_connections_per_host)
6565
.build(https);
@@ -105,13 +105,12 @@ impl SubgraphExecutorMap {
105105
Ok(subgraph_executor_map)
106106
}
107107

108-
pub async fn execute<'a, 'req>(
108+
pub async fn execute<'exec, 'req>(
109109
&self,
110110
subgraph_name: &str,
111-
execution_request: HttpExecutionRequest<'a>,
112-
client_request: &ClientRequestDetails<'a, 'req>,
111+
execution_request: HttpExecutionRequest<'exec, 'req>,
113112
) -> HttpExecutionResponse {
114-
match self.get_or_create_executor(subgraph_name, client_request) {
113+
match self.get_or_create_executor(subgraph_name, execution_request.client_request) {
115114
Ok(Some(executor)) => executor.execute(execution_request).await,
116115
Err(err) => {
117116
error!(
@@ -295,13 +294,33 @@ impl SubgraphExecutorMap {
295294
.or_insert_with(|| Arc::new(Semaphore::new(self.max_connections_per_host)))
296295
.clone();
297296

297+
let mut client = self.client.clone();
298+
let mut dedupe_enabled = self.config.traffic_shaping.all.dedupe_enabled;
299+
let mut timeout = self.config.traffic_shaping.all.request_timeout.clone();
300+
if let Some(subgraph_traffic_shaping_config) =
301+
self.config.traffic_shaping.subgraphs.get(subgraph_name)
302+
{
303+
client = Arc::new(
304+
Client::builder(TokioExecutor::new())
305+
.pool_timer(TokioTimer::new())
306+
.pool_idle_timeout(Duration::from_secs(
307+
subgraph_traffic_shaping_config.pool_idle_timeout_seconds,
308+
))
309+
.pool_max_idle_per_host(self.max_connections_per_host)
310+
.build(HttpsConnector::new()),
311+
);
312+
dedupe_enabled = subgraph_traffic_shaping_config.dedupe_enabled;
313+
timeout = subgraph_traffic_shaping_config.request_timeout.clone();
314+
}
315+
298316
let executor = HTTPSubgraphExecutor::new(
299317
subgraph_name.to_string(),
300318
endpoint_uri,
301-
self.client.clone(),
319+
client,
302320
semaphore,
303-
self.config.clone(),
321+
dedupe_enabled,
304322
self.in_flight_requests.clone(),
323+
timeout,
305324
);
306325

307326
self.executors_by_subgraph

0 commit comments

Comments
 (0)