Skip to content

Commit 8f3b76c

Browse files
Only stream 1 /solve request body in driver (#4160)
# Description Because the driver serves multiple solvers it receives a bunch of duplicated `/solve` requests. There is already logic to deduplicate the pre-processing but we there is still one part left that's done unnecessarily often: streaming the HTTP body. Streaming the http body currently takes up to 700ms which is surprisingly slow considering that the HTTP request goes from one k8s pod to another and not via the public internet. I suspect the problem is that we are actually streaming ~10MB `/solve` requests 23 times in parallel (numbers from mainnet). #4159 introduced a new header (`X-Auction-Id`) that can be used to detect which auction a request is related to without having to stream the entire body. With this change everything but prioritizing (i.e. sorting and allocating balances for orders) and the serialization of the driver's `/solve` request will be de-duplicated. That means adding more solvers to the driver will be less costly. If we consider enforcing the same prioritization logic for ALL solvers that could also be de-duplicated leading to more or less 0 overhead for adding more solvers to the same driver. # Changes - inspect `X-Auction-Id` header to figure out whether we have to process the request or just await an existing pre-processing task Note that this change must be released AFTER https://github.com/cowprotocol/services/pull/4159`. The reason is that k8s first rolls out `driver` pods so there would be a period where the old `autopilot` is still sending requests without the `X-Auction-Id` header. ## How to test e2e tests --------- Co-authored-by: ilya <ilya@cow.fi>
1 parent 52f7d7f commit 8f3b76c

4 files changed

Lines changed: 60 additions & 75 deletions

File tree

crates/driver/src/domain/competition/mod.rs

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,9 @@ use {
2222
util::math,
2323
},
2424
alloy::primitives::Bytes,
25+
axum::body::Body,
2526
futures::{StreamExt, future::Either, stream::FuturesUnordered},
26-
hyper::body::Bytes as RequestBytes,
27+
hyper::Request,
2728
itertools::Itertools,
2829
std::{
2930
cmp::Reverse,
@@ -114,14 +115,14 @@ impl Competition {
114115
}
115116

116117
/// Solve an auction as part of this competition.
117-
pub async fn solve(&self, auction: RequestBytes) -> Result<Option<Solved>, Error> {
118+
pub async fn solve(&self, request: Request<Body>) -> Result<Option<Solved>, Error> {
118119
let start = Instant::now();
119120
let timer = ::observe::metrics::metrics()
120121
.on_auction_overhead_start("driver", "pre_processing_total");
121122

122123
let tasks = self
123124
.fetcher
124-
.start_or_get_tasks_for_auction(auction)
125+
.start_or_get_tasks_for_auction(request)
125126
.await
126127
.map_err(|err| {
127128
tracing::error!(?err, "pre-processing auction failed");

crates/driver/src/domain/competition/pre_processing.rs

Lines changed: 51 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -11,9 +11,10 @@ use {
1111
},
1212
alloy::primitives::{Bytes, FixedBytes},
1313
anyhow::{Context, Result},
14+
axum::body::Body,
1415
chrono::Utc,
1516
futures::{FutureExt, StreamExt, future::BoxFuture, stream::FuturesUnordered},
16-
hyper::body::Bytes as RequestBytes,
17+
hyper::{Request, body::Bytes as RequestBytes},
1718
itertools::Itertools,
1819
model::{
1920
interaction::InteractionData,
@@ -25,9 +26,14 @@ use {
2526
price_estimation::trade_verifier::balance_overrides::BalanceOverrideRequest,
2627
signature_validator::SignatureValidating,
2728
},
28-
std::{collections::HashMap, future::Future, sync::Arc, time::Duration},
29+
std::{
30+
collections::HashMap,
31+
future::Future,
32+
sync::Arc,
33+
time::{Duration, Instant},
34+
},
2935
tokio::sync::Mutex,
30-
tracing::Instrument,
36+
tracing::{Instrument, instrument},
3137
};
3238

3339
type Shared<T> = futures::future::Shared<BoxFuture<'static, T>>;
@@ -74,7 +80,7 @@ impl std::fmt::Debug for Utilities {
7480
#[derive(Debug)]
7581
struct ControlBlock {
7682
/// Auction for which the data aggregation task was spawned.
77-
solve_request: RequestBytes,
83+
auction_id: i64,
7884
/// Data aggregation task.
7985
tasks: DataFetchingTasks,
8086
}
@@ -91,26 +97,34 @@ impl DataAggregator {
9197
/// only once for all connected solvers to share.
9298
pub async fn start_or_get_tasks_for_auction(
9399
&self,
94-
request: RequestBytes,
100+
request: Request<Body>,
95101
) -> Result<DataFetchingTasks> {
96102
let mut lock = self.control.lock().await;
97-
let current_auction = &lock.solve_request;
98-
99-
// The autopilot ensures that all drivers receive identical
100-
// requests per auction. That means we can use the significantly
101-
// cheaper string comparison instead of parsing the JSON to compare
102-
// the auction ids.
103-
if request == current_auction {
104-
let id = lock.tasks.auction.clone().await.id;
105-
init_auction_id_in_span(id.map(|i| i.0));
103+
let current_auction = &lock.auction_id;
104+
105+
// Figure out for which auction this `/solve` request was issued
106+
// by looking at the `X-Auction-Id` header.
107+
let request_auction_id: i64 = request
108+
.headers()
109+
.get("X-Auction-Id")
110+
.context("request has no X-Auction-Id header")?
111+
.to_str()
112+
.context("X-Auction-Id header is not ASCII")?
113+
.parse()
114+
.context("could not parse X-Auction-Id header as i64")?;
115+
116+
// Some other driver is already doing the pre-processing for this
117+
// auction. Stop processing here and just await the existing task.
118+
if request_auction_id == *current_auction {
119+
init_auction_id_in_span(Some(request_auction_id));
106120
tracing::debug!("await running data aggregation task");
107121
return Ok(lock.tasks.clone());
108122
}
109123

110-
let tasks = self.assemble_tasks(request.clone()).await?;
124+
let tasks = self.assemble_tasks(request).await?;
111125

112126
tracing::debug!("started new data aggregation task");
113-
lock.solve_request = request;
127+
lock.auction_id = request_auction_id;
114128
lock.tasks = tasks.clone();
115129

116130
Ok(tasks)
@@ -153,7 +167,7 @@ impl DataAggregator {
153167
cow_amm_cache,
154168
}),
155169
control: Mutex::new(ControlBlock {
156-
solve_request: Default::default(),
170+
auction_id: Default::default(),
157171
tasks: DataFetchingTasks {
158172
auction: futures::future::pending().boxed().shared(),
159173
balances: futures::future::pending().boxed().shared(),
@@ -165,7 +179,7 @@ impl DataAggregator {
165179
}
166180
}
167181

168-
async fn assemble_tasks(&self, request: RequestBytes) -> Result<DataFetchingTasks> {
182+
async fn assemble_tasks(&self, request: Request<Body>) -> Result<DataFetchingTasks> {
169183
let auction = self.utilities.parse_request(request).await?;
170184

171185
let balances =
@@ -212,7 +226,9 @@ impl Utilities {
212226
/// Parses the JSON body of the `/solve` request during the unified
213227
/// auction pre-processing since eagerly deserializing these requests
214228
/// is surprisingly costly because their are so big.
215-
async fn parse_request(&self, solve_request: RequestBytes) -> Result<Arc<Auction>> {
229+
async fn parse_request(&self, solve_request: Request<Body>) -> Result<Arc<Auction>> {
230+
let solve_request = collect_request_body(solve_request).await?;
231+
216232
let auction_dto: SolveRequest = {
217233
let _timer = metrics::get().processing_stage_timer("parse_dto");
218234
let _timer2 =
@@ -539,3 +555,19 @@ fn init_auction_id_in_span(id: Option<i64>) {
539555
debug_assert!(current_span.has_field("auction_id"));
540556
current_span.record("auction_id", id);
541557
}
558+
559+
#[instrument(skip_all)]
560+
async fn collect_request_body(request: Request<Body>) -> Result<RequestBytes> {
561+
tracing::trace!("start streaming request body");
562+
let _timer =
563+
observe::metrics::metrics().on_auction_overhead_start("driver", "stream_http_body");
564+
let start = Instant::now();
565+
566+
let body_bytes = hyper::body::to_bytes(request.into_body())
567+
.await
568+
.context("failed to stream request body")?;
569+
570+
let duration = start.elapsed();
571+
tracing::debug!(?duration, "finished streaming request body");
572+
Ok(body_bytes)
573+
}

crates/driver/src/infra/api/routes/solve/mod.rs

Lines changed: 4 additions & 53 deletions
Original file line numberDiff line numberDiff line change
@@ -2,16 +2,11 @@ pub mod dto;
22

33
pub use dto::AuctionError;
44
use {
5-
crate::{
6-
domain::competition,
7-
infra::{
8-
api::{Error, State},
9-
observe,
10-
},
5+
crate::infra::{
6+
api::{Error, State},
7+
observe,
118
},
129
axum::{body::Body, http::Request},
13-
hyper::body::Bytes,
14-
std::time::{Duration, Instant},
1510
tracing::Instrument,
1611
};
1712

@@ -29,9 +24,8 @@ async fn route(
2924
let solver = state.solver().name().as_str();
3025

3126
let handle_request = async {
32-
let body_bytes = collect_request_body(request, solver).await?;
3327
let competition = state.competition();
34-
let result = competition.solve(body_bytes).await;
28+
let result = competition.solve(request).await;
3529
// Solving takes some time, so there is a chance for the settlement queue to
3630
// have capacity again.
3731
competition.ensure_settle_queue_capacity()?;
@@ -50,46 +44,3 @@ async fn route(
5044
))
5145
.await
5246
}
53-
54-
async fn collect_request_body(
55-
request: Request<Body>,
56-
solver: &str,
57-
) -> Result<Bytes, competition::Error> {
58-
tracing::trace!("start streaming request body");
59-
let start = Instant::now();
60-
61-
let body_bytes = hyper::body::to_bytes(request.into_body())
62-
.await
63-
.map_err(|err| {
64-
tracing::warn!(?err, "failed to stream request body");
65-
competition::Error::MalformedRequest
66-
})?;
67-
68-
let duration = start.elapsed();
69-
Metrics::measure_solve_transfer_time(solver, duration);
70-
tracing::trace!(?duration, "finished streaming request body");
71-
Ok(body_bytes)
72-
}
73-
74-
#[derive(prometheus_metric_storage::MetricStorage)]
75-
struct Metrics {
76-
/// Time spent by the driver reading the full solve request body into
77-
/// memory.
78-
#[metric(labels("solver"))]
79-
#[metric(buckets(0.0001, 0.0005, 0.002, 0.05, 0.1, 0.2, 0.3, 0.4, 0.5, 0.75, 1, 1.5))]
80-
solve_request_body_read_duration_seconds: prometheus::HistogramVec,
81-
}
82-
83-
impl Metrics {
84-
fn get() -> &'static Metrics {
85-
Metrics::instance(::observe::metrics::get_storage_registry())
86-
.expect("unexpected error getting metrics instance")
87-
}
88-
89-
fn measure_solve_transfer_time(solver: &str, time: Duration) {
90-
Self::get()
91-
.solve_request_body_read_duration_seconds
92-
.with_label_values(&[solver])
93-
.observe(time.as_secs_f64());
94-
}
95-
}

crates/driver/src/tests/setup/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1059,6 +1059,7 @@ impl Test {
10591059
let res = self
10601060
.client
10611061
.post(format!("http://{}/{}/solve", self.driver.addr, solver))
1062+
.header("X-Auction-Id", self.auction_id)
10621063
.json(&driver::solve_req(self))
10631064
.send()
10641065
.await

0 commit comments

Comments
 (0)