-
Notifications
You must be signed in to change notification settings - Fork 3.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[consensus] rename some network methods and failpoints to make them more consistent #2130
Conversation
consensus/src/network.rs
Outdated
let msg = ConsensusMsg::SyncInfo(Box::new(sync_info_msg)); | ||
self.broadcast(msg).await | ||
} | ||
|
||
pub async fn broadcast_timeout_vote(&mut self, timeout_vote_msg: VoteMsg) { | ||
fail_point!("consensus::send_vote", |_| ()); | ||
fail_point!("consensus::broadcast_timeout_vote", |_| ()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Everything else is just aesthetics. This is the only significant change (it conflicts with L210)
@bchocho - I do feel like we should have a same prefix for all process failpoints, and then the same prefix for all sending messages failpoints. I am also adding one joint failpoint for those two types here - 3507996 (that's a draft for consensus testing I am doing) how about having these prefixes: so my new fail_points become: "consensus::broadcast_timeout_vote" becomes "consensus::send::broadcast_timeout_vote" what do you think? |
✅ Forge test successForge is land-blocking
|
@@ -188,7 +188,7 @@ impl BlockStore { | |||
if highest_commit_cert.ledger_info().ledger_info().ends_epoch() { | |||
retriever | |||
.network | |||
.notify_epoch_change(EpochChangeProof::new( | |||
.send_epoch_change(EpochChangeProof::new( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was trying to differentiate it from send but maybe did a bad job. notify == notify self so this doesn't go through network
This change is