Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Fix build after suggested changes
  • Loading branch information
montekki committed Sep 8, 2020
commit 0b6c57256276b0fb4456f1b9773ef8b460e7561f
13 changes: 8 additions & 5 deletions node/network/collator-protocol/src/validator_side.rs
Original file line number Diff line number Diff line change
Expand Up @@ -456,10 +456,13 @@ async fn handle_our_view_change(
) -> Result<()> {
let old_view = std::mem::replace(&mut (state.view), view);

let removed = old_view.difference(&state.view).collect::<Vec<_>>();
let removed = old_view
.difference(&state.view)
.cloned()
.collect::<Vec<_>>();

for removed in removed {
remove_relay_parent(state, *removed).await?;
for removed in removed.into_iter() {
remove_relay_parent(state, removed).await?;
}

Ok(())
Expand All @@ -480,7 +483,7 @@ where
if let Some(request_info) = state.requests_info.remove(&id) {
let (relay_parent, para_id, peer_id) = key;

modify_reputation(ctx, peer_id.clone(), COST_REQUEST_TIMEDOUT).await?;
modify_reputation(ctx, peer_id.clone(), COST_REQUEST_TIMED_OUT).await?;

// the callee will check if the parent is still in view, so do no checks here.
request_collation(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does this immediately request the collation again?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the request times out, we need to note the collator as being unreliable and reduce its priority relative to other collators. And then make another request - repeat until we get a response or the chain has moved on.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, yeah, that was coming from the perspective of there being no CandidateSelection subsystem. If there is CandidateSelection, it should be CandidateSelection that decides to make another request. It doesn't really make sense to request from the collator who just timed out over and over again - the implication of the line quoted from the guide is that we should make another request of another collator.

Expand Down Expand Up @@ -899,7 +902,7 @@ mod tests {
NetworkBridgeMessage::ReportPeer(peer, rep)
) => {
assert_eq!(peer, peer_b);
assert_eq!(rep, COST_REQUEST_TIMEDOUT);
assert_eq!(rep, COST_REQUEST_TIMED_OUT);
}
);

Expand Down