Fold "circuit_timing" and "request_timing" into a single section.

This commit is contained in:
Nick Mathewson 2021-11-18 09:44:42 -05:00
parent 5184f5ba84
commit d592e86f9c
6 changed files with 33 additions and 86 deletions

View File

@ -13,7 +13,7 @@ pub use tor_config::ConfigBuildError;
pub mod circ {
pub use tor_circmgr::{
CircMgrConfig, CircMgrConfigBuilder, CircuitTiming, CircuitTimingBuilder, PathConfig,
PathConfigBuilder, RequestTiming, RequestTimingBuilder,
PathConfigBuilder,
};
}

View File

@ -82,16 +82,14 @@ microdesc_parallelism = 4
# IPv4 addresses begin with the same 16 bits.
enforce_distance = { "subnets_family_v4" = 16, "subnets_family_v6" = 32 }
# Rules for how long circuits should survive.
# Rules for how long circuits should survive, and how long pending
# requests should wait for a circuit.
[circuit_timing]
# Once a circuit has been used for a request, we stop giving it out for
# other requests after this time.
max_dirtiness = "10 minutes"
# Rules for how long pending requests should wait for a circuit.
[request_timing]
# When a circuit is requested, we keep trying to build circuits for up
# to this long before the request gives up.
request_timeout = "60 sec"

View File

