Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make idle connections configurable #56

Merged
merged 4 commits into from
Nov 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions src/client.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,10 @@ impl Client {
// Fix "Too many open files" and DNS errors (rate limit for DNS
// server) by choosing a sensible value for `pool_idle_timeout()`
// and `pool_max_idle_per_host()`.
.pool_idle_timeout(Duration::from_secs(5))
.pool_max_idle_per_host(100)
.pool_idle_timeout(Duration::from_millis(
config.settings.idle_connections_timeout,
))
.pool_max_idle_per_host(config.settings.max_idle_connections_per_host)
.build()
.map_err(BogrepError::CreateClient)?;
let throttler = Some(Throttler::new(request_throttling));
Expand Down
12 changes: 9 additions & 3 deletions src/cmd/configure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,9 @@ mod tests {
"cache_mode": "text",
"max_concurrent_requests": 100,
"request_timeout": 60000,
"request_throttling": 3000
"request_throttling": 3000,
"max_idle_connections_per_host": 100,
"idle_connections_timeout": 5000
}"#;
assert_eq!(actual_settings, expected_settings);
}
Expand All @@ -121,7 +123,9 @@ mod tests {
"cache_mode": "html",
"max_concurrent_requests": 100,
"request_timeout": 60000,
"request_throttling": 3000
"request_throttling": 3000,
"max_idle_connections_per_host": 100,
"idle_connections_timeout": 5000
}"#;
assert_eq!(actual_settings, expected_settings);
}
Expand All @@ -147,7 +151,9 @@ mod tests {
"cache_mode": "text",
"max_concurrent_requests": 100,
"request_timeout": 60000,
"request_throttling": 3000
"request_throttling": 3000,
"max_idle_connections_per_host": 100,
"idle_connections_timeout": 5000
}"#;
assert_eq!(actual_settings, expected_settings);
}
Expand Down
4 changes: 4 additions & 0 deletions src/cmd/fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,6 +148,10 @@
debug!("{err}");
failed_response += 1;
}
BogrepError::ParseHttpResponse(_) => {
debug!("{err}");
failed_response += 1;

Check warning on line 153 in src/cmd/fetch.rs

View check run for this annotation

Codecov / codecov/patch

src/cmd/fetch.rs#L152-L153

Added lines #L152 - L153 were not covered by tests
}
BogrepError::BinaryResponse => {
debug!("{err}");
binary_response += 1;
Expand Down
17 changes: 17 additions & 0 deletions src/settings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,12 @@
/// The default for `Settings::request_throttling`.
const REQUEST_THROTTLING_DEFAULT: u64 = 3_000;

/// The default for `Setting::max_idle_connections_per_host`.
const MAX_IDLE_CONNECTIONS_PER_HOST: usize = 100;

/// The default for `Setting::idle_connections_timeout`.
const IDLE_CONNECTIONS_TIMEOUT: u64 = 5_000;

/// Describes the settings used in Bogrep.
#[derive(Debug, Serialize, Deserialize, PartialEq, Clone)]
pub struct Settings {
Expand All @@ -35,6 +41,10 @@
pub request_timeout: u64,
/// The throttling between requests in milliseconds.
pub request_throttling: u64,
/// The maximum number of idle connections allowed in the connection pool.
pub max_idle_connections_per_host: usize,
/// The timeout for idle connections to be kept alive in milliseconds.
pub idle_connections_timeout: u64,
}

impl Default for Settings {
Expand All @@ -46,18 +56,23 @@
max_concurrent_requests: MAX_CONCURRENT_REQUESTS_DEFAULT,
request_timeout: REQUEST_TIMEOUT_DEFAULT,
request_throttling: REQUEST_THROTTLING_DEFAULT,
max_idle_connections_per_host: MAX_IDLE_CONNECTIONS_PER_HOST,
idle_connections_timeout: IDLE_CONNECTIONS_TIMEOUT,
}
}
}

impl Settings {
#[allow(clippy::too_many_arguments)]
pub fn new(
sources: Vec<RawSource>,
ignored_urls: Vec<String>,
cache_mode: CacheMode,
max_concurrent_requests: usize,
request_timeout: u64,
request_throttling: u64,
max_idle_connections_per_host: usize,
idle_connections_timeout: u64,

Check warning on line 75 in src/settings.rs

View check run for this annotation

Codecov / codecov/patch

src/settings.rs#L74-L75

Added lines #L74 - L75 were not covered by tests
) -> Self {
Self {
sources,
Expand All @@ -66,6 +81,8 @@
max_concurrent_requests,
request_timeout,
request_throttling,
max_idle_connections_per_host,
idle_connections_timeout,

Check warning on line 85 in src/settings.rs

View check run for this annotation

Codecov / codecov/patch

src/settings.rs#L84-L85

Added lines #L84 - L85 were not covered by tests
}
}

Expand Down