Update Thanos to latest main

[siggy]

Update Thanos dependency to include thanos-io/thanos#4928, to conserve memory.

Signed-off-by: Andrew Seigner [email protected]

What this PR does:

Bumps Thanos dependency to the latest main: thanos-io/thanos@d1acaea

Most notably this pulls in thanos-io/thanos#4928, which gives a 2x~4x memory reduction when using Azure blocks storage:

This PR, combined with #4581, brings Cortex's memory usage back inline with v1.10.0.

Relates to:

• Introduce http config settings in Azure storage #4581
• objstore/azure: Only create an http client once thanos-io/thanos#4928

Checklist

• Tests updated
• Documentation added
• CHANGELOG.md updated - the order of entries should be [CHANGE], [FEATURE], [ENHANCEMENT], [BUGFIX]

[bboreham]

Thanks for doing this. It's best to cover all important changes when updating a dependency, so I checked
thanos-io/thanos@de0e384...d1acaea
and I see that Redis is now a cache option. This sounds quite interesting; is it usable from Cortex now or does it need plumbing?

Please can you check the list and summarise all user-visible changes in the CHANGELOG. Just in a few words - it's not necessary to link to individual PRs besides this one where a keen reader can find all the details. Readers of CHANGELOG are assumed to be admins who don't want code-level details.

[bboreham]

Question: is the pin still necessary?

[siggy]

Thanos release tags are still not on the main branch. This replace pattern allowed me to simply put main on this line, and then go mod vendor correctly resolved it to v0.19.1-0.20211122085937-de0e3848ff60.

[bboreham]

Thanks.
The comment above says it's about merging release branches, but this kinda looks like a merge of the release branch: thanos-io/thanos#4738
Did it not work? Not what Go is expecting?

[pstibrany]

That was a squash merge, not merge commit. It didn't make tag v0.23.1 reachable from main branch.

[pstibrany]

I tried to update release process instructions in Thanos at thanos-io/thanos#4770 to hopefully fix this situation in the future.

[siggy]

Thanks for filing thanos-io/thanos#4770 @pstibrany!

Did it not work? Not what Go is expecting?

So we can do a go get on a release tag, and it works:

$ go get github.com/thanos-io/[email protected]
$ go mod vendor
$ git diff
...
-       github.com/thanos-io/thanos v0.22.0
+       github.com/thanos-io/thanos v0.24.0-rc.1
-replace github.com/thanos-io/thanos v0.22.0 => github.com/thanos-io/thanos v0.19.1-0.20211208205607-d1acaea2a11a
+// replace github.com/thanos-io/thanos v0.22.0 => github.com/thanos-io/thanos v0.19.1-0.20211208205607-d1acaea2a11a

... I don't know how important it is that we be able to do go get github.com/thanos-io/thanos@main, but that still doesn't work:

$ go get github.com/thanos-io/thanos@main
go: downloading github.com/thanos-io/thanos v0.19.1-0.20211213141611-c7a44e23febe
go get: github.com/thanos-io/thanos@main (v0.19.1-0.20211213141611-c7a44e23febe) requires github.com/thanos-io/[email protected], not github.com/thanos-io/thanos@main (v0.19.1-0.20211213141611-c7a44e23febe)

So I guess we have a few options:

1. Leave as-is, wait for Add step for merging release branch back to main. thanos-io/thanos#4770.
2. Remove the replace and the comment, require v0.24.0-rc.1 directly.
3. Remove the replace and the comment, wait for v0.24.0 and require it directly.

[siggy]

I see that Redis is now a cache option. This sounds quite interesting; is it usable from Cortex now or does it need plumbing?

I think this would need plumbing. Cache instantation in Cortex:

cortex/pkg/storage/tsdb/caching_bucket.go

Lines 152 to 169 in 4474da8

	func createCache(cacheName string, backend string, memcached MemcachedClientConfig, logger log.Logger, reg prometheus.Registerer) (cache.Cache, error) {
	switch backend {
	case "":
	// No caching.
	return nil, nil
	case CacheBackendMemcached:
	var client cacheutil.MemcachedClient
	client, err := cacheutil.NewMemcachedClientWithConfig(logger, cacheName, memcached.ToMemcachedClientConfig(), reg)
	if err != nil {
	return nil, errors.Wrapf(err, "failed to create memcached client")
	}
	return cache.NewMemcachedCache(cacheName, logger, client, reg), nil
	default:
	return nil, errors.Errorf("unsupported cache type for cache %s: %s", cacheName, backend)
	}
	}

cortex/pkg/storage/tsdb/index_cache.go

Lines 79 to 89 in 4474da8

	// NewIndexCache creates a new index cache based on the input configuration.
	func NewIndexCache(cfg IndexCacheConfig, logger log.Logger, registerer prometheus.Registerer) (storecache.IndexCache, error) {
	switch cfg.Backend {
	case IndexCacheBackendInMemory:
	return newInMemoryIndexCache(cfg.InMemory, logger, registerer)
	case IndexCacheBackendMemcached:
	return newMemcachedIndexCache(cfg.Memcached, logger, registerer)
	default:
	return nil, errUnsupportedIndexCacheBackend
	}
	}

... would need to call cacheutil.NewRedisClient in Thanos:
https://github.com/thanos-io/thanos/blob/9a26b0edee19a06c6e99a09e33ebceca734c91f9/pkg/cacheutil/redis_client.go#L122-L133

Interestingly the Thanos PR mentions Cortex's redis support as a reference implementation: thanos-io/thanos#4888

[bboreham]

Thanks; LGTM

[bboreham]

Thanks.
The comment above says it's about merging release branches, but this kinda looks like a merge of the release branch: thanos-io/thanos#4738
Did it not work? Not what Go is expecting?

[pracucci]

LGTM, thanks!