Skip to content

Commit a3e8644

Browse files
eliothedemanfacebook-github-bot
authored andcommitted
Replace tuples with named structures in net.rs (#731)
Summary: Pull Request resolved: #731 Reviewed By: mariusae, highker, vidhyav Differential Revision: D78846166 fbshipit-source-id: 50de45135039ef30d60a00dd3481a5fc54e2749c
1 parent 39a556a commit a3e8644

File tree

1 file changed

+38
-25
lines changed

1 file changed

+38
-25
lines changed

hyperactor/src/channel/net.rs

Lines changed: 38 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -209,13 +209,21 @@ impl<M: RemoteMessage> NetTx<M> {
209209
// If we can't deliver a message within this limit consider
210210
// `link` broken and return.
211211

212+
#[derive(Debug)]
213+
struct QueuedMessage<M: RemoteMessage> {
214+
seq: u64,
215+
data: Bytes,
216+
received_at: Instant,
217+
return_channel: oneshot::Sender<M>,
218+
}
219+
212220
#[derive(Debug)]
213221
struct Outbox<'a, M: RemoteMessage> {
214222
// The seq number of the next new message put into outbox. Requeued
215223
// unacked messages should still use their already assigned seq
216224
// numbers.
217225
next_seq: u64,
218-
deque: VecDeque<(u64, Bytes, Instant, oneshot::Sender<M>)>,
226+
deque: VecDeque<QueuedMessage<M>>,
219227
log_id: &'a str,
220228
}
221229

@@ -231,8 +239,9 @@ impl<M: RemoteMessage> NetTx<M> {
231239
fn is_expired(&self) -> bool {
232240
match self.deque.front() {
233241
None => false,
234-
Some((_, _, since, _)) => {
235-
since.elapsed() > config::global::get(config::MESSAGE_DELIVERY_TIMEOUT)
242+
Some(msg) => {
243+
msg.received_at.elapsed()
244+
> config::global::get(config::MESSAGE_DELIVERY_TIMEOUT)
236245
}
237246
}
238247
}
@@ -256,17 +265,17 @@ impl<M: RemoteMessage> NetTx<M> {
256265
self.log_id,
257266
)
258267
})?
259-
.1
268+
.data
260269
.clone();
261270
sink.send(data).await.map_err(|e| e.to_string())?;
262271
Ok(())
263272
}
264273

265274
fn front_size(&self) -> Option<usize> {
266-
self.deque.front().map(|(_, bytes, _, _)| bytes.len())
275+
self.deque.front().map(|msg| msg.data.len())
267276
}
268277

269-
fn pop_front(&mut self) -> Option<(u64, Bytes, Instant, oneshot::Sender<M>)> {
278+
fn pop_front(&mut self) -> Option<QueuedMessage<M>> {
270279
self.deque.pop_front()
271280
}
272281

@@ -275,7 +284,7 @@ impl<M: RemoteMessage> NetTx<M> {
275284
(message, return_channel, received_at): (M, oneshot::Sender<M>, Instant),
276285
) -> Result<(), String> {
277286
assert!(
278-
self.deque.back().is_none_or(|msg| msg.0 < self.next_seq),
287+
self.deque.back().is_none_or(|msg| msg.seq < self.next_seq),
279288
"{}: unexpected: seq should be in ascending order, but got {:?} vs {}",
280289
self.log_id,
281290
self.deque.back(),
@@ -287,8 +296,12 @@ impl<M: RemoteMessage> NetTx<M> {
287296
.map_err(|e| format!("serialization error: {e}"))?
288297
.into();
289298
REMOTE_MESSAGE_SEND_SIZE.record(data.len() as f64, &[]);
290-
self.deque
291-
.push_back((self.next_seq, data, received_at, return_channel));
299+
self.deque.push_back(QueuedMessage {
300+
seq: self.next_seq,
301+
data,
302+
received_at,
303+
return_channel,
304+
});
292305
self.next_seq += 1;
293306
Ok(())
294307
}
@@ -297,11 +310,11 @@ impl<M: RemoteMessage> NetTx<M> {
297310
match (unacked.deque.back(), self.deque.front()) {
298311
(Some(last), Some(first)) => {
299312
assert!(
300-
last.0 < first.0,
313+
last.seq < first.seq,
301314
"{}: seq should be in ascending order, but got {} vs {:?}",
302315
self.log_id,
303-
last.0,
304-
first.0,
316+
last.seq,
317+
first.seq,
305318
);
306319
}
307320
_ => (),
@@ -315,7 +328,7 @@ impl<M: RemoteMessage> NetTx<M> {
315328

316329
#[derive(Debug)]
317330
struct Unacked<'a, M: RemoteMessage> {
318-
deque: VecDeque<(u64, Bytes, Instant, oneshot::Sender<M>)>,
331+
deque: VecDeque<QueuedMessage<M>>,
319332
largest_acked: Option<u64>,
320333
log_id: &'a str,
321334
}
@@ -329,13 +342,13 @@ impl<M: RemoteMessage> NetTx<M> {
329342
}
330343
}
331344

332-
fn push_back(&mut self, message: (u64, Bytes, Instant, oneshot::Sender<M>)) {
345+
fn push_back(&mut self, message: QueuedMessage<M>) {
333346
assert!(
334-
self.deque.back().is_none_or(|msg| msg.0 < message.0),
347+
self.deque.back().is_none_or(|msg| msg.seq < message.seq),
335348
"{}: seq should be in ascending order, but got {:?} vs {}",
336349
self.log_id,
337350
self.deque.back(),
338-
message.0
351+
message.seq
339352
);
340353

341354
if let Some(largest) = self.largest_acked {
@@ -370,7 +383,7 @@ impl<M: RemoteMessage> NetTx<M> {
370383
// Tx resends. As a result, this message's ack would be
371384
// recorded already by `largest_acked` before it is put into
372385
// unacked queue.
373-
if message.0 <= largest {
386+
if message.seq <= largest {
374387
// since the message is already delivered and acked, it
375388
// does need to be put in the queue again.
376389
return;
@@ -392,8 +405,8 @@ impl<M: RemoteMessage> NetTx<M> {
392405

393406
self.largest_acked = Some(acked);
394407
let deque = &mut self.deque;
395-
while let Some((seq, _, _, _)) = deque.front() {
396-
if *seq <= acked {
408+
while let Some(msg) = deque.front() {
409+
if msg.seq <= acked {
397410
deque.pop_front();
398411
} else {
399412
// Messages in the deque are orderd by seq in ascending
@@ -407,7 +420,7 @@ impl<M: RemoteMessage> NetTx<M> {
407420
fn is_expired(&self) -> bool {
408421
matches!(
409422
self.deque.front(),
410-
Some((_, _, received_at, _)) if received_at.elapsed() > config::global::get(config::MESSAGE_DELIVERY_TIMEOUT)
423+
Some(msg) if msg.received_at.elapsed() > config::global::get(config::MESSAGE_DELIVERY_TIMEOUT)
411424
)
412425
}
413426

@@ -416,10 +429,10 @@ impl<M: RemoteMessage> NetTx<M> {
416429
/// branches.
417430
async fn wait_for_timeout(&self) {
418431
match self.deque.front() {
419-
Some((_, _, received_at, _)) => {
432+
Some(msg) => {
420433
RealClock
421434
.sleep_until(
422-
received_at.clone()
435+
msg.received_at.clone()
423436
+ config::global::get(config::MESSAGE_DELIVERY_TIMEOUT),
424437
)
425438
.await
@@ -728,11 +741,11 @@ impl<M: RemoteMessage> NetTx<M> {
728741
.deque
729742
.drain(..)
730743
.chain(outbox.deque.drain(..))
731-
.filter_map(|(_, bytes, _, return_channel)| {
732-
bincode::deserialize(&bytes)
744+
.filter_map(|queued_msg| {
745+
bincode::deserialize(&queued_msg.data)
733746
.ok()
734747
.and_then(|frame| match frame {
735-
Frame::Message(_, msg) => Some((return_channel, msg)),
748+
Frame::Message(_, msg) => Some((queued_msg.return_channel, msg)),
736749
_ => None,
737750
})
738751
})

0 commit comments

Comments
 (0)