feat(census): keep track of liveness checks #1575
Conversation
|
Followup PR: #1576 |
looks good
portal-bridge/src/census/peer.rs
Outdated
| } else { | ||
| self.enr = enr.clone(); | ||
| } |
There was a problem hiding this comment.
| } else { | |
| self.enr = enr.clone(); | |
| } | |
| } else if self.enr.seq() < enr.seq() { | |
| self.enr = enr.clone(); | |
| } |
it probably doesn't matter, but wouldn't we only need to do the clone if a updated enr was given, which should be fairly infrequent so we might as well avoid the clone
There was a problem hiding this comment.
I changed function signature so it accepts Enr, in which case we never clone it.
portal-bridge/src/census/network.rs
Outdated
|
|
||
| let total_peers = self.network.peers.len(); | ||
| let ending_peers = self.network.peers.len(); | ||
| let new_peers = ending_peers - starting_peers; |
There was a problem hiding this comment.
I am assuming the possibility that new_peers can be negative is a feature to display we lost peers? Because isn't there a case where ending_peers can be less then starting_peers i.e. if more peers were removed then added?
This doesn't really look like an issue, just wanted to point it out.
There was a problem hiding this comment.
I was thinking about it and concluded that it shouldn't happen. Unless there is some weird race-condition and/or Census is performing very badly.
But, you are right, it's technically possible. It's also possible that we change logic in the future and forget to update and this would panic (or difference no longer means that these are added).
So I updated the code to not panic and changed the message.
What was wrong?
Census doesn't keep track of previous liveness checks or other request sent to peers.
Needed for: #1501
How was it fixed?
This PR does ground work focused on liveness checks, and followup PR will introduce other requests (that will later be used for peer scoring).