From aedcb2e05b28f7fa35695be9c4127646dea5246e Mon Sep 17 00:00:00 2001 From: Antonio Santos Date: Thu, 16 May 2019 16:02:39 +0200 Subject: [PATCH] objstore: Increase the response header timeout in the S3 provider to 2 minutes and make it configurable (#1094) * Increase the response header timeout * Format * Make response header timeout configurable * Test default values for the HTTPConfig struct * Document new feature in the changelog * Add explanation for new option and refactor existing documentation * Run make docs * Typo * Remove redundant blank line --- CHANGELOG.md | 4 ++++ docs/storage.md | 18 ++++++++++---- pkg/objstore/s3/s3.go | 19 +++++++++------ pkg/objstore/s3/s3_test.go | 49 ++++++++++++++++++++++++++++++++------ 4 files changed, 71 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5e1dcad4ec..6a2903b9d4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,10 @@ We use *breaking* word for marking changes that are not backward compatible (rel ## Unreleased +### Added + +- [#1094](https://github.com/improbable-eng/thanos/pull/1094) Allow configuring the response header timeout for the S3 client. + ### Changed - [#1066](https://github.com/improbable-eng/thanos/pull/1066) Upgrade Thanos ui to Prometheus v2.9.1. diff --git a/docs/storage.md b/docs/storage.md index 8cd58a3dc7..38d02150fa 100644 --- a/docs/storage.md +++ b/docs/storage.md @@ -39,9 +39,9 @@ At that point, anyone can use your provider by spec ## AWS S3 configuration -Thanos uses minio client to upload Prometheus data into AWS S3. +Thanos uses the [minio client](https://github.com/minio/minio-go) library to upload Prometheus data into AWS S3. -To configure S3 bucket as an object store you need to set these mandatory S3 variables in yaml format stored in a file: +You can configure an S3 bucket as an object store with YAML, either by passing the configuration directly to the `--objstore.config` parameter, or (preferably) by passing the path to a configuration file to the `--objstore.config-file` option. [embedmd]:# (flags/config_s3.txt yaml) ```yaml @@ -58,20 +58,28 @@ config: put_user_metadata: {} http_config: idle_conn_timeout: 0s + response_header_timeout: 0s insecure_skip_verify: false trace: enable: false ``` -The attribute `region` is optional. AWS region to endpoint mapping can be found in this [link](https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region). +At a minimum, you will need to provide a value for the `bucket`, `endpoint`, `access_key`, and `secret_key` keys. The rest of the keys are optional. -Make sure you use a correct signature version. -Currently AWS require signature v4, so it needs `signature-version2: false`, otherwise, you will get Access Denied error, but several other S3 compatible use `signature-version2: true` +The AWS region to endpoint mapping can be found in this [link](https://docs.aws.amazon.com/general/latest/gr/rande.html#s3_region). + +Make sure you use a correct signature version. Currently AWS requires signature v4, so it needs `signature-version2: false`. If you don't specify it, you will get an `Access Denied` error. On the other hand, several S3 compatible APIs use `signature-version2: true`. + +You can configure the timeout settings for the HTTP client by setting the `http_config.idle_conn_timeout` and `http_config.response_header_timeout` keys. As a rule of thumb, if you are seeing errors like `timeout awaiting response headers` in your logs, you may want to increase the value of `http_config.response_header_timeout`. + +Please refer to the documentation of [the Transport type](https://golang.org/pkg/net/http/#Transport) in the `net/http` package for detailed information on what each option does. For debug and testing purposes you can set * `insecure: true` to switch to plain insecure HTTP instead of HTTPS + * `http_config.insecure_skip_verify: true` to disable TLS certificate verification (if your S3 based storage is using a self-signed certificate, for example) + * `trace.enable: true` to enable the minio client's verbose logging. Each request and response will be logged into the debug logger, so debug level logging must be enabled for this functionality. ### Credentials diff --git a/pkg/objstore/s3/s3.go b/pkg/objstore/s3/s3.go index e41ff96bae..48cf39fd77 100644 --- a/pkg/objstore/s3/s3.go +++ b/pkg/objstore/s3/s3.go @@ -53,8 +53,9 @@ type TraceConfig struct { // HTTPConfig stores the http.Transport configuration for the s3 minio client. type HTTPConfig struct { - IdleConnTimeout model.Duration `yaml:"idle_conn_timeout"` - InsecureSkipVerify bool `yaml:"insecure_skip_verify"` + IdleConnTimeout model.Duration `yaml:"idle_conn_timeout"` + ResponseHeaderTimeout model.Duration `yaml:"response_header_timeout"` + InsecureSkipVerify bool `yaml:"insecure_skip_verify"` } // Bucket implements the store.Bucket interface against s3-compatible APIs. @@ -68,7 +69,10 @@ type Bucket struct { // parseConfig unmarshals a buffer into a Config with default HTTPConfig values. func parseConfig(conf []byte) (Config, error) { - defaultHTTPConfig := HTTPConfig{IdleConnTimeout: model.Duration(90 * time.Second)} + defaultHTTPConfig := HTTPConfig{ + IdleConnTimeout: model.Duration(90 * time.Second), + ResponseHeaderTimeout: model.Duration(2 * time.Minute), + } config := Config{HTTPConfig: defaultHTTPConfig} if err := yaml.Unmarshal(conf, &config); err != nil { return Config{}, err @@ -139,10 +143,11 @@ func NewBucketWithConfig(logger log.Logger, config Config, component string) (*B IdleConnTimeout: time.Duration(config.HTTPConfig.IdleConnTimeout), TLSHandshakeTimeout: 10 * time.Second, ExpectContinueTimeout: 1 * time.Second, - // The ResponseHeaderTimeout here is the only change from the - // default minio transport, it was introduced to cover cases - // where the tcp connection works but the server never answers - ResponseHeaderTimeout: 15 * time.Second, + // The ResponseHeaderTimeout here is the only change + // from the default minio transport, it was introduced + // to cover cases where the tcp connection works but + // the server never answers. Defaults to 2 minutes. + ResponseHeaderTimeout: time.Duration(config.HTTPConfig.ResponseHeaderTimeout), // Set this value so that the underlying transport round-tripper // doesn't try to auto decode the body of objects with // content-encoding set to `gzip`. diff --git a/pkg/objstore/s3/s3_test.go b/pkg/objstore/s3/s3_test.go index c8d8204a46..9f47f0c496 100644 --- a/pkg/objstore/s3/s3_test.go +++ b/pkg/objstore/s3/s3_test.go @@ -7,12 +7,48 @@ import ( "github.com/improbable-eng/thanos/pkg/testutil" ) -func TestParseConfig_DefaultHTTPOpts(t *testing.T) { +func TestParseConfig(t *testing.T) { + input := []byte(`bucket: abcd +insecure: false`) + cfg, err := parseConfig(input) + testutil.Ok(t, err) + + if cfg.Bucket != "abcd" { + t.Errorf("parsing of bucket failed: got %v, expected %v", cfg.Bucket, "abcd") + } + if cfg.Insecure { + t.Errorf("parsing of insecure failed: got %v, expected %v", cfg.Insecure, false) + } +} + +func TestParseConfig_DefaultHTTPConfig(t *testing.T) { + input := []byte(`bucket: abcd +insecure: false`) + cfg, err := parseConfig(input) + testutil.Ok(t, err) + + if time.Duration(cfg.HTTPConfig.IdleConnTimeout) != time.Duration(90*time.Second) { + t.Errorf("parsing of idle_conn_timeout failed: got %v, expected %v", + time.Duration(cfg.HTTPConfig.IdleConnTimeout), time.Duration(90*time.Second)) + } + + if time.Duration(cfg.HTTPConfig.ResponseHeaderTimeout) != time.Duration(2*time.Minute) { + t.Errorf("parsing of response_header_timeout failed: got %v, expected %v", + time.Duration(cfg.HTTPConfig.IdleConnTimeout), time.Duration(2*time.Minute)) + } + + if cfg.HTTPConfig.InsecureSkipVerify { + t.Errorf("parsing of insecure_skip_verify failed: got %v, expected %v", cfg.HTTPConfig.InsecureSkipVerify, false) + } +} + +func TestParseConfig_CustomHTTPConfig(t *testing.T) { input := []byte(`bucket: abcd insecure: false http_config: insecure_skip_verify: true - idle_conn_timeout: 50s`) + idle_conn_timeout: 50s + response_header_timeout: 1m`) cfg, err := parseConfig(input) testutil.Ok(t, err) @@ -21,12 +57,11 @@ http_config: time.Duration(cfg.HTTPConfig.IdleConnTimeout), time.Duration(50*time.Second)) } - if cfg.Bucket != "abcd" { - t.Errorf("parsing of bucket failed: got %v, expected %v", cfg.Bucket, "abcd") - } - if cfg.Insecure { - t.Errorf("parsing of insecure failed: got %v, expected %v", cfg.Insecure, false) + if time.Duration(cfg.HTTPConfig.ResponseHeaderTimeout) != time.Duration(1*time.Minute) { + t.Errorf("parsing of response_header_timeout failed: got %v, expected %v", + time.Duration(cfg.HTTPConfig.IdleConnTimeout), time.Duration(1*time.Minute)) } + if !cfg.HTTPConfig.InsecureSkipVerify { t.Errorf("parsing of insecure_skip_verify failed: got %v, expected %v", cfg.HTTPConfig.InsecureSkipVerify, false) }