@ -71,10 +71,7 @@ pub struct ArtiConfig {
/// Information about how to build paths through the network.
path_rules: arti_client::config::circ::PathConfig,
/// Information about how to retry requests for circuits.
request_timing: arti_client::config::circ::RequestTiming,
/// Information about how to expire circuits.
/// Information about how to retry and expire circuits and request for circuits.
circuit_timing: arti_client::config::circ::CircuitTiming,
/// Information about client address configuration parameters.
@ -128,7 +125,6 @@ impl ArtiConfig {
let mut builder = CircMgrConfigBuilder::default();
builder
.path_config(self.path_rules.clone())
.request_timing(self.request_timing.clone())
.circuit_timing(self.circuit_timing.clone())
.build()
}

View File

@ -11,44 +11,6 @@ use serde::Deserialize;
use std::time::Duration;
/// Configuration for circuit timeouts and retries.
///
/// This type is immutable once constructed. To create an object of this type,
/// use [`RequestTimingBuilder`], or deserialize it from a string.
#[derive(Debug, Clone, Builder, Deserialize)]
#[builder(build_fn(error = "ConfigBuildError"))]
pub struct RequestTiming {
/// When a circuit is requested, we stop retrying new circuits
/// after this much time.
// TODO: Impose a maximum or minimum?
#[builder(default = "Duration::from_secs(60)")]
#[serde(with = "humantime_serde")]
pub(crate) request_timeout: Duration,
/// When a circuit is requested, we stop retrying new circuits after
/// this many attempts.
// TODO: Impose a maximum or minimum?
#[builder(default = "32")]
pub(crate) request_max_retries: u32,
/// When waiting for requested circuits, wait at least this long
/// before using a suitable-looking circuit launched by some other
/// request.
#[builder(default = "Duration::from_millis(50)")]
#[serde(with = "humantime_serde")]
pub(crate) request_loyalty: Duration,
}
// NOTE: it seems that `unwrap` may be safe because of builder defaults
// check `derive_builder` documentation for details
// https://docs.rs/derive_builder/0.10.2/derive_builder/#default-values
#[allow(clippy::unwrap_used)]
impl Default for RequestTiming {
fn default() -> Self {
RequestTimingBuilder::default().build().unwrap()
}
}
/// Rules for building paths over the network.
///
/// This type is immutable once constructed. To build one, use
@ -74,6 +36,26 @@ pub struct CircuitTiming {
#[builder(default = "Duration::from_secs(60 * 10)")]
#[serde(with = "humantime_serde")]
pub(crate) max_dirtiness: Duration,
/// When a circuit is requested, we stop retrying new circuits
/// after this much time.
// TODO: Impose a maximum or minimum?
#[builder(default = "Duration::from_secs(60)")]
#[serde(with = "humantime_serde")]
pub(crate) request_timeout: Duration,
/// When a circuit is requested, we stop retrying new circuits after
/// this many attempts.
// TODO: Impose a maximum or minimum?
#[builder(default = "32")]
pub(crate) request_max_retries: u32,
/// When waiting for requested circuits, wait at least this long
/// before using a suitable-looking circuit launched by some other
/// request.
#[builder(default = "Duration::from_millis(50)")]
#[serde(with = "humantime_serde")]
pub(crate) request_loyalty: Duration,
}
// NOTE: it seems that `unwrap` may be safe because of builder defaults
@ -104,10 +86,6 @@ pub struct CircMgrConfig {
#[builder(default)]
pub(crate) path_config: PathConfig,
/// Timing and retry information related to requests for circuits.
#[builder(default)]
pub(crate) request_timing: RequestTiming,
/// Timing and retry information related to circuits themselves.
#[builder(default)]
pub(crate) circuit_timing: CircuitTiming,

View File

@ -74,7 +74,7 @@ pub use usage::{IsolationToken, StreamIsolation, StreamIsolationBuilder, TargetP
pub use config::{
CircMgrConfig, CircMgrConfigBuilder, CircuitTiming, CircuitTimingBuilder, PathConfig,
PathConfigBuilder, RequestTiming, RequestTimingBuilder,
PathConfigBuilder,
};
use usage::TargetCircUsage;
@ -164,7 +164,6 @@ impl<R: Runtime> CircMgr<R> {
{
let CircMgrConfig {
path_config,
request_timing,
circuit_timing,
} = config;
@ -179,8 +178,7 @@ impl<R: Runtime> CircMgr<R> {
storage_handle,
guardmgr,
);
let mgr =
mgr::AbstractCircMgr::new(builder, runtime.clone(), request_timing, circuit_timing);
let mgr = mgr::AbstractCircMgr::new(builder, runtime.clone(), circuit_timing);
let circmgr = Arc::new(CircMgr { mgr: Arc::new(mgr) });
runtime.spawn(continually_expire_circuits(

View File

@ -22,7 +22,7 @@
// - Error from prepare_action()
// - Error reported by restrict_mut?
use crate::config::{CircuitTiming, RequestTiming};
use crate::config::CircuitTiming;
use crate::{DirInfo, Error, Result};
use retry_error::RetryError;
@ -622,9 +622,7 @@ pub(crate) struct AbstractCircMgr<B: AbstractCircBuilder, R: Runtime> {
/// pending circuits.
circs: sync::Mutex<CircList<B>>,
/// Configured timing and retry rules for attaching requests to circuits.
request_timing: RequestTiming,
/// Configured information about when to expire circuits.
/// Configured information about when to expire circuits and requests.
circuit_timing: CircuitTiming,
/// Minimum lifetime of an unused circuit.
@ -645,12 +643,7 @@ enum Action<B: AbstractCircBuilder> {
impl<B: AbstractCircBuilder + 'static, R: Runtime> AbstractCircMgr<B, R> {
/// Construct a new AbstractCircMgr.
pub(crate) fn new(
builder: B,
runtime: R,
request_timing: RequestTiming,
circuit_timing: CircuitTiming,
) -> Self {
pub(crate) fn new(builder: B, runtime: R, circuit_timing: CircuitTiming) -> Self {
let circs = sync::Mutex::new(CircList::new());
let dflt_params = tor_netdir::params::NetParameters::default();
let unused_timing = (&dflt_params).into();
@ -658,7 +651,6 @@ impl<B: AbstractCircBuilder + 'static, R: Runtime> AbstractCircMgr<B, R> {
builder,
runtime,
circs,
request_timing,
circuit_timing,
unused_timing: sync::Mutex::new(unused_timing),
}
@ -683,9 +675,9 @@ impl<B: AbstractCircBuilder + 'static, R: Runtime> AbstractCircMgr<B, R> {
usage: &<B::Spec as AbstractSpec>::Usage,
dir: DirInfo<'_>,
) -> Result<Arc<B::Circ>> {
let wait_for_circ = self.request_timing.request_timeout;
let wait_for_circ = self.circuit_timing.request_timeout;
let timeout_at = self.runtime.now() + wait_for_circ;
let max_tries = self.request_timing.request_max_retries;
let max_tries = self.circuit_timing.request_max_retries;
let mut retry_err =
RetryError /* ::<Box<Error>> */::in_attempt_to("find or build a circuit");
@ -1035,7 +1027,7 @@ impl<B: AbstractCircBuilder + 'static, R: Runtime> AbstractCircMgr<B, R> {
// delay will give the circuits that were originally
// specifically intended for a request a little more time
// to finish, before we offer it this circuit instead.
let briefly = self.request_timing.request_loyalty;
let briefly = self.circuit_timing.request_loyalty;
let sl = runtime_copy.sleep(briefly);
runtime_copy.allow_one_advance(briefly);
sl.await;
@ -1345,7 +1337,6 @@ mod test {
let mgr = Arc::new(AbstractCircMgr::new(
builder,
rt.clone(),
RequestTiming::default(),
CircuitTiming::default(),
));
@ -1428,7 +1419,6 @@ mod test {
let mgr = Arc::new(AbstractCircMgr::new(
builder,
rt.clone(),
RequestTiming::default(),
CircuitTiming::default(),
));
let c1 = mgr
@ -1461,7 +1451,6 @@ mod test {
let mgr = Arc::new(AbstractCircMgr::new(
builder,
rt.clone(),
RequestTiming::default(),
CircuitTiming::default(),
));
let c1 = mgr
@ -1487,7 +1476,6 @@ mod test {
let mgr = Arc::new(AbstractCircMgr::new(
builder,
rt.clone(),
RequestTiming::default(),
CircuitTiming::default(),
));
let c1 = rt.wait_for(mgr.get_or_launch(&ports, di())).await;
@ -1509,7 +1497,6 @@ mod test {
let mgr = Arc::new(AbstractCircMgr::new(
builder,
rt.clone(),
RequestTiming::default(),
CircuitTiming::default(),
));
let c1 = rt.wait_for(mgr.get_or_launch(&ports, di())).await;
@ -1536,7 +1523,6 @@ mod test {
let mgr = Arc::new(AbstractCircMgr::new(
builder,
rt.clone(),
RequestTiming::default(),
CircuitTiming::default(),
));
let c1 = rt.wait_for(mgr.get_or_launch(&ports, di())).await;
@ -1560,7 +1546,6 @@ mod test {
let mgr = Arc::new(AbstractCircMgr::new(
builder,
rt.clone(),
RequestTiming::default(),
CircuitTiming::default(),
));
@ -1589,7 +1574,6 @@ mod test {
let mgr = Arc::new(AbstractCircMgr::new(
builder,
rt.clone(),
RequestTiming::default(),
CircuitTiming::default(),
));
@ -1657,7 +1641,6 @@ mod test {
let mgr = Arc::new(AbstractCircMgr::new(
builder,
rt.clone(),
RequestTiming::default(),
CircuitTiming::default(),
));
// Note that ports2 will be wider than ports1, so the second
@ -1692,7 +1675,6 @@ mod test {
let mgr = Arc::new(AbstractCircMgr::new(
builder,
rt.clone(),
RequestTiming::default(),
CircuitTiming::default(),
));
@ -1738,12 +1720,7 @@ mod test {
.build()
.unwrap();
let mgr = Arc::new(AbstractCircMgr::new(
builder,
rt.clone(),
RequestTiming::default(),
circuit_timing,
));
let mgr = Arc::new(AbstractCircMgr::new(builder, rt.clone(), circuit_timing));
let imap = FakeSpec::new(vec![993_u16]);
let pop = FakeSpec::new(vec![995_u16]);