feat(CLI): separate CLI args request and response

ethcore/light/src/on_demand/mod.rs

@@ -60,13 +60,15 @@ pub type ExecutionResult = Result<Executed, ExecutionError>;
 /// 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,83 +330,68 @@ pub struct OnDemand {
 	in_transit: RwLock<HashMap<ReqId, Pending>>,
 	cache: Arc<Mutex<Cache>>,
 	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 {
 
 	/// Create a new `OnDemand` service with the given cache.
 	pub fn new(
 		cache: Arc<Mutex<Cache>>,
-		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
 	// until you trigger it manually.
 	#[cfg(test)]
 	fn new_test(
 		cache: Arc<Mutex<Cache>>,
-		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)

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);

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,

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<u64>) = None, or |c: &Config| c.light.as_ref()?.on_demand_time_window,
+			ARG arg_on_demand_response_time_window: (Option<u64>) = 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<u64>) = 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<u64>) = None, or |c: &Config| c.light.as_ref()?.on_demand_start_backoff,
+			ARG arg_on_demand_request_backoff_start: (Option<u64>) = 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<u64>) = None, or |c: &Config| c.light.as_ref()?.on_demand_end_backoff,
+			ARG arg_on_demand_request_backoff_max: (Option<u64>) = 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<usize>) = None, or |c: &Config| c.light.as_ref()?.on_demand_max_backoff_rounds,
+			ARG arg_on_demand_request_backoff_rounds_max: (Option<usize>) = 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<u64>,
-	on_demand_start_backoff: Option<u64>,
-	on_demand_end_backoff: Option<u64>,
-	on_demand_max_backoff_rounds: Option<usize>,
+	on_demand_response_time_window: Option<u64>,
+	on_demand_request_time_window: Option<u64>,
+	on_demand_request_backoff_start: Option<u64>,
+	on_demand_request_backoff_max: Option<u64>,
+	on_demand_request_backoff_rounds_max: Option<usize>
 }
 
 #[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),

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

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

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");