diff --git a/ethcore/light/src/on_demand/mod.rs b/ethcore/light/src/on_demand/mod.rs index ef4470740ca..d241148d7f5 100644 --- a/ethcore/light/src/on_demand/mod.rs +++ b/ethcore/light/src/on_demand/mod.rs @@ -60,13 +60,15 @@ pub type ExecutionResult = Result; /// The default number of a request is ```exponentially backed off``` until it gets dropped pub const DEFAULT_REQUEST_BACKOFF_ATTEMPTS: usize = 10; /// The initial backoff interval for OnDemand queries -pub const DEFAULT_MIN_BACKOFF_DURATION: Duration = Duration::from_secs(10); +pub const DEFAULT_REQUEST_MIN_BACKOFF_DURATION: Duration = Duration::from_secs(10); /// The maximum request interval for OnDemand queries -pub const DEFAULT_MAX_BACKOFF_DURATION: Duration = Duration::from_secs(100); -/// The default window length when a requested is evaluated as successful or as a failure -pub const DEFAULT_WINDOW_DURATION: Duration = Duration::from_secs(10); +pub const DEFAULT_REQUEST_MAX_BACKOFF_DURATION: Duration = Duration::from_secs(100); +/// The default window length a response is evaluated +pub const DEFAULT_RESPONSE_WINDOW_DURATION: Duration = Duration::from_secs(10); +/// The default window length a request is evaluated +pub const DEFAULT_REQUEST_WINDOW_DURATION: Duration = Duration::from_secs(10); /// The default number of maximum backoff iterations -pub const DEFAULT_MAX_BACKOFF_ROUNDS: usize = 10; +pub const DEFAULT_MAX_REQUEST_BACKOFF_ROUNDS: usize = 10; /// OnDemand related errors pub mod error { @@ -328,10 +330,11 @@ pub struct OnDemand { in_transit: RwLock>, cache: Arc>, no_immediate_dispatch: bool, - time_window_dur: Duration, - start_backoff_dur: Duration, - max_backoff_dur: Duration, - max_backoff_rounds: usize, + response_time_window: Duration, + request_time_window: Duration, + request_backoff_start: Duration, + request_backoff_max: Duration, + request_backoff_rounds_max: usize } impl OnDemand { @@ -339,59 +342,35 @@ impl OnDemand { /// Create a new `OnDemand` service with the given cache. pub fn new( cache: Arc>, - time_window_dur: Duration, - start_backoff_dur: Duration, - max_backoff_dur: Duration, - max_backoff_rounds: usize, + response_time_window: Duration, + request_time_window: Duration, + request_backoff_start: Duration, + request_backoff_max: Duration, + request_backoff_rounds_max: usize, ) -> Self { - // santize input in order to make sure that it doesn't panic - let (s_window_dur, s_start_backoff_dur, s_max_backoff_dur) = - Self::santize_circuit_breaker_input(time_window_dur, start_backoff_dur, max_backoff_dur); - OnDemand { pending: RwLock::new(Vec::new()), peers: RwLock::new(HashMap::new()), in_transit: RwLock::new(HashMap::new()), cache, no_immediate_dispatch: false, - time_window_dur: s_window_dur, - start_backoff_dur: s_start_backoff_dur, - max_backoff_dur: s_max_backoff_dur, - max_backoff_rounds, + response_time_window: Self::santize_circuit_breaker_input(response_time_window, "Response time window"), + request_time_window: Self::santize_circuit_breaker_input(request_time_window, "Request time window"), + request_backoff_start: Self::santize_circuit_breaker_input(request_backoff_start, "Request initial backoff time window"), + request_backoff_max: Self::santize_circuit_breaker_input(request_backoff_max, "Request maximum backoff time window"), + request_backoff_rounds_max, } } - fn santize_circuit_breaker_input( - time_window_dur: Duration, - start_backoff_dur: Duration, - max_backoff_dur: Duration - ) -> (Duration, Duration, Duration) { - let time_window_dur = if time_window_dur.as_secs() < 1 { - warn!(target: "on_demand", - "Time window is too short must be at least 1 second, configuring it to 1 second"); - Duration::from_secs(1) - } else { - time_window_dur - }; - - let start_backoff_dur = if start_backoff_dur.as_secs() < 1 { - warn!(target: "on_demand", - "Initial backoff time is too short must be at least 1 second, configuring it to 1 second"); - Duration::from_secs(1) - } else { - start_backoff_dur - }; - - let max_backoff_dur = if max_backoff_dur.as_secs() < 1 { + fn santize_circuit_breaker_input(dur: Duration, name: &'static str) -> Duration { + if dur.as_secs() < 1 { warn!(target: "on_demand", - "Maximum backoff time is too short must be at least 1 second, configuring it 1 second"); + "{} is too short must be at least 1 second, configuring it to 1 second", name); Duration::from_secs(1) } else { - start_backoff_dur - }; - - (time_window_dur, start_backoff_dur, max_backoff_dur) + dur + } } // make a test version: this doesn't dispatch pending requests @@ -399,12 +378,20 @@ impl OnDemand { #[cfg(test)] fn new_test( cache: Arc>, - time_window_dur: Duration, - start_backoff_dur: Duration, - max_backoff_dur: Duration, - max_backoff_rounds: usize, + response_time_window: Duration, + request_time_window: Duration, + request_backoff_start: Duration, + request_backoff_max: Duration, + request_backoff_rounds_max: usize, ) -> Self { - let mut me = OnDemand::new(cache, time_window_dur, start_backoff_dur, max_backoff_dur, max_backoff_rounds); + let mut me = OnDemand::new( + cache, + response_time_window, + request_time_window, + request_backoff_start, + request_backoff_max, + request_backoff_rounds_max + ); me.no_immediate_dispatch = true; me @@ -459,12 +446,12 @@ impl OnDemand { responses, sender, request_guard: RequestGuard::new( - self.start_backoff_dur, - self.max_backoff_dur, self. - time_window_dur, - self.max_backoff_rounds + self.request_time_window, + self.request_backoff_start, + self.request_backoff_max, + self.request_backoff_rounds_max ), - response_guard: ResponseGuard::new(self.time_window_dur), + response_guard: ResponseGuard::new(self.response_time_window), }); Ok(receiver) diff --git a/ethcore/light/src/on_demand/request_guard.rs b/ethcore/light/src/on_demand/request_guard.rs index 946b883567a..b182a8d72fb 100644 --- a/ethcore/light/src/on_demand/request_guard.rs +++ b/ethcore/light/src/on_demand/request_guard.rs @@ -41,9 +41,9 @@ pub struct RequestGuard { impl RequestGuard { /// Constructor pub fn new( + window_dur: Duration, min_backoff_dur: Duration, max_backoff_dur: Duration, - window_dur: Duration, max_backoff_rounds: usize ) -> Self { let backoff = failsafe::backoff::exponential(min_backoff_dur, max_backoff_dur); diff --git a/ethcore/light/src/on_demand/tests.rs b/ethcore/light/src/on_demand/tests.rs index ce4d5963a9d..3435cd5be4e 100644 --- a/ethcore/light/src/on_demand/tests.rs +++ b/ethcore/light/src/on_demand/tests.rs @@ -95,9 +95,15 @@ impl Harness { Harness { service: OnDemand::new_test( cache, + // response duration Duration::from_secs(5), + // request duration Duration::from_secs(5), + // request start backoff + Duration::from_secs(5), + // request max backoff Duration::from_secs(10), + // request max backoff rounds super::DEFAULT_REQUEST_BACKOFF_ATTEMPTS ) } @@ -533,7 +539,7 @@ fn request_without_response_should_backoff_and_then_be_dropped() { for (i, &backoff) in binary_exp_backoff.iter().enumerate() { harness.service.dispatch_pending(&Context::FaultyRequest); let now = Instant::now(); - while now.elapsed() <= harness.service.time_window_dur {} + while now.elapsed() <= harness.service.request_time_window {} let now = Instant::now(); while now.elapsed() < Duration::from_secs(backoff + 2) {} if i < binary_exp_backoff.len() - 1 { @@ -576,7 +582,7 @@ fn empty_responses_exceeds_limit_should_be_dropped() { // Send `empty responses` in the current time window // Use only half of the `time_window` because we can't be sure exactly // when the window started and the clock accurancy - while now.elapsed() < harness.service.time_window_dur / 2 { + while now.elapsed() < harness.service.response_time_window / 2 { harness.service.on_responses( &Context::RequestFrom(13, req_id), req_id, diff --git a/parity/cli/mod.rs b/parity/cli/mod.rs index f62b6d45909..9ef192cfc6c 100644 --- a/parity/cli/mod.rs +++ b/parity/cli/mod.rs @@ -563,21 +563,25 @@ usage! { "Specify CORS header for IPFS API responses. Special options: \"all\", \"none\".", ["Light Client Options"] - ARG arg_on_demand_time_window: (Option) = None, or |c: &Config| c.light.as_ref()?.on_demand_time_window, + ARG arg_on_demand_response_time_window: (Option) = None, or |c: &Config| c.light.as_ref()?.on_demand_response_time_window, + "--on-demand-response-time-window=[MS]", + "Specify the maximum time to wait for a successful response", + + ARG arg_on_demand_request_time_window: (Option) = None, or |c: &Config| c.light.as_ref()?.on_demand_request_time_window, "--on-demand-time-window=[MS]", - "Specify the time window", + "Specify the maximum inital time to wait for a successful request (it will backoff exponentially according to the request options", - ARG arg_on_demand_start_backoff: (Option) = None, or |c: &Config| c.light.as_ref()?.on_demand_start_backoff, + ARG arg_on_demand_request_backoff_start: (Option) = None, or |c: &Config| c.light.as_ref()?.on_demand_request_backoff_start, "--on-demand-start-backoff=[MS]", - "Specify light client start backoff time", + "Specify light client initial backoff time for a request", - ARG arg_on_demand_end_backoff: (Option) = None, or |c: &Config| c.light.as_ref()?.on_demand_end_backoff, + ARG arg_on_demand_request_backoff_max: (Option) = None, or |c: &Config| c.light.as_ref()?.on_demand_request_backoff_max, "--on-demand-end-backoff=[MS]", - "Specify light client end backoff time", + "Specify light client maximum backoff time for a request", - ARG arg_on_demand_max_backoff_rounds: (Option) = None, or |c: &Config| c.light.as_ref()?.on_demand_max_backoff_rounds, + ARG arg_on_demand_request_backoff_rounds_max: (Option) = None, or |c: &Config| c.light.as_ref()?.on_demand_request_backoff_rounds_max, "--on-demand-max-backoff-rounds=[TIMES]", - "Specify light client maximum number of backoff iterations", + "Specify light client maximum number of backoff iterations for a request", ["Secret Store Options"] FLAG flag_no_secretstore: (bool) = false, or |c: &Config| c.secretstore.as_ref()?.disable.clone(), @@ -1395,10 +1399,11 @@ struct Whisper { #[derive(Default, Debug, PartialEq, Deserialize)] #[serde(deny_unknown_fields)] struct Light { - on_demand_time_window: Option, - on_demand_start_backoff: Option, - on_demand_end_backoff: Option, - on_demand_max_backoff_rounds: Option, + on_demand_response_time_window: Option, + on_demand_request_time_window: Option, + on_demand_request_backoff_start: Option, + on_demand_request_backoff_max: Option, + on_demand_request_backoff_rounds_max: Option } #[cfg(test)] @@ -1812,10 +1817,11 @@ mod tests { arg_snapshot_threads: None, // -- Light options. - arg_on_demand_time_window: Some(1000), - arg_on_demand_start_backoff: Some(100), - arg_on_demand_end_backoff: Some(9000), - arg_on_demand_max_backoff_rounds: Some(100), + arg_on_demand_response_time_window: Some(2000), + arg_on_demand_request_time_window: Some(3000), + arg_on_demand_request_backoff_start: Some(9000), + arg_on_demand_request_backoff_max: Some(15000), + arg_on_demand_request_backoff_rounds_max: Some(10), // -- Whisper options. flag_whisper: false, @@ -2066,10 +2072,11 @@ mod tests { num_verifiers: None, }), light: Some(Light { - on_demand_time_window: Some(1000), - on_demand_start_backoff: Some(100), - on_demand_end_backoff: Some(10000), - on_demand_max_backoff_rounds: Some(10), + on_demand_response_time_window: Some(2000), + on_demand_request_time_window: Some(3000), + on_demand_request_backoff_start: Some(9000), + on_demand_request_backoff_max: Some(15000), + on_demand_request_backoff_rounds_max: Some(10), }), snapshots: Some(Snapshots { disable_periodic: Some(true), diff --git a/parity/cli/tests/config.full.toml b/parity/cli/tests/config.full.toml index ccab4b2aa4d..de0555a4d90 100644 --- a/parity/cli/tests/config.full.toml +++ b/parity/cli/tests/config.full.toml @@ -156,10 +156,11 @@ scale_verifiers = true num_verifiers = 6 [light] -on_demand_time_window = 1000 -on_demand_start_backoff = 100 -on_demand_end_backoff = 9000 -on_demand_max_backoff_rounds = 100 +on_demand_response_time_window = 1000 +on_demand_request_time_window = 1200 +on_demand_request_backoff_start = 1500 +on_demand_request_backoff_max = 9000 +on_demand_request_backoff_rounds_max = 100 [snapshots] disable_periodic = false diff --git a/parity/cli/tests/config.toml b/parity/cli/tests/config.toml index 97c2f6d172e..35a2df735cc 100644 --- a/parity/cli/tests/config.toml +++ b/parity/cli/tests/config.toml @@ -71,10 +71,11 @@ fat_db = "off" scale_verifiers = false [light] -on_demand_time_window = 1000 -on_demand_start_backoff = 100 -on_demand_end_backoff = 10000 -on_demand_max_backoff_rounds = 10 +on_demand_response_time_window = 1000 +on_demand_request_time_window = 1200 +on_demand_request_backoff_start = 2000 +on_demand_request_backoff_end = 10000 +on_demand_request_backoff_rounds_max = 10 [snapshots] disable_periodic = true diff --git a/parity/configuration.rs b/parity/configuration.rs index 7bc9d21d35a..d6420e07850 100644 --- a/parity/configuration.rs +++ b/parity/configuration.rs @@ -394,10 +394,11 @@ impl Configuration { whisper: whisper_config, no_hardcoded_sync: self.args.flag_no_hardcoded_sync, max_round_blocks_to_import: self.args.arg_max_round_blocks_to_import, - on_demand_time_window: self.args.arg_on_demand_time_window, - on_demand_start_backoff: self.args.arg_on_demand_start_backoff, - on_demand_end_backoff: self.args.arg_on_demand_end_backoff, - on_demand_max_backoff_rounds: self.args.arg_on_demand_max_backoff_rounds, + on_demand_response_time_window: self.args.arg_on_demand_response_time_window, + on_demand_request_time_window: self.args.arg_on_demand_request_time_window, + on_demand_request_backoff_start: self.args.arg_on_demand_request_backoff_start, + on_demand_request_backoff_max: self.args.arg_on_demand_request_backoff_max, + on_demand_request_backoff_rounds_max: self.args.arg_on_demand_request_backoff_rounds_max, }; Cmd::Run(run_cmd) }; @@ -1440,10 +1441,11 @@ mod tests { no_persistent_txqueue: false, whisper: Default::default(), max_round_blocks_to_import: 12, - on_demand_time_window: None, - on_demand_start_backoff: None, - on_demand_end_backoff: None, - on_demand_max_backoff_rounds: None, + on_demand_response_time_window: None, + on_demand_request_time_window: None, + on_demand_request_backoff_start: None, + on_demand_request_backoff_max: None, + on_demand_request_backoff_rounds_max: None, }; expected.secretstore_conf.enabled = cfg!(feature = "secretstore"); expected.secretstore_conf.http_enabled = cfg!(feature = "secretstore"); diff --git a/parity/run.rs b/parity/run.rs index fd73fbd4f16..bae044f5a47 100644 --- a/parity/run.rs +++ b/parity/run.rs @@ -134,10 +134,11 @@ pub struct RunCmd { pub whisper: ::whisper::Config, pub no_hardcoded_sync: bool, pub max_round_blocks_to_import: usize, - pub on_demand_time_window: Option, - pub on_demand_start_backoff: Option, - pub on_demand_end_backoff: Option, - pub on_demand_max_backoff_rounds: Option, + pub on_demand_request_time_window: Option, + pub on_demand_response_time_window: Option, + pub on_demand_request_backoff_start: Option, + pub on_demand_request_backoff_max: Option, + pub on_demand_request_backoff_rounds_max: Option, } // node info fetcher for the local store. @@ -217,28 +218,34 @@ fn execute_light_impl(cmd: RunCmd, logger: Arc) -> Result