Skip to content

Commit 4585879

Browse files
committed
fix(millstone): retry initial gRPC connections
Retry eager gRPC channel connections with bounded exponential backoff so Millstone does not fail when a target receiver becomes ready shortly after its container health check passes. Preserve the final transport error chain after retries and cover both retry recovery and error propagation with unit tests.
1 parent 9807185 commit 4585879

2 files changed

Lines changed: 88 additions & 12 deletions

File tree

bin/correctness/millstone/Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ rand = { workspace = true, features = ["std_rng"] }
1818
saluki-error = { workspace = true }
1919
serde = { workspace = true }
2020
serde_yaml = { workspace = true }
21-
tokio = { workspace = true, features = ["rt-multi-thread"] }
21+
tokio = { workspace = true, features = ["rt-multi-thread", "time"] }
2222
tonic = { workspace = true }
2323
tracing = { workspace = true }
2424
tracing-subscriber = { workspace = true, features = [

bin/correctness/millstone/src/target.rs

Lines changed: 87 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
use std::os::unix::net::{UnixDatagram, UnixStream};
33
use std::{
44
fs::File,
5+
future::Future,
56
io::Write as _,
67
net::{Ipv4Addr, TcpStream, UdpSocket},
78
path::Path,
@@ -15,6 +16,9 @@ use tracing::warn;
1516

1617
use crate::config::{Config, TargetAddress};
1718

19+
const MAX_RETRIES: u32 = 5;
20+
const BASE_RETRY_DELAY_MS: u64 = 100;
21+
1822
enum TargetBackend {
1923
Tcp(TcpStream),
2024
Udp(UdpSocket),
@@ -186,14 +190,11 @@ async fn try_grpc_unary(channel: Channel, service_method_path: &str, payload: &[
186190
fn send_grpc_payload(
187191
runtime: &tokio::runtime::Runtime, channel: Channel, service_method_path: &str, payload: &[u8],
188192
) -> Result<(), GenericError> {
189-
const MAX_RETRIES: u32 = 5;
190-
const BASE_DELAY_MS: u64 = 100;
191-
192193
let mut last_status: Option<tonic::Status> = None;
193194

194195
for attempt in 0..=MAX_RETRIES {
195196
if let Some(ref status) = last_status {
196-
let delay_ms = BASE_DELAY_MS * (1u64 << attempt.saturating_sub(1));
197+
let delay_ms = retry_delay_ms(attempt);
197198
warn!(
198199
attempt,
199200
delay_ms,
@@ -221,6 +222,40 @@ fn send_grpc_payload(
221222
))
222223
}
223224

225+
const fn retry_delay_ms(attempt: u32) -> u64 {
226+
BASE_RETRY_DELAY_MS * (1u64 << attempt.saturating_sub(1))
227+
}
228+
229+
async fn retry_with_backoff<T, E, F, Fut>(operation_name: &str, mut operation: F) -> Result<T, E>
230+
where
231+
E: std::fmt::Display,
232+
F: FnMut() -> Fut,
233+
Fut: Future<Output = Result<T, E>>,
234+
{
235+
let mut last_error = None;
236+
237+
for attempt in 0..=MAX_RETRIES {
238+
if let Some(ref error) = last_error {
239+
let delay_ms = retry_delay_ms(attempt);
240+
warn!(
241+
operation_name,
242+
attempt,
243+
delay_ms,
244+
error = %error,
245+
"Operation failed, retrying after backoff."
246+
);
247+
tokio::time::sleep(Duration::from_millis(delay_ms)).await;
248+
}
249+
250+
match operation().await {
251+
Ok(value) => return Ok(value),
252+
Err(error) => last_error = Some(error),
253+
}
254+
}
255+
256+
Err(last_error.expect("retry loop always makes at least one attempt"))
257+
}
258+
224259
/// Creates a generic gRPC backend with a tokio runtime for the given gRPC URL.
225260
///
226261
/// The URL should be in the format: `<host>:<port>/<service>/<method>`.
@@ -238,14 +273,14 @@ fn create_grpc_client(url: &str) -> Result<(TargetBackend, Option<tokio::runtime
238273
let runtime = tokio::runtime::Runtime::new().error_context("Failed to create tokio runtime for gRPC client.")?;
239274
let endpoint = format!("http://{}", host_and_port);
240275

276+
let grpc_endpoint = Channel::from_shared(endpoint.clone())
277+
.map_err(|e| saluki_error::generic_error!("Invalid gRPC endpoint: {}", e))?;
241278
let channel = runtime
242-
.block_on(async {
243-
Channel::from_shared(endpoint.clone())
244-
.map_err(|e| saluki_error::generic_error!("Invalid gRPC endpoint: {}", e))?
245-
.connect()
246-
.await
247-
.map_err(|e| saluki_error::generic_error!("Failed to connect to gRPC endpoint: {}", e))
248-
})
279+
.block_on(retry_with_backoff("connect to gRPC endpoint", || {
280+
let grpc_endpoint = grpc_endpoint.clone();
281+
async move { grpc_endpoint.connect().await }
282+
}))
283+
.error_context("Failed to connect to gRPC endpoint.")
249284
.with_error_context(|| format!("Failed to connect to gRPC target '{}'.", endpoint))?;
250285

251286
let backend = GrpcBackend {
@@ -306,3 +341,44 @@ impl tonic::codec::Decoder for NoopDecoder {
306341
Ok(Some(Bytes::from(bytes)))
307342
}
308343
}
344+
345+
#[cfg(test)]
346+
mod tests {
347+
use std::sync::atomic::{AtomicUsize, Ordering};
348+
349+
use super::{create_grpc_client, retry_with_backoff};
350+
351+
#[test]
352+
fn preserves_the_final_grpc_connection_error() {
353+
let error = match create_grpc_client("127.0.0.1:0/test.Service/Call") {
354+
Ok(_) => panic!("connection should fail"),
355+
Err(error) => error,
356+
};
357+
let error_chain = error.chain().map(ToString::to_string).collect::<Vec<_>>();
358+
359+
assert!(
360+
error_chain.iter().any(|cause| cause == "tcp connect error"),
361+
"expected TCP connection failure in error chain, got: {error_chain:?}"
362+
);
363+
}
364+
365+
#[test]
366+
fn retries_transient_failures_until_the_operation_succeeds() {
367+
let attempts = AtomicUsize::new(0);
368+
let runtime = tokio::runtime::Runtime::new().expect("runtime should be created");
369+
370+
let result = runtime.block_on(retry_with_backoff("test operation", || {
371+
let attempt = attempts.fetch_add(1, Ordering::Relaxed);
372+
async move {
373+
if attempt < 2 {
374+
Err("target is not ready")
375+
} else {
376+
Ok("connected")
377+
}
378+
}
379+
}));
380+
381+
assert_eq!(result, Ok("connected"));
382+
assert_eq!(attempts.load(Ordering::Relaxed), 3);
383+
}
384+
}

0 commit comments

Comments
 (0)