GH-647: HOTFIX: Prevent duplication of records of connection-progress #213
Conversation
…previous pass targets
…eer address present in a different connection progress
…part of previous pass targets" This reverts commit afd43da.
…or and GossipHandler
| @@ -63,6 +63,7 @@ trait GossipHandler: NamedType + Send /* Send because lazily-written tests requi | |||
| database: &mut NeighborhoodDatabase, | |||
| agrs: Vec<AccessibleGossipRecord>, | |||
| gossip_source: SocketAddr, | |||
| peer_addrs: &[IpAddr], | |||
There was a problem hiding this comment.
Can you think of a better name than peer_addrs? When you're thinking about Gossip, you're probably not thinking about connection progress messages, and it's difficult to understand what a peer might be in this context. There's cpm_recipient right below it; maybe these two parameters could be named similarly to make their association clear?
There was a problem hiding this comment.
I used the name connection_progress_peers. I think it's conveying the correct idea.
node/src/neighborhood/mod.rs
Outdated
| msg.peer_addr | ||
| ); | ||
| Err(e) => { | ||
| trace!(self.logger, "{}", e); |
There was a problem hiding this comment.
Is this error specific enough to be able to track down if we see it in the log? Should an identifying string be added so that a codebase text search can lead us here?
There was a problem hiding this comment.
I also wanted to add some string to this trace log before raising the PR but somehow, I didn't. Later, when you added this comment, I got a bit excited to rephrase these logs.
I've used this prefix - "Found unnecessary connection progress message".
Now, it will be easier to find the log inside our codebase.
utkarshg6
changed the title
* GH-647: rename peers_addrs to connection_progress_peers * GH-647: provide the accurate name inside the constructor of System in the test * GH-647: reword the trace logs for the unnecessary connection progress * GH-647: use a unique number to generate a unique socket address for peer 2 * GH-647: add review changes for overall_connection_status.rs * GH-647: add connection_progress_peers as a param in the mock (#216)
No description provided.