Skip to content

Commit 59637ff

Browse files
committed
refactor(handshake): async TransportSender
1 parent 0dfc8d0 commit 59637ff

File tree

10 files changed

+93
-112
lines changed

10 files changed

+93
-112
lines changed

core/handshake/src/handshake.rs

+6-6
Original file line numberDiff line numberDiff line change
@@ -178,7 +178,7 @@ impl<P: ExecutorProviderInterface> Context<P> {
178178

179179
// Attempt to connect to the service, getting the unix socket.
180180
let Some(mut socket) = self.provider.connect(service).await else {
181-
sender.terminate(TerminationReason::InvalidService);
181+
sender.terminate(TerminationReason::InvalidService).await;
182182
warn!("failed to connect to service {service}");
183183
return;
184184
};
@@ -189,7 +189,7 @@ impl<P: ExecutorProviderInterface> Context<P> {
189189
};
190190

191191
if let Err(e) = write_header(&header, &mut socket).await {
192-
sender.terminate(TerminationReason::ServiceTerminated);
192+
sender.terminate(TerminationReason::ServiceTerminated).await;
193193
warn!("failed to write connection header to service {service}: {e}");
194194
return;
195195
}
@@ -226,12 +226,12 @@ impl<P: ExecutorProviderInterface> Context<P> {
226226
let connection_id = u64::from_be_bytes(*arrayref::array_ref![access_token, 0, 8]);
227227

228228
let Some(connection) = self.connections.get(&connection_id) else {
229-
sender.terminate(TerminationReason::InvalidToken);
229+
sender.terminate(TerminationReason::InvalidToken).await;
230230
return;
231231
};
232232

233233
if connection.access_token != access_token {
234-
sender.terminate(TerminationReason::InvalidToken);
234+
sender.terminate(TerminationReason::InvalidToken).await;
235235
return;
236236
}
237237

@@ -241,7 +241,7 @@ impl<P: ExecutorProviderInterface> Context<P> {
241241
.expect("Failed to get current time")
242242
.as_millis()
243243
{
244-
sender.terminate(TerminationReason::InvalidToken);
244+
sender.terminate(TerminationReason::InvalidToken).await;
245245
return;
246246
}
247247

@@ -255,7 +255,7 @@ impl<P: ExecutorProviderInterface> Context<P> {
255255
retry: Some(id), ..
256256
} => {
257257
let Some(connection) = self.connections.get(&id) else {
258-
sender.terminate(TerminationReason::InvalidToken);
258+
sender.terminate(TerminationReason::InvalidToken).await;
259259
return;
260260
};
261261

core/handshake/src/proxy.rs

+16-16
Original file line numberDiff line numberDiff line change
@@ -179,7 +179,7 @@ impl<P: ExecutorProviderInterface> Proxy<P> {
179179
res = receiver.recv() => {
180180
match async_map(res, |r| self.handle_incoming(is_primary, r)).await {
181181
Some(HandleRequestResult::Ok) if is_primary => {
182-
self.maybe_flush_primary_queue(true, &mut sender);
182+
self.maybe_flush_primary_queue(true, &mut sender).await;
183183
},
184184
Some(HandleRequestResult::Ok) => {},
185185
Some(HandleRequestResult::TerminateConnection) => {
@@ -201,13 +201,13 @@ impl<P: ExecutorProviderInterface> Proxy<P> {
201201
// the same connection mode. The client must have lost its connection.
202202
// We terminate the current (old) connection with a `ConnectionInUse` error.
203203
(true, true) => {
204-
sender.terminate(TerminationReason::ConnectionInUse);
204+
sender.terminate(TerminationReason::ConnectionInUse).await;
205205
self.discard_bytes = true;
206206
self.queued_primary_response.clear();
207207
State::OnlyPrimaryConnection(pair)
208208
},
209209
(false, false) => {
210-
sender.terminate(TerminationReason::ConnectionInUse);
210+
sender.terminate(TerminationReason::ConnectionInUse).await;
211211
self.discard_bytes = true;
212212
State::OnlySecondaryConnection(pair)
213213
}
@@ -255,12 +255,12 @@ impl<P: ExecutorProviderInterface> Proxy<P> {
255255
// This might be a window to flush some pending responses to the
256256
// primary.
257257
if is_primary {
258-
self.maybe_flush_primary_queue(true, &mut sender);
258+
self.maybe_flush_primary_queue(true, &mut sender).await;
259259
}
260260

261261
let bytes = self.buffer.split_to(4);
262262
let len = u32::from_be_bytes(*array_ref![bytes, 0, 4]) as usize;
263-
sender.start_write(len);
263+
sender.start_write(len).await;
264264
self.current_write = len;
265265
continue 'inner; // to handle `len` == 0.
266266
}
@@ -274,7 +274,7 @@ impl<P: ExecutorProviderInterface> Proxy<P> {
274274
continue 'inner;
275275
}
276276

277-
if sender.write(bytes.freeze()).is_err() {
277+
if sender.write(bytes.freeze()).await.is_err() {
278278
self.discard_bytes = true;
279279
self.queued_primary_response.clear();
280280
return State::NoConnection;
@@ -285,7 +285,7 @@ impl<P: ExecutorProviderInterface> Proxy<P> {
285285
}
286286
};
287287

288-
sender.terminate(reason);
288+
sender.terminate(reason).await;
289289
State::Terminated
290290
}
291291

@@ -312,7 +312,7 @@ impl<P: ExecutorProviderInterface> Proxy<P> {
312312
res = p_receiver.recv() => {
313313
match async_map(res, |r| self.handle_incoming(true, r)).await {
314314
Some(HandleRequestResult::Ok) => {
315-
self.maybe_flush_primary_queue(false, &mut p_sender);
315+
self.maybe_flush_primary_queue(false, &mut p_sender).await;
316316
},
317317
Some(HandleRequestResult::TerminateConnection) => {
318318
break 'outer TerminationReason::InternalError;
@@ -393,11 +393,11 @@ impl<P: ExecutorProviderInterface> Proxy<P> {
393393

394394
// This might be a window to flush some pending responses to the
395395
// primary.
396-
self.maybe_flush_primary_queue(false, &mut p_sender);
396+
self.maybe_flush_primary_queue(false, &mut p_sender).await;
397397

398398
let bytes = self.buffer.split_to(4);
399399
let len = u32::from_be_bytes(*array_ref![bytes, 0, 4]) as usize;
400-
s_sender.start_write(len);
400+
s_sender.start_write(len).await;
401401
self.current_write = len;
402402
continue 'inner; // to handle `len` == 0.
403403
}
@@ -412,14 +412,14 @@ impl<P: ExecutorProviderInterface> Proxy<P> {
412412
}
413413

414414
return if self.is_primary_the_current_sender {
415-
if p_sender.write(bytes.freeze()).is_ok() {
415+
if p_sender.write(bytes.freeze()).await.is_ok() {
416416
continue 'inner;
417417
}
418418
self.discard_bytes = true;
419419
self.queued_primary_response.clear();
420420
State::OnlySecondaryConnection((s_sender, s_receiver).into())
421421
} else {
422-
if s_sender.write(bytes.freeze()).is_ok() {
422+
if s_sender.write(bytes.freeze()).await.is_ok() {
423423
continue 'inner;
424424
}
425425
self.discard_bytes = true;
@@ -431,8 +431,8 @@ impl<P: ExecutorProviderInterface> Proxy<P> {
431431
}
432432
};
433433

434-
s_sender.terminate(reason);
435-
p_sender.terminate(reason);
434+
s_sender.terminate(reason).await;
435+
p_sender.terminate(reason).await;
436436
State::Terminated
437437
}
438438

@@ -493,7 +493,7 @@ impl<P: ExecutorProviderInterface> Proxy<P> {
493493
}
494494

495495
#[inline(always)]
496-
fn maybe_flush_primary_queue<S: TransportSender>(
496+
async fn maybe_flush_primary_queue<S: TransportSender>(
497497
&mut self,
498498
only_primary: bool,
499499
sender: &mut S,
@@ -505,7 +505,7 @@ impl<P: ExecutorProviderInterface> Proxy<P> {
505505
let is_primary_current_writer = only_primary || self.is_primary_the_current_sender;
506506
if !is_primary_current_writer || self.current_write == 0 || self.discard_bytes {
507507
while let Some(res) = self.queued_primary_response.pop_back() {
508-
sender.send(res);
508+
sender.send(res).await;
509509
}
510510
}
511511
}

core/handshake/src/transports/http/handler.rs

+1-5
Original file line numberDiff line numberDiff line change
@@ -52,11 +52,7 @@ pub async fn handler<P: ExecutorProviderInterface>(
5252
};
5353

5454
let (frame_tx, frame_rx) = async_channel::bounded(8);
55-
// Todo: Fix synchronization between data transfer and connection proxy.
56-
// Notes: Reducing the size of this queue produces enough backpressure to slow down the
57-
// transfer of data and if the socket is dropped before all the data is transferred,
58-
// the proxy drops the connection and the client receives incomplete data.
59-
let (body_tx, body_rx) = async_channel::bounded(2_000_000);
55+
let (body_tx, body_rx) = async_channel::bounded(16);
6056
let (termination_tx, termination_rx) = oneshot::channel();
6157

6258
let sender = HttpSender::new(service_id, frame_tx, body_tx, termination_tx);

core/handshake/src/transports/http/mod.rs

+13-17
Original file line numberDiff line numberDiff line change
@@ -95,25 +95,22 @@ impl HttpSender {
9595
}
9696

9797
#[inline(always)]
98-
fn inner_send(&mut self, bytes: Bytes) {
99-
if let Err(e) = self.body_tx.try_send(Ok(bytes)) {
98+
async fn inner_send(&mut self, bytes: Bytes) {
99+
if let Err(e) = self.body_tx.send(Ok(bytes)).await {
100100
warn!("payload dropped, failed to send to write loop: {e}");
101101
}
102102
}
103103

104-
fn send_with_http_override(&mut self, bytes: Bytes) {
104+
async fn send_with_http_override(&mut self, bytes: Bytes) {
105105
if let Some(header_buffer) = self.header_buffer.as_mut() {
106106
header_buffer.extend(bytes);
107107
if self.current_write == 0 {
108108
// always Some due to previous check
109109
let payload = self.header_buffer.take().unwrap_or_default();
110-
111-
if let Err(e) = self.body_tx.try_send(Ok(payload.into())) {
112-
warn!("payload dropped, failed to send to write loop: {e}");
113-
}
110+
self.inner_send(payload.freeze()).await;
114111
}
115-
} else if let Err(e) = self.body_tx.try_send(Ok(bytes)) {
116-
warn!("payload dropped, failed to send to write loop: {e}");
112+
} else {
113+
self.inner_send(bytes).await
117114
}
118115
}
119116

@@ -126,21 +123,21 @@ impl HttpSender {
126123
}
127124

128125
impl TransportSender for HttpSender {
129-
fn send_handshake_response(&mut self, _: HandshakeResponse) {
126+
async fn send_handshake_response(&mut self, _: HandshakeResponse) {
130127
unimplemented!()
131128
}
132129

133-
fn send(&mut self, _: ResponseFrame) {
130+
async fn send(&mut self, _: ResponseFrame) {
134131
unimplemented!()
135132
}
136133

137-
fn terminate(mut self, reason: TerminationReason) {
134+
async fn terminate(mut self, reason: TerminationReason) {
138135
if let Some(reason_sender) = self.termination_tx.take() {
139136
let _ = reason_sender.send(reason);
140137
}
141138
}
142139

143-
fn start_write(&mut self, len: usize) {
140+
async fn start_write(&mut self, len: usize) {
144141
// if the header buffer is gone it means we sent the headers already and are ready to stream
145142
// the body
146143
if self.header_buffer.is_none() || !self.service.supports_http_overrides() {
@@ -155,7 +152,7 @@ impl TransportSender for HttpSender {
155152
self.current_write = len;
156153
}
157154

158-
fn write(&mut self, buf: Bytes) -> anyhow::Result<usize> {
155+
async fn write(&mut self, buf: Bytes) -> anyhow::Result<usize> {
159156
let len = buf.len();
160157

161158
debug_assert!(self.current_write != 0);
@@ -164,9 +161,9 @@ impl TransportSender for HttpSender {
164161
self.current_write -= len;
165162

166163
if self.service.supports_http_overrides() {
167-
self.send_with_http_override(buf);
164+
self.send_with_http_override(buf).await;
168165
} else {
169-
self.inner_send(buf);
166+
self.inner_send(buf).await;
170167
}
171168

172169
Ok(len)
@@ -187,7 +184,6 @@ impl HttpReceiver {
187184
}
188185
}
189186

190-
#[async_trait]
191187
impl TransportReceiver for HttpReceiver {
192188
fn detail(&mut self) -> TransportDetail {
193189
self.detail

core/handshake/src/transports/mock.rs

+9-9
Original file line numberDiff line numberDiff line change
@@ -115,36 +115,37 @@ pub struct MockTransportSender {
115115
}
116116

117117
impl MockTransportSender {
118-
fn send_inner(&mut self, bytes: Bytes) {
118+
async fn send_inner(&mut self, bytes: Bytes) {
119119
self.tx
120120
.try_send(bytes)
121121
.expect("failed to send bytes over the mock connection")
122122
}
123123
}
124124

125125
impl TransportSender for MockTransportSender {
126-
fn send_handshake_response(&mut self, response: schema::HandshakeResponse) {
127-
self.send_inner(response.encode());
126+
async fn send_handshake_response(&mut self, response: schema::HandshakeResponse) {
127+
self.send_inner(response.encode()).await;
128128
}
129129

130-
fn send(&mut self, frame: schema::ResponseFrame) {
131-
self.send_inner(frame.encode());
130+
async fn send(&mut self, frame: schema::ResponseFrame) {
131+
self.send_inner(frame.encode()).await;
132132
}
133133

134-
fn start_write(&mut self, len: usize) {
134+
async fn start_write(&mut self, len: usize) {
135135
debug_assert!(self.buffer.is_empty());
136136
self.buffer.reserve(len);
137137
self.current_write = len;
138138
}
139139

140-
fn write(&mut self, buf: Bytes) -> anyhow::Result<usize> {
140+
async fn write(&mut self, buf: Bytes) -> anyhow::Result<usize> {
141141
let len = buf.len();
142142
debug_assert!(len <= self.current_write);
143143
self.buffer.put(buf);
144144
if self.buffer.len() >= self.current_write {
145145
let bytes = self.buffer.split_to(self.current_write).into();
146146
self.current_write = 0;
147-
self.send(schema::ResponseFrame::ServicePayload { bytes });
147+
self.send(schema::ResponseFrame::ServicePayload { bytes })
148+
.await;
148149
}
149150
Ok(len)
150151
}
@@ -155,7 +156,6 @@ pub struct MockTransportReceiver {
155156
rx: async_channel::Receiver<Bytes>,
156157
}
157158

158-
#[async_trait]
159159
impl TransportReceiver for MockTransportReceiver {
160160
async fn recv(&mut self) -> Option<schema::RequestFrame> {
161161
let bytes = self.rx.recv().await.ok()?;

core/handshake/src/transports/mod.rs

+14-8
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use async_trait::async_trait;
22
use axum::Router;
33
use bytes::{BufMut, Bytes, BytesMut};
44
use fn_sdk::header::TransportDetail;
5+
use futures::Future;
56
use lightning_interfaces::prelude::*;
67
use serde::de::DeserializeOwned;
78
use serde::Serialize;
@@ -101,25 +102,30 @@ pub trait Transport: Sized + Send + Sync + 'static {
101102
// for secondary connections are added
102103
pub trait TransportSender: Sized + Send + Sync + 'static {
103104
/// Send the initial handshake response to the client.
104-
fn send_handshake_response(&mut self, response: schema::HandshakeResponse);
105+
fn send_handshake_response(
106+
&mut self,
107+
response: schema::HandshakeResponse,
108+
) -> impl Future<Output = ()> + Send;
105109

106110
/// Send a frame to the client.
107-
fn send(&mut self, frame: schema::ResponseFrame);
111+
fn send(&mut self, frame: schema::ResponseFrame) -> impl Future<Output = ()> + Send;
108112

109113
/// Terminate the connection
110-
fn terminate(mut self, reason: schema::TerminationReason) {
111-
self.send(schema::ResponseFrame::Termination { reason })
114+
fn terminate(mut self, reason: schema::TerminationReason) -> impl Future<Output = ()> + Send {
115+
async move {
116+
self.send(schema::ResponseFrame::Termination { reason })
117+
.await
118+
}
112119
}
113120

114121
/// Declare a number of bytes to write as service payloads.
115-
fn start_write(&mut self, len: usize);
122+
fn start_write(&mut self, len: usize) -> impl Future<Output = ()> + Send;
116123

117124
/// Write some bytes as service payloads. Must ALWAYS be called after
118125
/// [`TransportSender::start_write`].
119-
fn write(&mut self, buf: Bytes) -> anyhow::Result<usize>;
126+
fn write(&mut self, buf: Bytes) -> impl Future<Output = anyhow::Result<usize>> + Send;
120127
}
121128

122-
#[async_trait]
123129
pub trait TransportReceiver: Send + Sync + 'static {
124130
/// Returns the transport detail from this connection which is then sent to the service on
125131
/// the hello frame.
@@ -129,7 +135,7 @@ pub trait TransportReceiver: Send + Sync + 'static {
129135

130136
/// Receive a frame from the connection. Returns `None` when the connection
131137
/// is closed.
132-
async fn recv(&mut self) -> Option<schema::RequestFrame>;
138+
fn recv(&mut self) -> impl Future<Output = Option<schema::RequestFrame>> + Send;
133139
}
134140

135141
pub async fn spawn_transport_by_config<P: ExecutorProviderInterface>(

0 commit comments

Comments
 (0)