Skip to content

Commit a96bb73

Browse files
committed
Add gRPC connect timeout on LeaderManager::connect
Refactors the related code to plumb the value through to the grpc builder Config is currently limited to the controller with a default of None. This _should_ only affect the Controller -> Leader connection, which currently does not have any supervisor to detect issues. Most (if not all) other connections have some supervisor that will timeout and handle the error accordingly.
1 parent 20b2805 commit a96bb73

9 files changed

Lines changed: 54 additions & 22 deletions

File tree

crates/arroyo-api/src/jobs.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ async fn fetch_from_leader<
5959
&config().api.tls,
6060
leader_context.worker_id,
6161
leader_context.rpc_address,
62+
None,
6263
)
6364
.await
6465
.map_err(|_| ErrorResp {

crates/arroyo-api/src/lib.rs

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -124,12 +124,13 @@ pub async fn compiler_service() -> Result<CompilerGrpcClient<Channel>, ErrorResp
124124
let config = config();
125125
let endpoint = config.compiler_endpoint();
126126

127-
let channel = arroyo_rpc::connect_grpc("api", endpoint, &config.api.tls, &config.compiler.tls)
128-
.await
129-
.map_err(|e| {
130-
error!("Failed to connect to compiler service: {}", e);
131-
service_unavailable("compiler-service")
132-
})?;
127+
let channel =
128+
arroyo_rpc::connect_grpc("api", endpoint, &config.api.tls, &config.compiler.tls, None)
129+
.await
130+
.map_err(|e| {
131+
error!("Failed to connect to compiler service: {}", e);
132+
service_unavailable("compiler-service")
133+
})?;
133134

134135
Ok(CompilerGrpcClient::new(channel))
135136
}

crates/arroyo-controller/src/job_controller/leader_manager.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,15 @@ impl LeaderManager {
3333
generation: u64,
3434
worker_id: WorkerId,
3535
address: String,
36+
connect_timeout: Option<Duration>,
3637
) -> anyhow::Result<Self> {
3738
let leader_client = retry!(
3839
job_status_client(
3940
"controller",
4041
&config().worker.tls,
4142
worker_id,
42-
address.clone()
43+
address.clone(),
44+
connect_timeout,
4345
)
4446
.await,
4547
5,

crates/arroyo-controller/src/schedulers/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -605,6 +605,7 @@ impl NodeScheduler {
605605
format!("http://{}", node.addr),
606606
&config().controller.tls,
607607
&config().node.tls,
608+
None,
608609
)
609610
.await?;
610611
Ok(NodeGrpcClient::new(channel))

crates/arroyo-controller/src/states/mod.rs

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -927,7 +927,7 @@ pub(crate) async fn state_backoff(retries_attempted: usize, job_id: &str, pipeli
927927

928928
#[allow(clippy::too_many_arguments)]
929929
async fn run_to_completion(
930-
config: Arc<RwLock<(JobConfig, AppliedStatus)>>,
930+
job_config_and_status: Arc<RwLock<(JobConfig, AppliedStatus)>>,
931931
pipeline_info: Arc<PipelineInfo>,
932932
mut program: LogicalProgram,
933933
mut status: JobStatus,
@@ -937,7 +937,7 @@ async fn run_to_completion(
937937
scheduler: Arc<dyn Scheduler>,
938938
metrics: Arc<tokio::sync::RwLock<HashMap<Arc<String>, JobMetrics>>>,
939939
) {
940-
let job_config = config.read().unwrap().0.clone();
940+
let job_config = job_config_and_status.read().unwrap().0.clone();
941941

942942
let leader_manager = if let Some(ctx) = &status.state_context.leader {
943943
LeaderManager::connect(
@@ -946,6 +946,7 @@ async fn run_to_completion(
946946
ctx.generation,
947947
ctx.worker_id,
948948
ctx.rpc_address.clone(),
949+
config().controller.connect_timeout.as_deref().copied(),
949950
)
950951
.await
951952
.map(Some)
@@ -977,7 +978,7 @@ async fn run_to_completion(
977978
};
978979

979980
loop {
980-
config.write().unwrap().1 = AppliedStatus::Applied;
981+
job_config_and_status.write().unwrap().1 = AppliedStatus::Applied;
981982
match execute_state(state, ctx).await {
982983
(Some(new_state), new_ctx) => {
983984
state = new_state;
@@ -986,7 +987,7 @@ async fn run_to_completion(
986987
(None, _) => break,
987988
}
988989

989-
ctx.config = config.read().unwrap().0.clone();
990+
ctx.config = job_config_and_status.read().unwrap().0.clone();
990991
}
991992
}
992993

crates/arroyo-controller/src/states/scheduling.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,7 @@ async fn handle_worker_connect<'a>(
160160
rpc_address.clone(),
161161
&config().controller.tls,
162162
&config().worker.tls,
163+
None,
163164
)
164165
.await
165166
.unwrap()
@@ -935,6 +936,7 @@ impl State for Scheduling {
935936
ctx.status.generation,
936937
id,
937938
addr,
939+
config().controller.connect_timeout.as_deref().copied(),
938940
).await
939941
&& let Ok(status) = leader_manager.poll_leader_status().await {
940942
match JobState::try_from(status.job_state) {
@@ -1031,6 +1033,7 @@ impl State for Scheduling {
10311033
ctx.status.generation,
10321034
id,
10331035
addr.clone(),
1036+
config().controller.connect_timeout.as_deref().copied(),
10341037
)
10351038
.await
10361039
{

crates/arroyo-rpc/src/config.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -401,6 +401,10 @@ pub struct ControllerConfig {
401401
/// Poll interval for leader status
402402
pub leader_poll_interval: HumanReadableDuration,
403403

404+
/// Timeout for connecting to gRPC services
405+
#[serde(default)]
406+
pub connect_timeout: Option<HumanReadableDuration>,
407+
404408
/// Metric system configurations
405409
pub metrics: MetricsConfig,
406410
}

crates/arroyo-rpc/src/lib.rs

Lines changed: 29 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1002,9 +1002,10 @@ pub async fn grpc_channel_builder(
10021002
endpoint: String,
10031003
our_tls: &Option<TlsConfig>,
10041004
target_tls: &Option<TlsConfig>,
1005+
connect_timeout: Option<Duration>,
10051006
) -> Result<Endpoint> {
10061007
let config = config();
1007-
if let Some(target_tls) = config.get_tls_config(target_tls) {
1008+
let endpoint = if let Some(target_tls) = config.get_tls_config(target_tls) {
10081009
let mut endpoint = Url::parse(&endpoint)?;
10091010
endpoint
10101011
.set_scheme("https")
@@ -1031,11 +1032,17 @@ pub async fn grpc_channel_builder(
10311032
config_builder = config_builder.identity(Identity::from_pem(our_tls.cert, our_tls.key));
10321033
}
10331034

1034-
Ok(b.tls_config(config_builder).context("configuring TLS")?)
1035+
b.tls_config(config_builder).context("configuring TLS")?
10351036
} else {
10361037
debug!("connecting to grpc endpoint {endpoint}");
1037-
Ok(Channel::from_shared(endpoint.to_string())?)
1038-
}
1038+
Channel::from_shared(endpoint.to_string())?
1039+
};
1040+
1041+
Ok(if let Some(connect_timeout) = connect_timeout {
1042+
endpoint.connect_timeout(connect_timeout)
1043+
} else {
1044+
endpoint
1045+
})
10391046
}
10401047

10411048
/// Connect to a gRPC service with optional TLS
@@ -1044,17 +1051,20 @@ pub async fn connect_grpc(
10441051
endpoint: String,
10451052
our_tls: &Option<TlsConfig>,
10461053
tls: &Option<TlsConfig>,
1054+
connect_timeout: Option<Duration>,
10471055
) -> Result<Channel> {
1048-
Ok(grpc_channel_builder(our_name, endpoint, our_tls, tls)
1049-
.await?
1050-
.connect()
1051-
.await?)
1056+
Ok(
1057+
grpc_channel_builder(our_name, endpoint, our_tls, tls, connect_timeout)
1058+
.await?
1059+
.connect()
1060+
.await?,
1061+
)
10521062
}
10531063

10541064
/// Connect a raw gRPC channel to the controller endpoint.
10551065
pub async fn connect_controller(our_name: &str, our_tls: &Option<TlsConfig>) -> Result<Channel> {
10561066
let endpoint = config().controller_endpoint();
1057-
connect_grpc(our_name, endpoint, our_tls, &config().controller.tls).await
1067+
connect_grpc(our_name, endpoint, our_tls, &config().controller.tls, None).await
10581068
}
10591069

10601070
pub async fn controller_client(
@@ -1077,7 +1087,7 @@ pub async fn job_controller_client(
10771087
&config().controller.tls
10781088
};
10791089

1080-
let channel = connect_grpc(our_name, addr, our_tls, their_tls).await?;
1090+
let channel = connect_grpc(our_name, addr, our_tls, their_tls, None).await?;
10811091
Ok(job_controller_grpc_client::JobControllerGrpcClient::new(
10821092
channel,
10831093
))
@@ -1088,9 +1098,17 @@ pub async fn job_status_client(
10881098
our_tls: &Option<TlsConfig>,
10891099
worker_id: WorkerId,
10901100
addr: String,
1101+
connect_timeout: Option<Duration>,
10911102
) -> Result<job_status_grpc_client::JobStatusGrpcClient<InterceptedService<Channel, InjectWorkerId>>>
10921103
{
1093-
let channel = connect_grpc(our_name, addr, our_tls, &config().worker.tls).await?;
1104+
let channel = connect_grpc(
1105+
our_name,
1106+
addr,
1107+
our_tls,
1108+
&config().worker.tls,
1109+
connect_timeout,
1110+
)
1111+
.await?;
10941112
Ok(
10951113
job_status_grpc_client::JobStatusGrpcClient::with_interceptor(
10961114
channel,

crates/arroyo-worker/src/job_controller/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -317,6 +317,7 @@ pub(crate) async fn connect_to_worker(id: WorkerId, addr: String) -> anyhow::Res
317317
addr.clone(),
318318
&config().worker.tls,
319319
&config().worker.tls,
320+
None,
320321
)
321322
.await?
322323
.timeout(Duration::from_secs(15))

0 commit comments

Comments
 (0)