Skip to content

Commit

Permalink
fix(comms): always send disconnect notification to conn manager
Browse files Browse the repository at this point in the history
  • Loading branch information
sdbondi committed Aug 30, 2024
1 parent 169572a commit 51f3057
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 15 deletions.
1 change: 0 additions & 1 deletion comms/core/src/connection_manager/peer_connection.rs
Original file line number Diff line number Diff line change
Expand Up @@ -532,7 +532,6 @@ impl PeerConnectionActor {
self.peer_node_id.short_str(),
e
);
return Ok(());
},
e => trace!(target: LOG_TARGET, "On disconnect: ({})", e),
}
Expand Down
8 changes: 7 additions & 1 deletion comms/core/src/connectivity/connection_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,12 @@ pub enum ConnectionStatus {
Disconnected(Minimized),
}

impl ConnectionStatus {
pub fn is_connected(&self) -> bool {
matches!(self, ConnectionStatus::Connected)
}
}

impl fmt::Display for ConnectionStatus {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
write!(f, "{:?}", self)
Expand All @@ -58,7 +64,7 @@ impl PeerConnectionState {

/// Return true if the underlying connection exists and is connected, otherwise false
pub fn is_connected(&self) -> bool {
self.connection().filter(|c| c.is_connected()).is_some()
self.status.is_connected() && self.connection().map_or(false, |c| c.is_connected())
}

pub fn connection_mut(&mut self) -> Option<&mut PeerConnection> {
Expand Down
37 changes: 24 additions & 13 deletions comms/core/src/connectivity/manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,11 +188,9 @@ impl ConnectivityManagerActor {
self.handle_request(req).await;
},

event = connection_manager_events.recv() => {
if let Ok(event) = event {
if let Err(err) = self.handle_connection_manager_event(&event).await {
error!(target:LOG_TARGET, "Error handling connection manager event: {:?}", err);
}
Ok(event) = connection_manager_events.recv() => {
if let Err(err) = self.handle_connection_manager_event(&event).await {
error!(target:LOG_TARGET, "Error handling connection manager event: {:?}", err);
}
},

Expand Down Expand Up @@ -738,24 +736,37 @@ impl ConnectivityManagerActor {
}

async fn on_new_connection(&mut self, new_conn: &PeerConnection) -> TieBreak {
match self.pool.get_connection(new_conn.peer_node_id()).cloned() {
Some(existing_conn) if !existing_conn.is_connected() => {
match self.pool.get(new_conn.peer_node_id()).cloned() {
Some(existing_state) if !existing_state.is_connected() => {
debug!(
target: LOG_TARGET,
"Tie break: Existing connection (id: {}, peer: {}, direction: {}) was not connected, resolving \
tie break by using the new connection. (New: id: {}, peer: {}, direction: {})",
existing_conn.id(),
existing_conn.peer_node_id(),
existing_conn.direction(),
existing_state.connection().map(|c| c.id()).unwrap_or_default(),
existing_state.node_id(),
existing_state.connection().map(|c| c.direction().as_str()).unwrap_or("--"),
new_conn.id(),
new_conn.peer_node_id(),
new_conn.direction(),
);
self.pool.remove(existing_conn.peer_node_id());
self.pool.remove(existing_state.node_id());
TieBreak::UseNew
},
Some(mut existing_conn) => {
if self.tie_break_existing_connection(&existing_conn, new_conn) {
Some(mut existing_state) => {
let Some(existing_conn) = existing_state.connection_mut() else {
error!(
target: LOG_TARGET,
"INVARIANT ERROR in Tie break: PeerConnection is None but state is CONNECTED: Existing connection (id: {}, peer: {}, direction: {}), new connection. (id: {}, peer: {}, direction: {})",
existing_state.connection().map(|c| c.id()).unwrap_or_default(),
existing_state.node_id(),
existing_state.connection().map(|c| c.direction().as_str()).unwrap_or("--"),
new_conn.id(),
new_conn.peer_node_id(),
new_conn.direction(),
);
return TieBreak::UseNew;
};
if self.tie_break_existing_connection(existing_conn, new_conn) {
warn!(
target: LOG_TARGET,
"Tie break: Keep new connection (id: {}, peer: {}, direction: {}). Disconnect existing \
Expand Down

0 comments on commit 51f3057

Please sign in to comment.