From 1208bf1769f493f475e1e19a66f8002120b7fb1d Mon Sep 17 00:00:00 2001 From: Jacob Lisi Date: Sun, 27 Sep 2020 12:48:43 -0400 Subject: [PATCH 1/4] feat: expose S3 HTTP configs for TSDB S3 client Signed-off-by: Jacob Lisi --- docs/blocks-storage/querier.md | 14 +++++++++++++ docs/blocks-storage/store-gateway.md | 14 +++++++++++++ docs/configuration/config-file-reference.md | 14 +++++++++++++ pkg/storage/backend/s3/bucket_client.go | 7 +++++++ pkg/storage/backend/s3/config.go | 22 +++++++++++++++++++++ 5 files changed, 71 insertions(+) diff --git a/docs/blocks-storage/querier.md b/docs/blocks-storage/querier.md index 6a03b97a3a4..f321d54065c 100644 --- a/docs/blocks-storage/querier.md +++ b/docs/blocks-storage/querier.md @@ -210,6 +210,20 @@ blocks_storage: # CLI flag: -blocks-storage.s3.insecure [insecure: | default = false] + http_config: + # If set, the time an idle connection will remain idle before closing. + # CLI flag: -blocks-storage.s3.http.idle-conn-timeout + [idle_conn_timeout: | default = 0s] + + # If set, it specifies the amount of time the client will wait for a + # servers response headers. + # CLI flag: -blocks-storage.s3.http.response-header-timeout + [response_header_timeout: | default = 0s] + + # If enabled, the client will accept any certificate and hostname. + # CLI flag: -blocks-storage.s3.http.insecure-skip-verify + [insecure_skip_verify: | default = false] + gcs: # GCS bucket name # CLI flag: -blocks-storage.gcs.bucket-name diff --git a/docs/blocks-storage/store-gateway.md b/docs/blocks-storage/store-gateway.md index 38c15b6437b..809ae6ea911 100644 --- a/docs/blocks-storage/store-gateway.md +++ b/docs/blocks-storage/store-gateway.md @@ -270,6 +270,20 @@ blocks_storage: # CLI flag: -blocks-storage.s3.insecure [insecure: | default = false] + http_config: + # If set, the time an idle connection will remain idle before closing. + # CLI flag: -blocks-storage.s3.http.idle-conn-timeout + [idle_conn_timeout: | default = 0s] + + # If set, it specifies the amount of time the client will wait for a + # servers response headers. + # CLI flag: -blocks-storage.s3.http.response-header-timeout + [response_header_timeout: | default = 0s] + + # If enabled, the client will accept any certificate and hostname. + # CLI flag: -blocks-storage.s3.http.insecure-skip-verify + [insecure_skip_verify: | default = false] + gcs: # GCS bucket name # CLI flag: -blocks-storage.gcs.bucket-name diff --git a/docs/configuration/config-file-reference.md b/docs/configuration/config-file-reference.md index 7b0cf208f97..fd8477bf3fc 100644 --- a/docs/configuration/config-file-reference.md +++ b/docs/configuration/config-file-reference.md @@ -3154,6 +3154,20 @@ s3: # CLI flag: -blocks-storage.s3.insecure [insecure: | default = false] + http_config: + # If set, the time an idle connection will remain idle before closing. + # CLI flag: -blocks-storage.s3.http.idle-conn-timeout + [idle_conn_timeout: | default = 0s] + + # If set, it specifies the amount of time the client will wait for a servers + # response headers. + # CLI flag: -blocks-storage.s3.http.response-header-timeout + [response_header_timeout: | default = 0s] + + # If enabled, the client will accept any certificate and hostname. + # CLI flag: -blocks-storage.s3.http.insecure-skip-verify + [insecure_skip_verify: | default = false] + gcs: # GCS bucket name # CLI flag: -blocks-storage.gcs.bucket-name diff --git a/pkg/storage/backend/s3/bucket_client.go b/pkg/storage/backend/s3/bucket_client.go index c9f54a099e6..5d3d7348857 100644 --- a/pkg/storage/backend/s3/bucket_client.go +++ b/pkg/storage/backend/s3/bucket_client.go @@ -2,6 +2,7 @@ package s3 import ( "github.com/go-kit/kit/log" + "github.com/prometheus/common/model" "github.com/thanos-io/thanos/pkg/objstore" "github.com/thanos-io/thanos/pkg/objstore/s3" ) @@ -23,5 +24,11 @@ func newS3Config(cfg Config) s3.Config { AccessKey: cfg.AccessKeyID, SecretKey: cfg.SecretAccessKey.Value, Insecure: cfg.Insecure, + HTTPConfig: s3.HTTPConfig{ + IdleConnTimeout: model.Duration(cfg.HTTP.IdleConnTimeout), + ResponseHeaderTimeout: model.Duration(cfg.HTTP.ResponseHeaderTimeout), + InsecureSkipVerify: cfg.HTTP.InsecureSkipVerify, + Transport: cfg.HTTP.Transport, + }, } } diff --git a/pkg/storage/backend/s3/config.go b/pkg/storage/backend/s3/config.go index 6677723cfef..afab1e95c55 100644 --- a/pkg/storage/backend/s3/config.go +++ b/pkg/storage/backend/s3/config.go @@ -2,10 +2,29 @@ package s3 import ( "flag" + "net/http" + "time" "github.com/cortexproject/cortex/pkg/util/flagext" ) +// HTTPConfig stores the http.Transport configuration for the s3 minio client. +type HTTPConfig struct { + IdleConnTimeout time.Duration `yaml:"idle_conn_timeout"` + ResponseHeaderTimeout time.Duration `yaml:"response_header_timeout"` + InsecureSkipVerify bool `yaml:"insecure_skip_verify"` + + // Allow upstream callers to inject a round tripper + Transport http.RoundTripper `yaml:"-"` +} + +// RegisterFlagsWithPrefix registers the flags for TSDB s3 storage with the provided prefix +func (cfg *HTTPConfig) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) { + f.DurationVar(&cfg.IdleConnTimeout, prefix+"s3.http.idle-conn-timeout", 0, "If set, the time an idle connection will remain idle before closing.") + f.DurationVar(&cfg.ResponseHeaderTimeout, prefix+"s3.http.response-header-timeout", 0, "If set, it specifies the amount of time the client will wait for a servers response headers.") + f.BoolVar(&cfg.InsecureSkipVerify, prefix+"s3.http.insecure-skip-verify", false, "If enabled, the client will accept any certificate and hostname.") +} + // Config holds the config options for an S3 backend type Config struct { Endpoint string `yaml:"endpoint"` @@ -13,6 +32,8 @@ type Config struct { SecretAccessKey flagext.Secret `yaml:"secret_access_key"` AccessKeyID string `yaml:"access_key_id"` Insecure bool `yaml:"insecure"` + + HTTP HTTPConfig `yaml:"http_config"` } // RegisterFlags registers the flags for TSDB s3 storage with the provided prefix @@ -27,4 +48,5 @@ func (cfg *Config) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) { f.StringVar(&cfg.BucketName, prefix+"s3.bucket-name", "", "S3 bucket name") f.StringVar(&cfg.Endpoint, prefix+"s3.endpoint", "", "The S3 bucket endpoint. It could be an AWS S3 endpoint listed at https://docs.aws.amazon.com/general/latest/gr/s3.html or the address of an S3-compatible service in hostname:port format.") f.BoolVar(&cfg.Insecure, prefix+"s3.insecure", false, "If enabled, use http:// for the S3 endpoint instead of https://. This could be useful in local dev/test environments while using an S3-compatible backend storage, like Minio.") + cfg.HTTP.RegisterFlagsWithPrefix(prefix, f) } From 0c4dbccc734c432335ecb6a9796f372c090ddc5d Mon Sep 17 00:00:00 2001 From: Jacob Lisi Date: Sun, 27 Sep 2020 12:54:29 -0400 Subject: [PATCH 2/4] update changelog Signed-off-by: Jacob Lisi --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index bca1603d828..cb5026836a3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,10 @@ - `POST /ingester/push` * [FEATURE] Added support for shuffle-sharding queriers in the query-frontend. When configured (`-frontend.max-queriers-per-user` globally, or using per-user limit `max_queriers_per_user`), each user's requests will be handled by different set of queriers. #3113 * [FEATURE] Query-frontend: added `compression` config to support results cache with compression. #3217 +* [ENHANCEMENT] Expose additional HTTP configs for the S3 backend client. New flag are listed below: #3244 + - `-blocks-storage.s3.http.idle-conn-timeout` + - `-blocks-storage.s3.http.response-header-timeout` + - `-blocks-storage.s3.http.insecure-skip-verify` * [ENHANCEMENT] Added `cortex_query_frontend_connected_clients` metric to show the number of workers currently connected to the frontend. #3207 * [ENHANCEMENT] Shuffle sharding: improved shuffle sharding in the write path. Shuffle sharding now should be explicitly enabled via `-distributor.sharding-strategy` CLI flag (or its respective YAML config option) and guarantees stability, consistency, shuffling and balanced zone-awareness properties. #3090 #3214 * [ENHANCEMENT] Ingester: added new metric `cortex_ingester_active_series` to track active series more accurately. Also added options to control whether active series tracking is enabled (`-ingester.active-series-enabled`, defaults to false), and how often this metric is updated (`-ingester.active-series-update-period`) and max idle time for series to be considered inactive (`-ingester.active-series-idle-timeout`). #3153 From e25d5fa0a874e8a3e71cb4e0a6589faaddc1e5d2 Mon Sep 17 00:00:00 2001 From: Jacob Lisi Date: Mon, 28 Sep 2020 12:07:52 -0400 Subject: [PATCH 3/4] set http configs to Thanos default values Signed-off-by: Jacob Lisi --- CHANGELOG.md | 3 +++ pkg/storage/backend/s3/config.go | 4 ++-- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cb5026836a3..4cdb855a6ed 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,9 @@ ## master / unreleased +* [CHANGE] Update the default HTTP configuration values for the S3 client to the upstream Thanos default values. #3244 + - `-blocks-storage.s3.http.idle-conn-timeout` is set 90 seconds. + - `-blocks-storage.s3.http.response-header-timeout` is set to 2 minutes. * [CHANGE] Improved shuffle sharding support in the write path. This work introduced some config changes: #3090 * Introduced `-distributor.sharding-strategy` CLI flag (and its respective `sharding_strategy` YAML config option) to explicitly specify which sharding strategy should be used in the write path * `-experimental.distributor.user-subring-size` flag renamed to `-distributor.ingestion-tenant-shard-size` diff --git a/pkg/storage/backend/s3/config.go b/pkg/storage/backend/s3/config.go index afab1e95c55..f2ae22b96cf 100644 --- a/pkg/storage/backend/s3/config.go +++ b/pkg/storage/backend/s3/config.go @@ -20,8 +20,8 @@ type HTTPConfig struct { // RegisterFlagsWithPrefix registers the flags for TSDB s3 storage with the provided prefix func (cfg *HTTPConfig) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) { - f.DurationVar(&cfg.IdleConnTimeout, prefix+"s3.http.idle-conn-timeout", 0, "If set, the time an idle connection will remain idle before closing.") - f.DurationVar(&cfg.ResponseHeaderTimeout, prefix+"s3.http.response-header-timeout", 0, "If set, it specifies the amount of time the client will wait for a servers response headers.") + f.DurationVar(&cfg.IdleConnTimeout, prefix+"s3.http.idle-conn-timeout", 90*time.Second, "The time an idle connection will remain idle before closing.") + f.DurationVar(&cfg.ResponseHeaderTimeout, prefix+"s3.http.response-header-timeout", 2*time.Minute, "The amount of time the client will wait for a servers response headers.") f.BoolVar(&cfg.InsecureSkipVerify, prefix+"s3.http.insecure-skip-verify", false, "If enabled, the client will accept any certificate and hostname.") } From bf8dc02495f7bdbf9d8574d53e5f6c19891972c6 Mon Sep 17 00:00:00 2001 From: Jacob Lisi Date: Mon, 28 Sep 2020 12:38:30 -0400 Subject: [PATCH 4/4] fixup per PR comments Signed-off-by: Jacob Lisi --- CHANGELOG.md | 2 +- docs/blocks-storage/querier.md | 14 +++++++------- docs/blocks-storage/store-gateway.md | 14 +++++++------- docs/configuration/config-file-reference.md | 14 +++++++------- pkg/storage/backend/s3/config.go | 4 ++-- 5 files changed, 24 insertions(+), 24 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4cdb855a6ed..a5511a91578 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,7 +2,7 @@ ## master / unreleased -* [CHANGE] Update the default HTTP configuration values for the S3 client to the upstream Thanos default values. #3244 +* [CHANGE] Blocks storage: update the default HTTP configuration values for the S3 client to the upstream Thanos default values. #3244 - `-blocks-storage.s3.http.idle-conn-timeout` is set 90 seconds. - `-blocks-storage.s3.http.response-header-timeout` is set to 2 minutes. * [CHANGE] Improved shuffle sharding support in the write path. This work introduced some config changes: #3090 diff --git a/docs/blocks-storage/querier.md b/docs/blocks-storage/querier.md index f321d54065c..6d805213cf3 100644 --- a/docs/blocks-storage/querier.md +++ b/docs/blocks-storage/querier.md @@ -210,17 +210,17 @@ blocks_storage: # CLI flag: -blocks-storage.s3.insecure [insecure: | default = false] - http_config: - # If set, the time an idle connection will remain idle before closing. + http: + # The time an idle connection will remain idle before closing. # CLI flag: -blocks-storage.s3.http.idle-conn-timeout - [idle_conn_timeout: | default = 0s] + [idle_conn_timeout: | default = 1m30s] - # If set, it specifies the amount of time the client will wait for a - # servers response headers. + # The amount of time the client will wait for a servers response headers. # CLI flag: -blocks-storage.s3.http.response-header-timeout - [response_header_timeout: | default = 0s] + [response_header_timeout: | default = 2m] - # If enabled, the client will accept any certificate and hostname. + # If the client connects to S3 via HTTPS and this option is enabled, the + # client will accept any certificate and hostname. # CLI flag: -blocks-storage.s3.http.insecure-skip-verify [insecure_skip_verify: | default = false] diff --git a/docs/blocks-storage/store-gateway.md b/docs/blocks-storage/store-gateway.md index 809ae6ea911..e83f96b239b 100644 --- a/docs/blocks-storage/store-gateway.md +++ b/docs/blocks-storage/store-gateway.md @@ -270,17 +270,17 @@ blocks_storage: # CLI flag: -blocks-storage.s3.insecure [insecure: | default = false] - http_config: - # If set, the time an idle connection will remain idle before closing. + http: + # The time an idle connection will remain idle before closing. # CLI flag: -blocks-storage.s3.http.idle-conn-timeout - [idle_conn_timeout: | default = 0s] + [idle_conn_timeout: | default = 1m30s] - # If set, it specifies the amount of time the client will wait for a - # servers response headers. + # The amount of time the client will wait for a servers response headers. # CLI flag: -blocks-storage.s3.http.response-header-timeout - [response_header_timeout: | default = 0s] + [response_header_timeout: | default = 2m] - # If enabled, the client will accept any certificate and hostname. + # If the client connects to S3 via HTTPS and this option is enabled, the + # client will accept any certificate and hostname. # CLI flag: -blocks-storage.s3.http.insecure-skip-verify [insecure_skip_verify: | default = false] diff --git a/docs/configuration/config-file-reference.md b/docs/configuration/config-file-reference.md index fd8477bf3fc..02598487395 100644 --- a/docs/configuration/config-file-reference.md +++ b/docs/configuration/config-file-reference.md @@ -3154,17 +3154,17 @@ s3: # CLI flag: -blocks-storage.s3.insecure [insecure: | default = false] - http_config: - # If set, the time an idle connection will remain idle before closing. + http: + # The time an idle connection will remain idle before closing. # CLI flag: -blocks-storage.s3.http.idle-conn-timeout - [idle_conn_timeout: | default = 0s] + [idle_conn_timeout: | default = 1m30s] - # If set, it specifies the amount of time the client will wait for a servers - # response headers. + # The amount of time the client will wait for a servers response headers. # CLI flag: -blocks-storage.s3.http.response-header-timeout - [response_header_timeout: | default = 0s] + [response_header_timeout: | default = 2m] - # If enabled, the client will accept any certificate and hostname. + # If the client connects to S3 via HTTPS and this option is enabled, the + # client will accept any certificate and hostname. # CLI flag: -blocks-storage.s3.http.insecure-skip-verify [insecure_skip_verify: | default = false] diff --git a/pkg/storage/backend/s3/config.go b/pkg/storage/backend/s3/config.go index f2ae22b96cf..f4e9874708b 100644 --- a/pkg/storage/backend/s3/config.go +++ b/pkg/storage/backend/s3/config.go @@ -22,7 +22,7 @@ type HTTPConfig struct { func (cfg *HTTPConfig) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) { f.DurationVar(&cfg.IdleConnTimeout, prefix+"s3.http.idle-conn-timeout", 90*time.Second, "The time an idle connection will remain idle before closing.") f.DurationVar(&cfg.ResponseHeaderTimeout, prefix+"s3.http.response-header-timeout", 2*time.Minute, "The amount of time the client will wait for a servers response headers.") - f.BoolVar(&cfg.InsecureSkipVerify, prefix+"s3.http.insecure-skip-verify", false, "If enabled, the client will accept any certificate and hostname.") + f.BoolVar(&cfg.InsecureSkipVerify, prefix+"s3.http.insecure-skip-verify", false, "If the client connects to S3 via HTTPS and this option is enabled, the client will accept any certificate and hostname.") } // Config holds the config options for an S3 backend @@ -33,7 +33,7 @@ type Config struct { AccessKeyID string `yaml:"access_key_id"` Insecure bool `yaml:"insecure"` - HTTP HTTPConfig `yaml:"http_config"` + HTTP HTTPConfig `yaml:"http"` } // RegisterFlags registers the flags for TSDB s3 storage with the provided prefix