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

add memcached auto-discovery support from thanos #4412

Merged
merged 2 commits into from
Sep 20, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
* [ENHANCEMENT] Exemplars are now emitted for all gRPC calls and many operations tracked by histograms. #4462
* [ENHANCEMENT] New options `-server.http-listen-network` and `-server.grpc-listen-network` allow binding as 'tcp4' or 'tcp6'. #4462
* [ENHANCEMENT] Rulers: Using shuffle sharding subring on GetRules API. #4466
* [ENHANCEMENT] Support memcached auto-discovery via `auto-discovery` flag, introduced by thanos in https://github.com/thanos-io/thanos/pull/4487. Both AWS and Google Cloud memcached service support auto-discovery, which returns a list of nodes of the memcached cluster. #4412
* [BUGFIX] Fixes a panic in the query-tee when comparing result. #4465
* [BUGFIX] Frontend: Fixes @ modifier functions (start/end) when splitting queries by time. #4464
* [BUGFIX] Compactor: compactor will no longer try to compact blocks that are already marked for deletion. Previously compactor would consider blocks marked for deletion within `-compactor.deletion-delay / 2` period as eligible for compaction. #4328
Expand Down
12 changes: 12 additions & 0 deletions docs/blocks-storage/querier.md
Original file line number Diff line number Diff line change
Expand Up @@ -512,6 +512,10 @@ blocks_storage:
# CLI flag: -blocks-storage.bucket-store.index-cache.memcached.max-item-size
[max_item_size: <int> | default = 1048576]

# Use memcached auto-discovery instead of DNS resolution
# CLI flag: -blocks-storage.bucket-store.index-cache.memcached.auto-discovery
[auto_discovery: <boolean> | default = false]

chunks_cache:
# Backend for chunks cache, if not empty. Supported values: memcached.
# CLI flag: -blocks-storage.bucket-store.chunks-cache.backend
Expand Down Expand Up @@ -559,6 +563,10 @@ blocks_storage:
# CLI flag: -blocks-storage.bucket-store.chunks-cache.memcached.max-item-size
[max_item_size: <int> | default = 1048576]

# Use memcached auto-discovery instead of DNS resolution
# CLI flag: -blocks-storage.bucket-store.chunks-cache.memcached.auto-discovery
[auto_discovery: <boolean> | default = false]

# Size of each subrange that bucket object is split into for better
# caching.
# CLI flag: -blocks-storage.bucket-store.chunks-cache.subrange-size
Expand Down Expand Up @@ -625,6 +633,10 @@ blocks_storage:
# CLI flag: -blocks-storage.bucket-store.metadata-cache.memcached.max-item-size
[max_item_size: <int> | default = 1048576]

# Use memcached auto-discovery instead of DNS resolution
# CLI flag: -blocks-storage.bucket-store.metadata-cache.memcached.auto-discovery
[auto_discovery: <boolean> | default = false]

# How long to cache list of tenants in the bucket.
# CLI flag: -blocks-storage.bucket-store.metadata-cache.tenants-list-ttl
[tenants_list_ttl: <duration> | default = 15m]
Expand Down
12 changes: 12 additions & 0 deletions docs/blocks-storage/store-gateway.md
Original file line number Diff line number Diff line change
Expand Up @@ -576,6 +576,10 @@ blocks_storage:
# CLI flag: -blocks-storage.bucket-store.index-cache.memcached.max-item-size
[max_item_size: <int> | default = 1048576]

# Use memcached auto-discovery instead of DNS resolution
# CLI flag: -blocks-storage.bucket-store.index-cache.memcached.auto-discovery
[auto_discovery: <boolean> | default = false]

chunks_cache:
# Backend for chunks cache, if not empty. Supported values: memcached.
# CLI flag: -blocks-storage.bucket-store.chunks-cache.backend
Expand Down Expand Up @@ -623,6 +627,10 @@ blocks_storage:
# CLI flag: -blocks-storage.bucket-store.chunks-cache.memcached.max-item-size
[max_item_size: <int> | default = 1048576]

# Use memcached auto-discovery instead of DNS resolution
# CLI flag: -blocks-storage.bucket-store.chunks-cache.memcached.auto-discovery
[auto_discovery: <boolean> | default = false]

# Size of each subrange that bucket object is split into for better
# caching.
# CLI flag: -blocks-storage.bucket-store.chunks-cache.subrange-size
Expand Down Expand Up @@ -689,6 +697,10 @@ blocks_storage:
# CLI flag: -blocks-storage.bucket-store.metadata-cache.memcached.max-item-size
[max_item_size: <int> | default = 1048576]

# Use memcached auto-discovery instead of DNS resolution
# CLI flag: -blocks-storage.bucket-store.metadata-cache.memcached.auto-discovery
[auto_discovery: <boolean> | default = false]

