Unverified Commit 2ee0250e authored by Andronik Ordian's avatar Andronik Ordian Committed by GitHub
Browse files

validator_discovery: small tweak in retrying logic (#3102)

* validator_discovery: small tweak in retrying logic

* validator_discovery: use timeouts instead
parent a3e5d6b3
Pipeline #139296 passed with stages
in 16 minutes and 42 seconds
...@@ -21,6 +21,7 @@ ...@@ -21,6 +21,7 @@
#[cfg(test)] #[cfg(test)]
mod tests; mod tests;
use std::time::{Duration, Instant};
use futures::{channel::oneshot, FutureExt as _}; use futures::{channel::oneshot, FutureExt as _};
use polkadot_node_subsystem::{ use polkadot_node_subsystem::{
messages::{ messages::{
...@@ -38,6 +39,9 @@ use sp_keystore::{CryptoStore, SyncCryptoStorePtr}; ...@@ -38,6 +39,9 @@ use sp_keystore::{CryptoStore, SyncCryptoStorePtr};
use sp_application_crypto::{Public, AppKey}; use sp_application_crypto::{Public, AppKey};
const LOG_TARGET: &str = "parachain::gossip-support"; const LOG_TARGET: &str = "parachain::gossip-support";
// How much time should we wait since the last
// authority discovery resolution failure.
const BACKOFF_DURATION: Duration = Duration::from_secs(5);
/// The Gossip Support subsystem. /// The Gossip Support subsystem.
pub struct GossipSupport { pub struct GossipSupport {
...@@ -47,7 +51,10 @@ pub struct GossipSupport { ...@@ -47,7 +51,10 @@ pub struct GossipSupport {
#[derive(Default)] #[derive(Default)]
struct State { struct State {
last_session_index: Option<SessionIndex>, last_session_index: Option<SessionIndex>,
force_request: bool, // Some(timestamp) if we failed to resolve
// at least a third of authorities the last time.
// `None` otherwise.
last_failure: Option<Instant>,
} }
impl GossipSupport { impl GossipSupport {
...@@ -162,13 +169,20 @@ impl State { ...@@ -162,13 +169,20 @@ impl State {
) -> Result<(), util::Error> { ) -> Result<(), util::Error> {
for leaf in leaves { for leaf in leaves {
let current_index = util::request_session_index_for_child(leaf, ctx.sender()).await.await??; let current_index = util::request_session_index_for_child(leaf, ctx.sender()).await.await??;
let since_failure = self.last_failure.map(|i| i.elapsed()).unwrap_or_default();
let force_request = since_failure >= BACKOFF_DURATION;
let maybe_new_session = match self.last_session_index { let maybe_new_session = match self.last_session_index {
Some(i) if current_index <= i && !self.force_request => None, Some(i) if current_index <= i && !force_request => None,
_ => Some((current_index, leaf)), _ => Some((current_index, leaf)),
}; };
if let Some((new_session, relay_parent)) = maybe_new_session { if let Some((new_session, relay_parent)) = maybe_new_session {
tracing::debug!(target: LOG_TARGET, %new_session, "New session detected"); tracing::debug!(
target: LOG_TARGET,
%new_session,
%force_request,
"New session detected",
);
let authorities = determine_relevant_authorities(ctx, relay_parent).await?; let authorities = determine_relevant_authorities(ctx, relay_parent).await?;
ensure_i_am_an_authority(keystore, &authorities).await?; ensure_i_am_an_authority(keystore, &authorities).await?;
let num = authorities.len(); let num = authorities.len();
...@@ -185,8 +199,13 @@ impl State { ...@@ -185,8 +199,13 @@ impl State {
let failures = failures.await.unwrap_or(num); let failures = failures.await.unwrap_or(num);
self.last_session_index = Some(new_session); self.last_session_index = Some(new_session);
// issue another request if at least a third of the authorities were not resolved // issue another request for the same session
self.force_request = failures >= num / 3; // if at least a third of the authorities were not resolved
self.last_failure = if failures >= num / 3 {
Some(Instant::now())
} else {
None
}
} }
} }
......
...@@ -160,7 +160,7 @@ fn issues_a_connection_request_on_new_session() { ...@@ -160,7 +160,7 @@ fn issues_a_connection_request_on_new_session() {
}); });
assert_eq!(state.last_session_index, Some(1)); assert_eq!(state.last_session_index, Some(1));
assert!(!state.force_request); assert!(state.last_failure.is_none());
// does not issue on the same session // does not issue on the same session
let hash = Hash::repeat_byte(0xBB); let hash = Hash::repeat_byte(0xBB);
...@@ -181,7 +181,7 @@ fn issues_a_connection_request_on_new_session() { ...@@ -181,7 +181,7 @@ fn issues_a_connection_request_on_new_session() {
}); });
assert_eq!(state.last_session_index, Some(1)); assert_eq!(state.last_session_index, Some(1));
assert!(!state.force_request); assert!(state.last_failure.is_none());
// does on the new one // does on the new one
let hash = Hash::repeat_byte(0xCC); let hash = Hash::repeat_byte(0xCC);
...@@ -225,13 +225,13 @@ fn issues_a_connection_request_on_new_session() { ...@@ -225,13 +225,13 @@ fn issues_a_connection_request_on_new_session() {
virtual_overseer virtual_overseer
}); });
assert_eq!(state.last_session_index, Some(2)); assert_eq!(state.last_session_index, Some(2));
assert!(!state.force_request); assert!(state.last_failure.is_none());
} }
#[test] #[test]
fn issues_a_connection_request_when_last_request_was_mostly_unresolved() { fn issues_a_connection_request_when_last_request_was_mostly_unresolved() {
let hash = Hash::repeat_byte(0xAA); let hash = Hash::repeat_byte(0xAA);
let state = test_harness(State::default(), |mut virtual_overseer| async move { let mut state = test_harness(State::default(), |mut virtual_overseer| async move {
let overseer = &mut virtual_overseer; let overseer = &mut virtual_overseer;
overseer_signal_active_leaves(overseer, hash).await; overseer_signal_active_leaves(overseer, hash).await;
assert_matches!( assert_matches!(
...@@ -271,7 +271,8 @@ fn issues_a_connection_request_when_last_request_was_mostly_unresolved() { ...@@ -271,7 +271,8 @@ fn issues_a_connection_request_when_last_request_was_mostly_unresolved() {
}); });
assert_eq!(state.last_session_index, Some(1)); assert_eq!(state.last_session_index, Some(1));
assert!(state.force_request); assert!(state.last_failure.is_some());
state.last_failure = state.last_failure.and_then(|i| i.checked_sub(BACKOFF_DURATION));
let hash = Hash::repeat_byte(0xBB); let hash = Hash::repeat_byte(0xBB);
let state = test_harness(state, |mut virtual_overseer| async move { let state = test_harness(state, |mut virtual_overseer| async move {
...@@ -314,6 +315,6 @@ fn issues_a_connection_request_when_last_request_was_mostly_unresolved() { ...@@ -314,6 +315,6 @@ fn issues_a_connection_request_when_last_request_was_mostly_unresolved() {
}); });
assert_eq!(state.last_session_index, Some(1)); assert_eq!(state.last_session_index, Some(1));
assert!(!state.force_request); assert!(state.last_failure.is_none());
} }
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment