Skip to content

Commit

Permalink
Merge branch 'stefan/rpc_bug' into 'master'
Browse files Browse the repository at this point in the history
fix: transport rpc SendError should time out

 

See merge request dfinity-lab/public/ic!15946
  • Loading branch information
stefanneamtu committed Nov 8, 2023
2 parents 273e4c4 + b8e9f5e commit 9704222
Showing 1 changed file with 7 additions and 9 deletions.
16 changes: 7 additions & 9 deletions rs/p2p/consensus_manager/src/receiver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -419,15 +419,13 @@ where
break;
}

rpc_response = transport.rpc(&peer, request) => {
if let Ok(response) = rpc_response {
if let StatusCode::OK = response.status() {
if let Ok(message) =
bincode::deserialize::<Artifact::Message>(response.body())
{
result = DownloadResult::Completed(message, peer);
break;
}
Ok(response) = transport.rpc(&peer, request) => {

This comment has been minimized.

Copy link
@ilbertt

ilbertt Nov 12, 2023

@stefanneamtu how does moving the Ok() check to the branch in the select affect the SendError timeout?

This comment has been minimized.

Copy link
@stefanneamtu

stefanneamtu Nov 12, 2023

Author Member

If the rpc call returns a SendError, it will not match the Ok(...) branch. Therefore, the timeout will happen in the first branch of the select if the other branches do not resolve first.

This comment has been minimized.

Copy link
@ilbertt

ilbertt Nov 12, 2023

I see, thanks!

if let StatusCode::OK = response.status() {
if let Ok(message) =
bincode::deserialize::<Artifact::Message>(response.body())
{
result = DownloadResult::Completed(message, peer);
break;
}
}
metrics.download_task_artifact_download_errors_total.inc();
Expand Down

0 comments on commit 9704222

Please sign in to comment.