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

Hash object names used as cache keys for CachingBucket #10024

Merged
merged 3 commits into from
Nov 26, 2024

Conversation

56quarters
Copy link
Contributor

What this PR does

When caching rule groups, the base64 encoded name of the rule group is used as part of the key for caching its contents. For long rule group names, this can exceed the max key length of Memcached. To solve this we use a cryptographic hash of the object name when the hashed version of the name is shorter than the full name. This is the same approach taken for postings in the store-gateway indexcache.

This has the added benefit of not invalidating most existing cache entries when rolling out this change. With the hash function picked, key generation is between 5 and 10 times slower than not hashing the key but still dramatically faster than a network operation.

Which issue(s) this PR fixes or relates to

Related #9386

Checklist

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

When caching rule groups, the base64 encoded name of the rule group is
used as part of the key for caching its contents. For long rule group
names, this can exceed the max key length of Memcached. To solve this
we use a cryptographic hash of the object name when the hashed version
of the name is shorter than the full name. This is the same approach
taken for postings in the store-gateway `indexcache`.

This has the added benefit of not invalidating most existing cache
entries when rolling out this change. With the hash function picked,
key generation is between 5 and 10 times slower than _not_ hashing
the key but still dramatically faster than a network operation.

Related #9386
@56quarters
Copy link
Contributor Author

Benchmarks:

$ go test -bench=BenchmarkCachingKey ./pkg/storage/tsdb/bucketcache/...
goos: linux
goarch: amd64
pkg: github.com/grafana/mimir/pkg/storage/tsdb/bucketcache
cpu: Intel(R) Core(TM) i9-10885H CPU @ 2.40GHz
BenchmarkCachingKey/cacheKeyBuilder.iter()_recursive_keyLength=7,_bucketID=false-16             15757104                83.31 ns/op
BenchmarkCachingKey/cacheKeyBuilder.iter()_recursive_keyLength=44,_bucketID=false-16             2516331               564.7 ns/op
BenchmarkCachingKey/cacheKeyBuilder.iter()_recursive_keyLength=7,_bucketID=true-16              13092854               108.2 ns/op
BenchmarkCachingKey/cacheKeyBuilder.iter()_recursive_keyLength=44,_bucketID=true-16              2075427               502.0 ns/op
BenchmarkCachingKey/cacheKeyBuilder.exists()_keyLength=7,_bucketID=false-16                     25929783                55.09 ns/op
BenchmarkCachingKey/cacheKeyBuilder.exists()_keyLength=44,_bucketID=false-16                     2331666               463.0 ns/op
BenchmarkCachingKey/cacheKeyBuilder.exists()_keyLength=7,_bucketID=true-16                      15586548                73.86 ns/op
BenchmarkCachingKey/cacheKeyBuilder.exists()_keyLength=44,_bucketID=true-16                      2609112               534.3 ns/op
BenchmarkCachingKey/cacheKeyBuilder.content()_keyLength=7,_bucketID=false-16                    20508844                56.16 ns/op
BenchmarkCachingKey/cacheKeyBuilder.content()_keyLength=44,_bucketID=false-16                    2680401               438.8 ns/op
BenchmarkCachingKey/cacheKeyBuilder.content()_keyLength=7,_bucketID=true-16                     21625657                62.53 ns/op
BenchmarkCachingKey/cacheKeyBuilder.content()_keyLength=44,_bucketID=true-16                     2454789               506.2 ns/op
BenchmarkCachingKey/cacheKeyBuilder.attributes()_keyLength=7,_bucketID=false-16                 25148311                52.79 ns/op
BenchmarkCachingKey/cacheKeyBuilder.attributes()_keyLength=44,_bucketID=false-16                 2213874               462.7 ns/op
BenchmarkCachingKey/cacheKeyBuilder.attributes()_keyLength=7,_bucketID=true-16                  16169638                77.97 ns/op
BenchmarkCachingKey/cacheKeyBuilder.attributes()_keyLength=44,_bucketID=true-16                  2882824               542.0 ns/op
BenchmarkCachingKey/cacheKeyBuilder.objectSubrange()_keyLength=7,_bucketID=false-16             16226624                80.00 ns/op
BenchmarkCachingKey/cacheKeyBuilder.objectSubrange()_keyLength=44,_bucketID=false-16             2056718               528.5 ns/op
BenchmarkCachingKey/cacheKeyBuilder.objectSubrange()_keyLength=7,_bucketID=true-16              14310009                98.74 ns/op
BenchmarkCachingKey/cacheKeyBuilder.objectSubrange()_keyLength=44,_bucketID=true-16              2499112               539.1 ns/op
BenchmarkCachingKey/cacheKeyBuilder.iter()_keyLength=7,_bucketID=false-16                       18043245                69.51 ns/op
BenchmarkCachingKey/cacheKeyBuilder.iter()_keyLength=44,_bucketID=false-16                       2124430               475.1 ns/op
BenchmarkCachingKey/cacheKeyBuilder.iter()_keyLength=7,_bucketID=true-16                        16253055                89.75 ns/op
BenchmarkCachingKey/cacheKeyBuilder.iter()_keyLength=44,_bucketID=true-16                        2312062               605.2 ns/op
PASS
ok      github.com/grafana/mimir/pkg/storage/tsdb/bucketcache   39.470s

@56quarters 56quarters marked this pull request as ready for review November 25, 2024 22:59
@56quarters 56quarters requested a review from a team as a code owner November 25, 2024 22:59
Copy link
Contributor

@dimitarvdimitrov dimitarvdimitrov left a comment

Choose a reason for hiding this comment

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

nice one

Copy link
Contributor

@narqo narqo left a comment

Choose a reason for hiding this comment

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

🔥

@56quarters 56quarters merged commit 73d2bdd into main Nov 26, 2024
29 checks passed
@56quarters 56quarters deleted the 56quarters/bucket-cache-key branch November 26, 2024 15:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants