Skip to content

Commit 38772b4

Browse files
committed
Don't panic if we cannot obtain connection attemp result
For logging purposes we try to obtain the result of the connection attempt in a synchronous function if the connection attempt task has finished. Unfortunately, we cannot rely on TaskHandle::is_finished for task_handle.now_or_never() to always return Some(...). The problem is that the underlying Tokio JoinHandle participates in the cooperative task scheduling and if we ran out of budget, then it would return Poll::Pending. That's why we ignore the result in this case and continue. This fixes #2879.
1 parent f53f105 commit 38772b4

File tree

1 file changed

+12
-7
lines changed

1 file changed

+12
-7
lines changed

crates/metadata-server/src/raft/network/networking.rs

+12-7
Original file line numberDiff line numberDiff line change
@@ -103,23 +103,28 @@ where
103103
.try_send(network_message)
104104
.map_err(|_err| TrySendError::Send(message))?;
105105
} else if let Some(address) = self.addresses.get(&target) {
106-
if let Some(task_handle) = self.connection_attempts.remove(&target) {
107-
if !task_handle.is_finished() {
106+
if let Entry::Occupied(occupied) = self.connection_attempts.entry(target) {
107+
if !occupied.get().is_finished() {
108108
return Err(TrySendError::Connecting(message));
109109
} else {
110-
match task_handle.now_or_never().expect("should be finished") {
111-
Ok(result) => match result {
110+
match occupied.remove().now_or_never() {
111+
None => {
112+
trace!(
113+
"Previous connection attempt to '{target}' finished. Polling the final \
114+
result failed because we most likely depleted our cooperative task budget."
115+
);
116+
}
117+
Some(Ok(result)) => match result {
112118
Ok(_) => trace!(
113119
"Previous connection attempt to '{target}' succeeded but connection was closed in meantime."
114120
),
115121
Err(err) => {
116122
trace!("Previous connection attempt to '{target}' failed: {}", err)
117123
}
118124
},
119-
Err(err) => {
125+
Some(Err(_)) => {
120126
trace!(
121-
"Previous connection attempt to '{target}' panicked: {}",
122-
err
127+
"Previous connection attempt to '{target}' panicked. The panic is swallowed by the TaskHandle abstraction.",
123128
)
124129
}
125130
}

0 commit comments

Comments
 (0)