directory-client: removed premature equality check which caused a lock update problem

This commit is contained in:
Dave Hrycyszyn
2020-01-27 14:59:56 +00:00
parent a55b87809e
commit e5580df77b
6 changed files with 3 additions and 169 deletions
@@ -1,6 +1,4 @@
use super::PresenceEq;
use serde::{Deserialize, Serialize};
use std::collections::HashSet;
use topology::coco;
#[derive(Clone, Debug, Deserialize, Serialize)]
@@ -33,24 +31,3 @@ impl From<topology::coco::Node> for CocoPresence {
}
}
}
impl PresenceEq for Vec<CocoPresence> {
fn presence_eq(&self, other: &Self) -> bool {
if self.len() != other.len() {
return false;
}
// we can't take the whole thing into set as it does not implement 'Eq' and we can't
// derive it as we don't want to take 'last_seen' into consideration
let self_set: HashSet<_> = self
.iter()
.map(|c| (&c.host, &c.pub_key, &c.version))
.collect();
let other_set: HashSet<_> = other
.iter()
.map(|c| (&c.host, &c.pub_key, &c.version))
.collect();
self_set == other_set
}
}
@@ -1,6 +1,4 @@
use super::PresenceEq;
use serde::{Deserialize, Serialize};
use std::collections::HashSet;
use std::convert::TryInto;
use std::io;
use std::net::ToSocketAddrs;
@@ -49,24 +47,3 @@ impl From<topology::mix::Node> for MixNodePresence {
}
}
}
impl PresenceEq for Vec<MixNodePresence> {
fn presence_eq(&self, other: &Self) -> bool {
if self.len() != other.len() {
return false;
}
// we can't take the whole thing into set as it does not implement 'Eq' and we can't
// derive it as we don't want to take 'last_seen' into consideration
let self_set: HashSet<_> = self
.iter()
.map(|m| (&m.host, &m.pub_key, &m.version, &m.layer))
.collect();
let other_set: HashSet<_> = other
.iter()
.map(|m| (&m.host, &m.pub_key, &m.version, &m.layer))
.collect();
self_set == other_set
}
}
@@ -12,12 +12,6 @@ pub mod coconodes;
pub mod mixnodes;
pub mod providers;
// special version of 'PartialEq' that does not care about last_seen field
//(where applicable) or order of elements in vectors
trait PresenceEq {
fn presence_eq(&self, other: &Self) -> bool;
}
// Topology shows us the current state of the overall Nym network
#[derive(Clone, Debug, Deserialize, Serialize)]
#[serde(rename_all = "camelCase")]
@@ -27,23 +21,6 @@ pub struct Topology {
pub mix_provider_nodes: Vec<providers::MixProviderPresence>,
}
impl PartialEq for Topology {
// we need a custom implementation as the order of nodes does not matter
// also we do not care about 'last_seen' when comparing topologies
fn eq(&self, other: &Self) -> bool {
if !self.coco_nodes.presence_eq(&other.coco_nodes)
|| !self.mix_nodes.presence_eq(&other.mix_nodes)
|| !self
.mix_provider_nodes
.presence_eq(&other.mix_provider_nodes)
{
return false;
}
true
}
}
impl NymTopology for Topology {
fn new(directory_server: String) -> Self {
debug!("Using directory server: {:?}", directory_server);
@@ -1,6 +1,4 @@
use super::PresenceEq;
use serde::{Deserialize, Serialize};
use std::collections::HashSet;
use topology::provider;
#[derive(Clone, Debug, Deserialize, Serialize)]
@@ -14,32 +12,6 @@ pub struct MixProviderPresence {
pub version: String,
}
impl PresenceEq for MixProviderPresence {
fn presence_eq(&self, other: &Self) -> bool {
if self.registered_clients.len() != other.registered_clients.len() {
return false;
}
if self.client_listener != other.client_listener
|| self.mixnet_listener != other.mixnet_listener
|| self.pub_key != other.pub_key
|| self.version != other.version
{
return false;
}
let clients_self_set: HashSet<_> =
self.registered_clients.iter().map(|c| &c.pub_key).collect();
let clients_other_set: HashSet<_> = other
.registered_clients
.iter()
.map(|c| &c.pub_key)
.collect();
clients_self_set == clients_other_set
}
}
impl Into<topology::provider::Node> for MixProviderPresence {
fn into(self) -> topology::provider::Node {
topology::provider::Node {
@@ -95,55 +67,3 @@ impl From<topology::provider::Client> for MixProviderClient {
}
}
}
impl PresenceEq for Vec<MixProviderPresence> {
fn presence_eq(&self, other: &Self) -> bool {
if self.len() != other.len() {
return false;
}
// we can't take the whole thing into set as it does not implement 'Eq' and we can't
// derive it as we don't want to take 'last_seen' into consideration.
// We also don't care about order of registered_clients
// since we're going to be getting rid of this very soon anyway, just clone registered
// clients vector and sort it
let self_set: HashSet<_> = self
.iter()
.map(|p| {
(
&p.client_listener,
&p.mixnet_listener,
&p.pub_key,
&p.version,
p.registered_clients
.iter()
.cloned()
.map(|c| c.pub_key)
.collect::<Vec<_>>()
.sort(),
)
})
.collect();
let other_set: HashSet<_> = other
.iter()
.map(|p| {
(
&p.client_listener,
&p.mixnet_listener,
&p.pub_key,
&p.version,
p.registered_clients
.iter()
.cloned()
.map(|c| c.pub_key)
.collect::<Vec<_>>()
.sort(),
)
})
.collect();
self_set == other_set
}
}
+1 -1
View File
@@ -10,7 +10,7 @@ mod filter;
pub mod mix;
pub mod provider;
pub trait NymTopology: Sized + PartialEq + std::fmt::Debug + Send + Sync {
pub trait NymTopology: Sized + std::fmt::Debug + Send + Sync {
fn new(directory_server: String) -> Self;
fn new_from_nodes(
mix_nodes: Vec<mix::Node>,
+2 -19
View File
@@ -58,7 +58,7 @@ impl<T: NymTopology> TopologyControl<T> {
let healthcheck_scores = match healthcheck_result {
Err(err) => {
error!("Error while performing the healtcheck: {:?}", err);
error!("Error while performing the healthcheck: {:?}", err);
return Err(TopologyError::HealthCheckError);
}
Ok(scores) => scores,
@@ -91,18 +91,6 @@ impl<T: NymTopology> TopologyControl<T> {
write_lock.topology = new_topology;
}
async fn should_update_topology(&mut self, new_topology: &Option<T>) -> bool {
let read_lock = self.inner.read().await;
match new_topology {
// if new topology is invalid, we MUST update to it as it is impossible to send packets through
None => true,
Some(new_topology) => match &read_lock.topology {
None => true,
Some(old_topology) => new_topology != old_topology,
},
}
}
pub(crate) async fn run_refresher(mut self) {
info!("Starting topology refresher");
let delay_duration = Duration::from_secs_f64(self.refresh_rate);
@@ -119,12 +107,7 @@ impl<T: NymTopology> TopologyControl<T> {
}
};
if self.should_update_topology(&new_topology).await {
info!("Detected changes in topology - updating global view!");
self.update_global_topology(new_topology).await;
}
self.update_global_topology(new_topology).await;
tokio::time::delay_for(delay_duration).await;
}
}