Purge old validators #355
Purge old validators #355
Conversation
Codecov Report
@@ Coverage Diff @@
## master #355 +/- ##
==========================================
+ Coverage 90.58% 91.13% +0.55%
==========================================
Files 36 36
Lines 3411 3465 +54
==========================================
+ Hits 3090 3158 +68
+ Misses 321 307 -14
Continue to review full report at Codecov.
|
src/crdt.rs
Outdated
| @@ -32,6 +32,9 @@ use std::sync::{Arc, RwLock}; | |||
| use std::thread::{sleep, Builder, JoinHandle}; | |||
| use std::time::Duration; | |||
| use streamer::{BlobReceiver, BlobSender, Window}; | |||
| use timing::timestamp; | |||
|
|
|||
| const GOSSIP_SLEEP_MILLIS: u64 = 100; | |||
There was a problem hiding this comment.
Can this be a Duration and then name it something more intuitive like VALIDATOR_HEARTBEAT_DURATION or VALIDATOR_TIMEOUT?
There was a problem hiding this comment.
duration is a pia to do compute on or declare. id rather use millis
There was a problem hiding this comment.
I think gossip sleep is actually more descriptive, since its specific to membership in the gossip network, and not to what is using it, which can be leader, thin client, validator, replicator client...
| @@ -235,6 +240,37 @@ impl Crdt { | |||
| self.table[&v.id].version | |||
| ); | |||
| } | |||
| //update the liveness table | |||
| let now = timestamp(); | |||
| *self.alive.entry(v.id).or_insert(now) = now; | |||
src/crdt.rs
Outdated
| //wait for 4x as long as it would randomly take to reach our node | ||
| //assuming everyone is waiting the same amount of time as this node | ||
| let limit = self.table.len() as u64 * GOSSIP_SLEEP_MILLIS * 4; | ||
| let purge_set: Vec<PublicKey> = self.alive |
There was a problem hiding this comment.
How about let dead_ids: Vec<_> =?
src/crdt.rs
Outdated
| .filter_map(|(k, v)| { | ||
| if *k != self.me && (now - v) > limit { | ||
| trace!("purging {:?} {}", &k[..4], v); | ||
| Some((*k).clone()) |
There was a problem hiding this comment.
That clone() probably isn't needed
src/crdt.rs
Outdated
| let limit = self.table.len() as u64 * GOSSIP_SLEEP_MILLIS * 4; | ||
| let purge_set: Vec<PublicKey> = self.alive | ||
| .iter() | ||
| .filter_map(|(k, v)| { |
There was a problem hiding this comment.
You might want to try something like (ref k, ref v) or &(k, v) to get rid of all those dereferences below. Clippy would probably tell you which.
| let len = crdt.table.len() as u64; | ||
| crdt.purge(now); | ||
| let rv = crdt.gossip_request().unwrap(); | ||
| assert_eq!(rv.0, nxt.gossip_addr); |
There was a problem hiding this comment.
Inlining rv on all 4 assertions would make this test easier to read
There was a problem hiding this comment.
it makes really long lines, harder to read for some
This needs an integration test still, and possibly a threshold to only purge when table is bigger than 2.
Fixes #344