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

feat(config): Make the connection timeout configurable #688

Merged
merged 4 commits into from
Jul 29, 2020
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
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

**Features**:

- Add the `http.connection_timeout` configuration option to adjust the connection and SSL handshake timeout. The default connect timeout is now increased from 1s to 3s. ([#688](https://github.com/getsentry/relay/pull/688))

**Bug Fixes**:

- Reuse connections for upstream event submission requests when the server supports connection keepalive. Relay did not consume the response body of all requests, which caused it to reopen a new connection for every event. ([#680](https://github.com/getsentry/relay/pull/680))
Expand Down
14 changes: 14 additions & 0 deletions docs/configuration/options.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,20 @@ Set various network-related settings.

Timeout for upstream requests in seconds.

This timeout covers the time from sending the request until receiving response
headers. Neither the connection process and handshakes, nor reading the response
body is covered in this timeout.

### `http.connection_timeout`

*Integer, default: `3`*

Timeout for establishing connections with the upstream in seconds.

This includes SSL handshakes. Relay reuses connections when the upstream
supports connection keep-alive. Connections are retained for a maximum 75
seconds, or 15 seconds of inactivity.

### `http.max_retry_interval`

*Integer, default: `60`*
Expand Down
15 changes: 15 additions & 0 deletions relay-config/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -510,7 +510,16 @@ impl Default for Limits {
#[serde(default)]
struct Http {
/// Timeout for upstream requests in seconds.
///
/// This timeout covers the time from sending the request until receiving response headers.
/// Neither the connection process and handshakes, nor reading the response body is covered in
/// this timeout.
timeout: u32,
/// Timeout for establishing connections with the upstream in seconds.
///
/// This includes SSL handshakes. Relay reuses connections when the upstream supports connection
/// keep-alive. Connections are retained for a maximum 75 seconds, or 15 seconds of inactivity.
connection_timeout: u32,
/// Maximum interval between failed request retries in seconds.
max_retry_interval: u32,
/// The custom HTTP Host header to send to the upstream.
Expand All @@ -521,6 +530,7 @@ impl Default for Http {
fn default() -> Self {
Http {
timeout: 5,
connection_timeout: 3,
max_retry_interval: 60,
host_header: None,
}
Expand Down Expand Up @@ -1173,6 +1183,11 @@ impl Config {
Duration::from_secs(self.values.http.timeout.into())
}

/// Returns the connection timeout for all upstream HTTP requests.
pub fn http_connection_timeout(&self) -> Duration {
Duration::from_secs(self.values.http.connection_timeout.into())
}

/// Returns the failed upstream request retry interval.
pub fn http_max_retry_interval(&self) -> Duration {
Duration::from_secs(self.values.http.max_retry_interval.into())
Expand Down
1 change: 1 addition & 0 deletions relay-server/src/actors/upstream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,7 @@ impl UpstreamRelay {
// 1. Have own connection pool for `store` requests
// 2. Buffer up/queue/synchronize events before creating the request
.wait_timeout(self.config.event_buffer_expiry())
.conn_timeout(self.config.http_connection_timeout())
// This is the timeout after wait + connect.
.timeout(self.config.http_timeout())
.map_err(UpstreamRequestError::SendFailed)
Expand Down
11 changes: 8 additions & 3 deletions relay-server/src/endpoints/forward.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,13 +96,18 @@ fn forward_upstream(request: &HttpRequest<ServiceState>) -> ResponseFuture<HttpR
.uri(upstream.get_url(path_and_query))
.set_header("Host", host_header)
.set_header("X-Forwarded-For", ForwardedFor::from(request))
.set_header("Connection", "close")
.timeout(config.http_timeout());
.set_header("Connection", "close");

ForwardBody::new(request, limit)
.map_err(Error::from)
.and_then(move |data| forwarded_request_builder.body(data).map_err(Error::from))
.and_then(move |request| request.send().map_err(Error::from))
.and_then(move |request| {
request
.send()
.conn_timeout(config.http_connection_timeout())
.timeout(config.http_timeout())
.map_err(Error::from)
})
.and_then(move |response| {
let mut forwarded_response = HttpResponse::build(response.status());

Expand Down