Skip to content
This repository has been archived by the owner on Jun 25, 2021. It is now read-only.

Refactor Node state: extract EstablishingNode state #1616

Merged
merged 15 commits into from
May 9, 2019
Merged
Show file tree
Hide file tree
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
9 changes: 5 additions & 4 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use crate::messages::{Request, CLIENT_GET_PRIORITY, DEFAULT_PRIORITY};
use crate::outbox::{EventBox, EventBuf};
use crate::routing_table::Authority;
use crate::state_machine::{State, StateMachine};
use crate::states::{Bootstrapping, BootstrappingTargetState};
use crate::states::{BootstrappingPeer, TargetState};
use crate::types::{MessageId, RoutingActionSender};
use crate::xor_name::XorName;
use crate::{BootstrapConfig, MIN_SECTION_SIZE};
Expand Down Expand Up @@ -72,16 +72,17 @@ impl Client {

StateMachine::new(
move |action_sender, crust_service, timer, _outbox2| {
Bootstrapping::new(
BootstrappingPeer::new(
action_sender,
Box::new(NullCache),
BootstrappingTargetState::Client { msg_expiry_dur },
TargetState::Client { msg_expiry_dur },
crust_service,
full_id,
min_section_size,
timer,
)
.map_or(State::Terminated, State::Bootstrapping)
.map(State::BootstrappingPeer)
.unwrap_or(State::Terminated)
},
pub_id,
bootstrap_config,
Expand Down
10 changes: 7 additions & 3 deletions src/mock_parsec/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -184,10 +184,14 @@ where
unimplemented!()
}

// TODO: rename this to `has_unpolled_observations`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't done yet because you would want to update Parsec to one that exposes the new API in the same PR?
I would add this to the comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I'd like to make the change in Parsec first, to avoid the situation where routing becomes uncompilable with --features=mock_base (that is, with real parsec).

pub fn has_unconsensused_observations(&self) -> bool {
self.observations
.iter()
.any(|(_, obs)| obs.state != ConsensusState::Polled)
state::with::<T, S::PublicId, _, _>(self.section_hash, |state| {
state
.observations
.iter()
.any(|(_, observation_state)| !observation_state.consensused())
}) || self.our_unpolled_observations().next().is_some()
}

pub fn our_unpolled_observations(&self) -> impl Iterator<Item = &Observation<T, S::PublicId>> {
Expand Down
4 changes: 4 additions & 0 deletions src/mock_parsec/observation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,10 @@ impl<P: PublicId> ObservationState<P> {
}
}

pub fn consensused(&self) -> bool {
self.consensused
}

fn compute_consensus<T: NetworkEvent>(
&mut self,
peers: &BTreeSet<P>,
Expand Down
27 changes: 10 additions & 17 deletions src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use crate::messages::{
use crate::outbox::{EventBox, EventBuf};
use crate::routing_table::Authority;
use crate::state_machine::{State, StateMachine};
use crate::states::{self, Bootstrapping, BootstrappingTargetState};
use crate::states::{self, BootstrappingPeer, TargetState};
use crate::types::{MessageId, RoutingActionSender};
use crate::xor_name::XorName;
#[cfg(feature = "mock_base")]
Expand Down Expand Up @@ -141,34 +141,27 @@ impl NodeBuilder {
StateMachine::new(
move |action_sender, crust_service, timer, outbox2| {
if self.first {
if let Some(state) = states::Node::first(
self.cache,
crust_service,
full_id,
min_section_size,
timer,
) {
State::Node(state)
} else {
State::Terminated
}
states::Node::first(self.cache, crust_service, full_id, min_section_size, timer)
.map(State::Node)
.unwrap_or(State::Terminated)
} else if !dev_config.allow_multiple_lan_nodes && crust_service.has_peers_on_lan() {
error!(
"More than one routing node found on LAN. Currently this is not supported."
);
outbox2.send_event(Event::Terminated);
State::Terminated
} else {
Bootstrapping::new(
BootstrappingPeer::new(
action_sender,
self.cache,
BootstrappingTargetState::RelocatingNode,
TargetState::RelocatingNode,
crust_service,
full_id,
min_section_size,
timer,
)
.map_or(State::Terminated, State::Bootstrapping)
.map(State::BootstrappingPeer)
.unwrap_or(State::Terminated)
}
},
pub_id,
Expand Down Expand Up @@ -616,10 +609,10 @@ impl Node {
}

/// Indicates if there are any pending observations in the parsec object
pub fn has_unconsensused_observations(&self, filter_opaque: bool) -> bool {
pub fn has_unpolled_observations(&self, filter_opaque: bool) -> bool {
self.machine
.current()
.has_unconsensused_observations(filter_opaque)
.has_unpolled_observations(filter_opaque)
}

/// Indicates if a given `PublicId` is in the peer manager as a routing peer
Expand Down
19 changes: 10 additions & 9 deletions src/parsec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,11 @@ pub struct ParsecMap {

impl ParsecMap {
pub fn new(full_id: FullId, gen_pfx_info: &GenesisPfxInfo) -> Self {
let parsec = create(full_id, gen_pfx_info);
let mut map = BTreeMap::new();
let _ = map.insert(*gen_pfx_info.first_info.version(), parsec);
let _ = map.insert(
*gen_pfx_info.first_info.version(),
create(full_id, gen_pfx_info),
);

Self { map }
}
Expand Down Expand Up @@ -100,11 +102,10 @@ impl ParsecMap {
}

pub fn create_gossip(&mut self, version: u64, target: &id::PublicId) -> Option<Message> {
let parsec = self.map.get_mut(&version)?;
parsec
.create_gossip(target)
.ok()
.map(|request| Message::Direct(DirectMessage::ParsecRequest(version, request)))
let request = self.map.get_mut(&version)?.create_gossip(target).ok()?;
Some(Message::Direct(DirectMessage::ParsecRequest(
version, request,
)))
}

pub fn vote_for(&mut self, event: chain::NetworkEvent, log_ident: &str) {
Expand Down Expand Up @@ -159,7 +160,7 @@ impl ParsecMap {
}

#[cfg(feature = "mock_base")]
pub fn has_unconsensused_observations(&self, filter_opaque: bool) -> bool {
pub fn has_unpolled_observations(&self, filter_opaque: bool) -> bool {
let parsec = if let Some(parsec) = self.map.values().last() {
parsec
} else {
Expand All @@ -177,7 +178,7 @@ impl ParsecMap {
}

/// Create Parsec instance.
pub fn create(full_id: FullId, gen_pfx_info: &GenesisPfxInfo) -> Parsec {
fn create(full_id: FullId, gen_pfx_info: &GenesisPfxInfo) -> Parsec {
if gen_pfx_info
.first_info
.members()
Expand Down
18 changes: 0 additions & 18 deletions src/peer_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -308,7 +308,6 @@ pub struct PeerManager {
our_public_id: PublicId,
candidate: Candidate,
disable_client_rate_limiter: bool,
established: bool,
}

impl PeerManager {
Expand All @@ -320,7 +319,6 @@ impl PeerManager {
our_public_id: our_public_id,
candidate: Candidate::None,
disable_client_rate_limiter: disable_client_rate_limiter,
established: false,
}
}

Expand Down Expand Up @@ -633,12 +631,7 @@ impl PeerManager {
}

/// Remove and return `PublicId`s of expired peers.
/// Will only be active once we are established.
pub fn remove_expired_peers(&mut self) -> Vec<PublicId> {
if !self.established {
return vec![];
}

let remove_candidate = if self.candidate.is_expired() {
match self.candidate {
Candidate::None => None,
Expand Down Expand Up @@ -979,17 +972,6 @@ impl PeerManager {

self.peers.remove(pub_id).is_some() || remove_candidate
}

/// Sets this peer as established.
/// Expired peers will be purged once established.
pub fn set_established(&mut self) {
self.established = true;
}

/// Returns whether this peer is established.
pub fn is_established(&self) -> bool {
self.established
}
}

impl fmt::Display for PeerManager {
Expand Down
Loading