Skip to content

A0-1663: remove unnecessary session start retries in connection manager #803

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 13, 2022
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
73 changes: 8 additions & 65 deletions finality-aleph/src/network/manager/service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ struct Session<D: Data, M: Data, A: AddressingInformation + TryFrom<Vec<M>> + In
}

#[derive(Clone)]
/// Stores all data needed for starting validator session
struct PreValidatorSession {
session_id: SessionId,
verifier: AuthorityVerifier,
Expand All @@ -55,26 +56,12 @@ struct PreValidatorSession {
}

#[derive(Clone)]
/// Stores all data needed for starting non-validator session
struct PreNonvalidatorSession {
session_id: SessionId,
verifier: AuthorityVerifier,
}

#[derive(Clone)]
enum PreSession {
Validator(PreValidatorSession),
Nonvalidator(PreNonvalidatorSession),
}

impl PreSession {
fn session_id(&self) -> SessionId {
match self {
Self::Validator(pre_session) => pre_session.session_id,
Self::Nonvalidator(pre_session) => pre_session.session_id,
}
}
}

/// Configuration for the session manager service. Controls how often the maintenance and
/// rebroadcasts are triggerred. Also controls when maintenance starts.
pub struct Config {
Expand Down Expand Up @@ -145,10 +132,6 @@ where
network_identity: NI,
connections: Connections<NI::PeerId>,
sessions: HashMap<SessionId, Session<D, M, NI::AddressingInformation>>,
to_retry: Vec<(
PreSession,
Option<oneshot::Sender<mpsc::UnboundedReceiver<D>>>,
)>,
discovery_cooldown: Duration,
maintenance_period: Duration,
initial_delay: Duration,
Expand All @@ -169,7 +152,6 @@ where
network_identity,
connections: Connections::new(),
sessions: HashMap::new(),
to_retry: Vec::new(),
discovery_cooldown,
maintenance_period,
initial_delay,
Expand All @@ -190,8 +172,6 @@ where
session_id: SessionId,
) -> Option<ConnectionCommand<NI::AddressingInformation>> {
self.sessions.remove(&session_id);
self.to_retry
.retain(|(pre_session, _)| pre_session.session_id() != session_id);
Self::delete_reserved(self.connections.remove_session(session_id))
}

Expand Down Expand Up @@ -307,21 +287,16 @@ where
pre_session: PreValidatorSession,
result_for_user: Option<oneshot::Sender<mpsc::UnboundedReceiver<D>>>,
) -> Result<ServiceActions<M, NI::AddressingInformation>, SessionHandlerError> {
match self.update_validator_session(pre_session.clone()).await {
Ok((actions, data_from_network)) => {
self.update_validator_session(pre_session)
.await
.map(|(actions, data_from_network)| {
if let Some(result_for_user) = result_for_user {
if result_for_user.send(data_from_network).is_err() {
warn!(target: "aleph-network", "Failed to send started session.")
}
}
Ok(actions)
}
Err(e) => {
self.to_retry
.push((PreSession::Validator(pre_session), result_for_user));
Err(e)
}
}
actions
})
}

async fn start_nonvalidator_session(
Expand Down Expand Up @@ -368,13 +343,7 @@ where
&mut self,
pre_session: PreNonvalidatorSession,
) -> Result<(), SessionHandlerError> {
self.update_nonvalidator_session(pre_session.clone())
.await
.map_err(|e| {
self.to_retry
.push((PreSession::Nonvalidator(pre_session), None));
e
})
self.update_nonvalidator_session(pre_session).await
}

/// Handle a session command.
Expand Down Expand Up @@ -495,28 +464,6 @@ where
}
}

/// Retries starting a validator session the user requested, but which failed to start
/// initially. Mostly useful when the network was not yet aware of its own address at time of
/// the request.
pub async fn retry_session_start(
&mut self,
) -> Result<ServiceActions<M, NI::AddressingInformation>, SessionHandlerError> {
let (pre_session, result_for_user) = match self.to_retry.pop() {
Some(to_retry) => to_retry,
None => return Ok(ServiceActions::noop()),
};
match pre_session {
PreSession::Validator(pre_session) => {
self.handle_validator_presession(pre_session, result_for_user)
.await
}
PreSession::Nonvalidator(pre_session) => {
self.handle_nonvalidator_presession(pre_session).await?;
Ok(ServiceActions::noop())
}
}
}

pub fn status_report(&self) {
let mut status = String::from("Connection Manager status report: ");

Expand Down Expand Up @@ -759,10 +706,6 @@ where
},
_ = maintenance.tick() => {
debug!(target: "aleph-network", "Manager starts maintenence");
match service.retry_session_start().await {
Ok(to_send) => self.handle_service_actions(to_send)?,
Err(e) => warn!(target: "aleph-network", "Retry failed to update handler: {:?}", e),
}
for to_send in service.discovery() {
self.send_authentications(to_send.into())?;
}
Expand Down