From 98c264f0d97e2032a2a511926a2b5d71764b3a19 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Mon, 3 Apr 2023 11:29:06 +0300 Subject: [PATCH 1/4] Cache `SessionInfo` on new activated leaf in `dispute-distribution` --- .../dispute-distribution/src/sender/mod.rs | 7 ++ .../dispute-distribution/src/tests/mod.rs | 90 ++++--------------- 2 files changed, 23 insertions(+), 74 deletions(-) diff --git a/node/network/dispute-distribution/src/sender/mod.rs b/node/network/dispute-distribution/src/sender/mod.rs index 629c4913b78e..efff2ecd3654 100644 --- a/node/network/dispute-distribution/src/sender/mod.rs +++ b/node/network/dispute-distribution/src/sender/mod.rs @@ -366,6 +366,13 @@ async fn get_active_session_indices( // Iterate all heads we track as active and fetch the child' session indices. for head in active_heads { let session_index = runtime.get_session_index_for_child(ctx.sender(), *head).await?; + // Cache session info + match runtime.get_session_info_by_index(ctx.sender(), *head, session_index).await { + Ok(_) => { /* do nothing */ }, + Err(err) => { + gum::warn!(target: LOG_TARGET, ?err, ?session_index, "Can't cache SessionInfo"); + }, + } indeces.insert(session_index, *head); } Ok(indeces) diff --git a/node/network/dispute-distribution/src/tests/mod.rs b/node/network/dispute-distribution/src/tests/mod.rs index 0b14afea2a30..337883be8ed7 100644 --- a/node/network/dispute-distribution/src/tests/mod.rs +++ b/node/network/dispute-distribution/src/tests/mod.rs @@ -81,7 +81,7 @@ fn send_dispute_sends_dispute() { let relay_parent = Hash::random(); let candidate = make_candidate_receipt(relay_parent); - send_dispute(&mut handle, candidate, true).await; + send_dispute(&mut handle, candidate).await; conclude(&mut handle).await; }; test_harness(test); @@ -96,7 +96,7 @@ fn send_honors_rate_limit() { let relay_parent = Hash::random(); let candidate = make_candidate_receipt(relay_parent); let before_request = Instant::now(); - send_dispute(&mut handle, candidate, true).await; + send_dispute(&mut handle, candidate).await; // First send should not be rate limited: gum::trace!("Passed time: {:#?}", Instant::now().saturating_duration_since(before_request)); // This test would likely be flaky on CI: @@ -104,7 +104,7 @@ fn send_honors_rate_limit() { let relay_parent = Hash::random(); let candidate = make_candidate_receipt(relay_parent); - send_dispute(&mut handle, candidate, false).await; + send_dispute(&mut handle, candidate).await; // Second send should be rate limited: gum::trace!( "Passed time for send_dispute: {:#?}", @@ -120,7 +120,6 @@ fn send_honors_rate_limit() { async fn send_dispute( handle: &mut TestSubsystemContextHandle, candidate: CandidateReceipt, - needs_session_info: bool, ) { let before_request = Instant::now(); let message = make_dispute_message(candidate.clone(), ALICE_INDEX, FERDIE_INDEX); @@ -138,25 +137,6 @@ async fn send_dispute( "Passed time for sending message: {:#?}", Instant::now().saturating_duration_since(before_request) ); - if needs_session_info { - // Requests needed session info: - assert_matches!( - handle.recv().await, - AllMessages::RuntimeApi( - RuntimeApiMessage::Request( - hash, - RuntimeApiRequest::SessionInfo(session_index, tx) - ) - ) => { - assert_eq!(session_index, MOCK_SESSION_INDEX); - assert_eq!( - hash, - message.candidate_receipt().descriptor.relay_parent - ); - tx.send(Ok(Some(MOCK_SESSION_INFO.clone()))).expect("Receiver should stay alive."); - } - ); - } let expected_receivers = { let info = &MOCK_SESSION_INFO; @@ -492,23 +472,6 @@ fn send_dispute_gets_cleaned_up() { msg: DisputeDistributionMessage::SendDispute(message.clone()), }) .await; - // Requests needed session info: - assert_matches!( - handle.recv().await, - AllMessages::RuntimeApi( - RuntimeApiMessage::Request( - hash, - RuntimeApiRequest::SessionInfo(session_index, tx) - ) - ) => { - assert_eq!(session_index, MOCK_SESSION_INDEX); - assert_eq!( - hash, - message.candidate_receipt().descriptor.relay_parent - ); - tx.send(Ok(Some(MOCK_SESSION_INFO.clone()))).expect("Receiver should stay alive."); - } - ); let expected_receivers = { let info = &MOCK_SESSION_INFO; @@ -558,23 +521,6 @@ fn dispute_retries_and_works_across_session_boundaries() { msg: DisputeDistributionMessage::SendDispute(message.clone()), }) .await; - // Requests needed session info: - assert_matches!( - handle.recv().await, - AllMessages::RuntimeApi( - RuntimeApiMessage::Request( - hash, - RuntimeApiRequest::SessionInfo(session_index, tx) - ) - ) => { - assert_eq!(session_index, MOCK_SESSION_INDEX); - assert_eq!( - hash, - message.candidate_receipt().descriptor.relay_parent - ); - tx.send(Ok(Some(MOCK_SESSION_INFO.clone()))).expect("Receiver should stay alive."); - } - ); let expected_receivers: HashSet<_> = { let info = &MOCK_SESSION_INFO; @@ -776,7 +722,6 @@ async fn activate_leaf( // Currently active disputes to send to the subsystem. active_disputes: Vec<(SessionIndex, CandidateHash, DisputeStatus)>, ) { - let has_active_disputes = !active_disputes.is_empty(); handle .send(FromOrchestra::Signal(OverseerSignal::ActiveLeaves(ActiveLeavesUpdate { activated: Some(ActivatedLeaf { @@ -798,27 +743,24 @@ async fn activate_leaf( tx.send(Ok(session_index)).expect("Receiver should stay alive."); } ); - assert_matches!( - handle.recv().await, - AllMessages::DisputeCoordinator(DisputeCoordinatorMessage::ActiveDisputes(tx)) => { - tx.send(active_disputes).expect("Receiver should stay alive."); - } - ); - - let new_session = match (new_session, has_active_disputes) { - (Some(new_session), true) => new_session, - _ => return, - }; - assert_matches!( + if let Some(session_info) = new_session { + assert_matches!( handle.recv().await, AllMessages::RuntimeApi(RuntimeApiMessage::Request( - h, - RuntimeApiRequest::SessionInfo(i, tx) + h, + RuntimeApiRequest::SessionInfo(session_idx, tx) )) => { assert_eq!(h, activate); - assert_eq!(i, session_index); - tx.send(Ok(Some(new_session))).expect("Receiver should stay alive."); + assert_eq!(session_index, session_idx); + tx.send(Ok(Some(session_info))).expect("Receiver should stay alive."); + }); + } + + assert_matches!( + handle.recv().await, + AllMessages::DisputeCoordinator(DisputeCoordinatorMessage::ActiveDisputes(tx)) => { + tx.send(active_disputes).expect("Receiver should stay alive."); } ); } From 6ac791063bdde7492d5d348563118c1913059827 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Mon, 3 Apr 2023 11:40:05 +0300 Subject: [PATCH 2/4] Update node/network/dispute-distribution/src/sender/mod.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- node/network/dispute-distribution/src/sender/mod.rs | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/node/network/dispute-distribution/src/sender/mod.rs b/node/network/dispute-distribution/src/sender/mod.rs index efff2ecd3654..a45b7646bd7e 100644 --- a/node/network/dispute-distribution/src/sender/mod.rs +++ b/node/network/dispute-distribution/src/sender/mod.rs @@ -367,11 +367,8 @@ async fn get_active_session_indices( for head in active_heads { let session_index = runtime.get_session_index_for_child(ctx.sender(), *head).await?; // Cache session info - match runtime.get_session_info_by_index(ctx.sender(), *head, session_index).await { - Ok(_) => { /* do nothing */ }, - Err(err) => { - gum::warn!(target: LOG_TARGET, ?err, ?session_index, "Can't cache SessionInfo"); - }, + if let Err(err) = runtime.get_session_info_by_index(ctx.sender(), *head, session_index).await { + gum::warn!(target: LOG_TARGET, ?err, ?session_index, "Can't cache SessionInfo"); } indeces.insert(session_index, *head); } From de0a310fc441cda40479fc6ec98c74c326223ce1 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Mon, 3 Apr 2023 11:44:30 +0300 Subject: [PATCH 3/4] fmt --- node/network/dispute-distribution/src/sender/mod.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/node/network/dispute-distribution/src/sender/mod.rs b/node/network/dispute-distribution/src/sender/mod.rs index a45b7646bd7e..9d2b2b2686c8 100644 --- a/node/network/dispute-distribution/src/sender/mod.rs +++ b/node/network/dispute-distribution/src/sender/mod.rs @@ -367,7 +367,9 @@ async fn get_active_session_indices( for head in active_heads { let session_index = runtime.get_session_index_for_child(ctx.sender(), *head).await?; // Cache session info - if let Err(err) = runtime.get_session_info_by_index(ctx.sender(), *head, session_index).await { + if let Err(err) = + runtime.get_session_info_by_index(ctx.sender(), *head, session_index).await + { gum::warn!(target: LOG_TARGET, ?err, ?session_index, "Can't cache SessionInfo"); } indeces.insert(session_index, *head); From 9d5f3e8ea6d7a9521e1cc311fd382a4805003de8 Mon Sep 17 00:00:00 2001 From: Tsvetomir Dimitrov Date: Mon, 3 Apr 2023 11:45:58 +0300 Subject: [PATCH 4/4] Decrease log level --- node/network/dispute-distribution/src/sender/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/node/network/dispute-distribution/src/sender/mod.rs b/node/network/dispute-distribution/src/sender/mod.rs index 9d2b2b2686c8..6a1dedcb3c33 100644 --- a/node/network/dispute-distribution/src/sender/mod.rs +++ b/node/network/dispute-distribution/src/sender/mod.rs @@ -370,7 +370,7 @@ async fn get_active_session_indices( if let Err(err) = runtime.get_session_info_by_index(ctx.sender(), *head, session_index).await { - gum::warn!(target: LOG_TARGET, ?err, ?session_index, "Can't cache SessionInfo"); + gum::debug!(target: LOG_TARGET, ?err, ?session_index, "Can't cache SessionInfo"); } indeces.insert(session_index, *head); }