# How long to cache list of tenants in the bucket.
# CLI flag: -blocks-storage.bucket-store.metadata-cache.tenants-list-ttl
[tenants_list_ttl: <duration> | default = 15m]
Expand Down
2 changes: 2 additions & 0 deletions docs/configuration/arguments.md
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,8 @@ The DNS service discovery, inspired from Thanos DNS SD, supports different disco

If **no prefix** is provided, the provided IP or hostname will be used straightaway without pre-resolving it.

If you are using a managed memcached service from [Google Cloud](https://cloud.google.com/memorystore/docs/memcached/auto-discovery-overview), or [AWS](https://docs.aws.amazon.com/AmazonElastiCache/latest/mem-ug/AutoDiscovery.HowAutoDiscoveryWorks.html), use the [auto-discovery](./config-file-reference.md#memcached-client-config) flag instead of DNS discovery.

## Logging of IP of reverse proxy

If a reverse proxy is used in front of Cortex it might be diffult to troubleshoot errors. The following 3 settings can be used to log the IP address passed along by the reverse proxy in headers like X-Forwarded-For.
Expand Down
12 changes: 12 additions & 0 deletions docs/configuration/config-file-reference.md
Original file line number Diff line number Diff line change
Expand Up @@ -4765,6 +4765,10 @@ bucket_store:
# CLI flag: -blocks-storage.bucket-store.index-cache.memcached.max-item-size
[max_item_size: <int> | default = 1048576]

# Use memcached auto-discovery instead of DNS resolution
# CLI flag: -blocks-storage.bucket-store.index-cache.memcached.auto-discovery
[auto_discovery: <boolean> | default = false]

chunks_cache:
# Backend for chunks cache, if not empty. Supported values: memcached.
# CLI flag: -blocks-storage.bucket-store.chunks-cache.backend
Expand Down Expand Up @@ -4812,6 +4816,10 @@ bucket_store:
# CLI flag: -blocks-storage.bucket-store.chunks-cache.memcached.max-item-size
[max_item_size: <int> | default = 1048576]

# Use memcached auto-discovery instead of DNS resolution
# CLI flag: -blocks-storage.bucket-store.chunks-cache.memcached.auto-discovery
[auto_discovery: <boolean> | default = false]

# Size of each subrange that bucket object is split into for better caching.
# CLI flag: -blocks-storage.bucket-store.chunks-cache.subrange-size
[subrange_size: <int> | default = 16000]
Expand Down Expand Up @@ -4877,6 +4885,10 @@ bucket_store:
# CLI flag: -blocks-storage.bucket-store.metadata-cache.memcached.max-item-size
[max_item_size: <int> | default = 1048576]

# Use memcached auto-discovery instead of DNS resolution
# CLI flag: -blocks-storage.bucket-store.metadata-cache.memcached.auto-discovery
[auto_discovery: <boolean> | default = false]

# How long to cache list of tenants in the bucket.
# CLI flag: -blocks-storage.bucket-store.metadata-cache.tenants-list-ttl
[tenants_list_ttl: <duration> | default = 15m]
Expand Down
2 changes: 1 addition & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ require (
github.com/sony/gobreaker v0.4.1
github.com/spf13/afero v1.2.2
github.com/stretchr/testify v1.7.0
github.com/thanos-io/thanos v0.22.0
github.com/thanos-io/thanos v0.19.1-0.20210803192524-baea4ce9ef52
github.com/uber/jaeger-client-go v2.29.1+incompatible
github.com/weaveworks/common v0.0.0-20210901124008-1fa3f9fa874c
go.etcd.io/bbolt v1.3.6
Expand Down
5 changes: 3 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1715,8 +1715,8 @@ github.com/thanos-io/thanos v0.13.1-0.20210204123931-82545cdd16fe/go.mod h1:ZLDG
github.com/thanos-io/thanos v0.13.1-0.20210224074000-659446cab117/go.mod h1:kdqFpzdkveIKpNNECVJd75RPvgsAifQgJymwCdfev1w=
github.com/thanos-io/thanos v0.13.1-0.20210226164558-03dace0a1aa1/go.mod h1:gMCy4oCteKTT7VuXVvXLTPGzzjovX1VPE5p+HgL1hyU=
github.com/thanos-io/thanos v0.13.1-0.20210401085038-d7dff0c84d17/go.mod h1:zU8KqE+6A+HksK4wiep8e/3UvCZLm+Wrw9AqZGaAm9k=
github.com/thanos-io/thanos v0.22.0 h1:bHTzC0ZaP5rBJ2pOeJ73+hO/p3U4tSshJoR3pumM6sA=
github.com/thanos-io/thanos v0.22.0/go.mod h1:SZDWz3phcUcBr4MYFoPFRvl+Z9Nbi45HlwQlwSZSt+Q=
github.com/thanos-io/thanos v0.19.1-0.20210803192524-baea4ce9ef52 h1:aRxV+ebdkd5XqY1+vkwmdOpc66OpWtcvSF1e0qiRKsU=
github.com/thanos-io/thanos v0.19.1-0.20210803192524-baea4ce9ef52/go.mod h1:Xskx78e0CYL6w0yDNOZHGdvwQMlsuzPsePmPtbp9Xuk=
github.com/themihai/gomemcache v0.0.0-20180902122335-24332e2d58ab h1:7ZR3hmisBWw77ZpO1/o86g+JV3VKlk3d48jopJxzTjU=
github.com/themihai/gomemcache v0.0.0-20180902122335-24332e2d58ab/go.mod h1:eheTFp954zcWZXCU8d0AT76ftsQOTo4DTqkN/h3k1MY=
github.com/tidwall/pretty v0.0.0-20180105212114-65a9db5fad51/go.mod h1:XNkn88O1ChpSDQmQeStsy+sBenx6DDtFZJxhVysOjyk=
Expand Down Expand Up @@ -1900,6 +1900,7 @@ go.uber.org/atomic v1.8.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
go.uber.org/atomic v1.9.0 h1:ECmE8Bn/WFTYwEW/bpKD3M8VtR/zQVbavAoalC1PYyE=
go.uber.org/atomic v1.9.0/go.mod h1:fEN4uk6kAWBTFdckzkM89CLk9XfWZrxpCo0nPH17wJc=
go.uber.org/automaxprocs v1.2.0/go.mod h1:YfO3fm683kQpzETxlTGZhGIVmXAhaw3gxeBADbpZtnU=
go.uber.org/automaxprocs v1.4.0/go.mod h1:/mTEdr7LvHhs0v7mjdxDreTz1OG5zdZGqgOnhWiR/+Q=
go.uber.org/goleak v1.0.0/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A=
go.uber.org/goleak v1.1.10 h1:z+mqJhf6ss6BSfSM671tgKyZBFPTTJM+HLxnhPC3wu0=
go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A=
Expand Down
3 changes: 3 additions & 0 deletions pkg/storage/tsdb/memcache_client_config.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ type MemcachedClientConfig struct {
MaxGetMultiConcurrency int `yaml:"max_get_multi_concurrency"`
MaxGetMultiBatchSize int `yaml:"max_get_multi_batch_size"`
MaxItemSize int `yaml:"max_item_size"`
AutoDiscovery bool `yaml:"auto_discovery"`
}

func (cfg *MemcachedClientConfig) RegisterFlagsWithPrefix(f *flag.FlagSet, prefix string) {
Expand All @@ -29,6 +30,7 @@ func (cfg *MemcachedClientConfig) RegisterFlagsWithPrefix(f *flag.FlagSet, prefi
f.IntVar(&cfg.MaxGetMultiConcurrency, prefix+"max-get-multi-concurrency", 100, "The maximum number of concurrent connections running get operations. If set to 0, concurrency is unlimited.")
f.IntVar(&cfg.MaxGetMultiBatchSize, prefix+"max-get-multi-batch-size", 0, "The maximum number of keys a single underlying get operation should run. If more keys are specified, internally keys are split into multiple batches and fetched concurrently, honoring the max concurrency. If set to 0, the max batch size is unlimited.")
f.IntVar(&cfg.MaxItemSize, prefix+"max-item-size", 1024*1024, "The maximum size of an item stored in memcached. Bigger items are not stored. If set to 0, no maximum size is enforced.")
f.BoolVar(&cfg.AutoDiscovery, prefix+"auto-discovery", false, "Use memcached auto-discovery instead of DNS resolution")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get a bit more descriptive than "memcached auto-discovery"?
I Googled that phrase and am not sure I found what you meant.

Copy link
Contributor

@jeromeinsf jeromeinsf Aug 13, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have updated the PR description to include the thanos PR link. Also added 2 links for explaining what auto-discovery means for memcached.

There are no other changes brought by upgrading Thanos in this PR.

Please let me know if you have other concerns.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@bboreham do you have anymore concern on this PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Changelog entry is circular and the Thanos link doesn't explain auto-discovery.
I think you're saying it is an ElasticCache feature that is also supported by Google Cloud; the Cortex docs should link to one or both of those if there is nothing more neutral.

}

func (cfg *MemcachedClientConfig) GetAddresses() []string {
Expand Down Expand Up @@ -59,5 +61,6 @@ func (cfg MemcachedClientConfig) ToMemcachedClientConfig() cacheutil.MemcachedCl
MaxGetMultiBatchSize: cfg.MaxGetMultiBatchSize,
MaxItemSize: model.Bytes(cfg.MaxItemSize),
DNSProviderUpdateInterval: 30 * time.Second,
AutoDiscovery: cfg.AutoDiscovery,
}
}

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading