From d95d15402fe8143169cff3a55b368ea45f70f963 Mon Sep 17 00:00:00 2001 From: Sudharshann D Date: Thu, 7 Jan 2021 13:09:41 -0600 Subject: [PATCH] addressing comments Signed-off-by: Sudharshann D --- CHANGELOG.md | 1 + go.mod | 2 -- go.sum | 7 ----- pkg/cache/inmemory.go | 52 ++++++++++++++++++------------------- pkg/cache/memcached_test.go | 2 +- 5 files changed, 28 insertions(+), 36 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d560b512187..7c69e68eadb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,7 @@ We use _breaking :warning:_ to mark changes that are not backward compatible (re ### Added +- [#3579](https://github.com/thanos-io/thanos/pull/3579) Cache: Added inmemory cache for caching bucket. - [#3469](https://github.com/thanos-io/thanos/pull/3469) StoreAPI: Added `hints` field to `LabelNamesRequest` and `LabelValuesRequest`. Hints in an opaque data structure that can be used to carry additional information from the store and its content is implementation specific. - [#3421](https://github.com/thanos-io/thanos/pull/3421) Tools: Added `thanos tools bucket rewrite` command allowing to delete series from given block. - [#3509](https://github.com/thanos-io/thanos/pull/3509) Store: Added touch series limit diff --git a/go.mod b/go.mod index 3ad7fdde331..8d1e9027d78 100644 --- a/go.mod +++ b/go.mod @@ -15,7 +15,6 @@ require ( github.com/cortexproject/cortex v1.5.1-0.20201111110551-ba512881b076 github.com/davecgh/go-spew v1.1.1 github.com/facette/natsort v0.0.0-20181210072756-2cd4dd1e2dcb - github.com/fatih/color v1.10.0 // indirect github.com/fatih/structtag v1.1.0 github.com/felixge/fgprof v0.9.1 github.com/fsnotify/fsnotify v1.4.9 @@ -69,7 +68,6 @@ require ( gopkg.in/fsnotify.v1 v1.4.7 gopkg.in/yaml.v2 v2.3.0 gopkg.in/yaml.v3 v3.0.0-20200615113413-eeeca48fe776 - honnef.co/go/tools v0.0.1-2020.1.6 // indirect ) replace ( diff --git a/go.sum b/go.sum index 6f8b291b468..622ef227b99 100644 --- a/go.sum +++ b/go.sum @@ -307,8 +307,6 @@ github.com/facette/natsort v0.0.0-20181210072756-2cd4dd1e2dcb/go.mod h1:bH6Xx7IW github.com/fatih/color v1.7.0/go.mod h1:Zm6kSWBoL9eyXnKyktHP6abPY2pDugNf5KwzbycvMj4= github.com/fatih/color v1.9.0 h1:8xPHl4/q1VyqGIPif1F+1V3Y3lSmrq01EabUW3CoW5s= github.com/fatih/color v1.9.0/go.mod h1:eQcE1qtQxscV5RaZvpXrrb8Drkc3/DdQ+uUYCNjL+zU= -github.com/fatih/color v1.10.0 h1:s36xzo75JdqLaaWoiEHk767eHiwo0598uUxyfiPkDsg= -github.com/fatih/color v1.10.0/go.mod h1:ELkj/draVOlAH/xkhN6mQ50Qd0MPOk5AAr3maGEBuJM= github.com/fatih/structtag v1.1.0 h1:6j4mUV/ES2duvnAzKMFkN6/A5mCaNYPD3xfbAkLLOF8= github.com/fatih/structtag v1.1.0/go.mod h1:mBJUNpUnHmRKrKlQQlmCrh5PuhftFbNv8Ys4/aAZl94= github.com/felixge/fgprof v0.9.1 h1:E6FUJ2Mlv043ipLOCFqo8+cHo9MhQ203E2cdEK/isEs= @@ -806,8 +804,6 @@ github.com/mattn/go-colorable v0.0.9/go.mod h1:9vuHe8Xs5qXnSaW/c/ABM9alt+Vo+STaO github.com/mattn/go-colorable v0.1.4/go.mod h1:U0ppj6V5qS13XJ6of8GYAs25YV2eR4EVcfRqFIhoBtE= github.com/mattn/go-colorable v0.1.6 h1:6Su7aK7lXmJ/U79bYtBjLNaha4Fs1Rg9plHpcH+vvnE= github.com/mattn/go-colorable v0.1.6/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= -github.com/mattn/go-colorable v0.1.8 h1:c1ghPdyEDarC70ftn0y+A/Ee++9zz8ljHG1b13eJ0s8= -github.com/mattn/go-colorable v0.1.8/go.mod h1:u6P/XSegPjTcexA+o6vUJrdnUu04hMope9wVRipJSqc= github.com/mattn/go-ieproxy v0.0.0-20190610004146-91bb50d98149/go.mod h1:31jz6HNzdxOmlERGGEc4v/dMssOfmp2p5bT/okiKFFc= github.com/mattn/go-ieproxy v0.0.0-20190702010315-6dee0af9227d/go.mod h1:31jz6HNzdxOmlERGGEc4v/dMssOfmp2p5bT/okiKFFc= github.com/mattn/go-ieproxy v0.0.0-20191113090002-7c0f6868bffe h1:YioO2TiJyAHWHyCRQCP8jk5IzTqmsbGc5qQPIhHo6xs= @@ -1529,7 +1525,6 @@ golang.org/x/tools v0.0.0-20200227222343-706bc42d1f0d/go.mod h1:TB2adYChydJhpapK golang.org/x/tools v0.0.0-20200304193943-95d2e580d8eb/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw= golang.org/x/tools v0.0.0-20200312045724-11d5b4c81c7d/go.mod h1:o4KQGtdN14AW+yjsvvwRTJJuXz8XRtIHtEnmAXLyFUw= golang.org/x/tools v0.0.0-20200331025713-a30bf2db82d4/go.mod h1:Sl4aGygMT6LrqrWclx+PTx3U+LnKx/seiNR+3G19Ar8= -golang.org/x/tools v0.0.0-20200410194907-79a7a3126eef/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200501065659-ab2804fb9c9d/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200512131952-2bc93b1c0c88/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= golang.org/x/tools v0.0.0-20200513201620-d5fe73897c97/go.mod h1:EkVYQZoAsY45+roYkvgYkIh4xh/qjgUK9TdY2XT94GE= @@ -1694,8 +1689,6 @@ honnef.co/go/tools v0.0.1-2020.1.3 h1:sXmLre5bzIR6ypkjXCDI3jHPssRhc8KD/Ome589sc3 honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= honnef.co/go/tools v0.0.1-2020.1.4 h1:UoveltGrhghAA7ePc+e+QYDHXrBps2PqFZiHkGR/xK8= honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k= -honnef.co/go/tools v0.0.1-2020.1.6 h1:W18jzjh8mfPez+AwGLxmOImucz/IFjpNlrKVnaj2YVc= -honnef.co/go/tools v0.0.1-2020.1.6/go.mod h1:pyyisuGw24ruLjrr1ddx39WE0y9OooInRzEYLhQB2YY= howett.net/plist v0.0.0-20181124034731-591f970eefbb h1:jhnBjNi9UFpfpl8YZhA9CrOqpnJdvzuiHsl/dnxl11M= howett.net/plist v0.0.0-20181124034731-591f970eefbb/go.mod h1:vMygbs4qMhSZSc4lCUl2OEE+rDiIIJAIdR4m7MiMcm0= k8s.io/api v0.0.0-20191115095533-47f6de673b26/go.mod h1:iA/8arsvelvo4IDqIhX4IbjTEKBGgvsf2OraTuRtLFU= diff --git a/pkg/cache/inmemory.go b/pkg/cache/inmemory.go index 0e8a5da9f6f..39c6e032438 100644 --- a/pkg/cache/inmemory.go +++ b/pkg/cache/inmemory.go @@ -7,7 +7,6 @@ import ( "context" "sync" "time" - "unsafe" "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" @@ -28,10 +27,6 @@ var ( const ( maxInt = int(^uint(0) >> 1) - // Size of a golang slice i.e unsafe.Sizeof([]byte{}) on amd64. - sliceHeaderSize = 16 - // As the cache key is of type String its size is 8 bytes. - keyHeaderSize = 8 ) // InMemoryCacheConfig holds the in-memory cache config. @@ -64,7 +59,12 @@ type InMemoryCache struct { } type cacheDataWithTTLWrapper struct { - data []byte + data []byte + // The objects that are over the TTL are not destroyed eagerly. + // When there is a hit for an item that is over the TTL, the object is removed from the cache + // and null is returned. + // There is ongoing effort to integrate TTL within the Hashicorp golang cache itself. + // This https://github.com/hashicorp/golang-lru/pull/41 can be integrated here once complete. expiryTime time.Time } @@ -78,7 +78,7 @@ func parseInMemoryCacheConfig(conf []byte) (InMemoryCacheConfig, error) { return config, nil } -// NewInMemoryCache creates a new thread-safe LRU cache for chunks and metadata and ensures the total cache +// NewInMemoryCache creates a new thread-safe LRU cache and ensures the total cache // size approximately does not exceed maxBytes. func NewInMemoryCache(name string, logger log.Logger, reg prometheus.Registerer, conf []byte) (*InMemoryCache, error) { config, err := parseInMemoryCacheConfig(conf) @@ -89,7 +89,7 @@ func NewInMemoryCache(name string, logger log.Logger, reg prometheus.Registerer, return NewInMemoryCacheWithConfig(name, logger, reg, config) } -// NewInMemoryCacheWithConfig creates a new thread-safe LRU cache chunks and metadata ensures the total cache +// NewInMemoryCacheWithConfig creates a new thread-safe LRU cache and ensures the total cache // size approximately does not exceed maxBytes. func NewInMemoryCacheWithConfig(name string, logger log.Logger, reg prometheus.Registerer, config InMemoryCacheConfig) (*InMemoryCache, error) { if config.MaxItemSize > config.MaxSize { @@ -103,65 +103,66 @@ func NewInMemoryCacheWithConfig(name string, logger log.Logger, reg prometheus.R } c.evicted = promauto.With(reg).NewCounter(prometheus.CounterOpts{ - Name: "thanos_store_cache_items_evicted_total", + Name: "thanos_cache_inmemory_items_evicted_total", Help: "Total number of items that were evicted from the inmemory cache.", ConstLabels: prometheus.Labels{"name": name}, }) c.added = promauto.With(reg).NewCounter(prometheus.CounterOpts{ - Name: "thanos_store_cache_items_added_total", + Name: "thanos_cache_inmemory_items_added_total", Help: "Total number of items that were added to the inmemory cache.", ConstLabels: prometheus.Labels{"name": name}, }) c.requests = promauto.With(reg).NewCounter(prometheus.CounterOpts{ - Name: "thanos_store_cache_requests_total", + Name: "thanos_cache_inmemory_requests_total", Help: "Total number of requests to the inmemory cache.", ConstLabels: prometheus.Labels{"name": name}, }) c.hitsExpired = promauto.With(reg).NewCounter(prometheus.CounterOpts{ - Name: "thanos_store_cache_hits_on_expired_data_total", + Name: "thanos_cache_inmemory_hits_on_expired_data_total", Help: "Total number of requests to the inmemory cache that were a hit but needed to be evicted due to TTL.", ConstLabels: prometheus.Labels{"name": name}, }) c.overflow = promauto.With(reg).NewCounter(prometheus.CounterOpts{ - Name: "thanos_store_cache_items_overflowed_total", + Name: "thanos_cache_inmemory_items_overflowed_total", Help: "Total number of items that could not be added to the inmemory cache due to being too big.", ConstLabels: prometheus.Labels{"name": name}, }) c.hits = promauto.With(reg).NewCounter(prometheus.CounterOpts{ - Name: "thanos_store_cache_hits_total", + Name: "thanos_cache_inmemory_hits_total", Help: "Total number of requests to the inmemory cache that were a hit.", ConstLabels: prometheus.Labels{"name": name}, }) c.current = promauto.With(reg).NewGauge(prometheus.GaugeOpts{ - Name: "thanos_store_cache_items", + Name: "thanos_cache_inmemory_items", Help: "Current number of items in the inmemory cache.", ConstLabels: prometheus.Labels{"name": name}, }) c.currentSize = promauto.With(reg).NewGauge(prometheus.GaugeOpts{ - Name: "thanos_store_cache_items_size_bytes", - Help: "Current byte size of items in the inmemory cache.", + Name: "thanos_cache_inmemory_items_size_bytes", + Help: "Current byte size of items in the inmemory cache.", + ConstLabels: prometheus.Labels{"name": name}, }) c.totalCurrentSize = promauto.With(reg).NewGauge(prometheus.GaugeOpts{ - Name: "thanos_store_cache_total_size_bytes", + Name: "thanos_cache_inmemory_total_size_bytes", Help: "Current byte size of items (both value and key) in the inmemory cache.", }) _ = promauto.With(reg).NewGaugeFunc(prometheus.GaugeOpts{ - Name: "thanos_store_cache_max_size_bytes", + Name: "thanos_cache_inmemory_max_size_bytes", Help: "Maximum number of bytes to be held in the inmemory cache.", }, func() float64 { return float64(c.maxSizeBytes) }) _ = promauto.With(reg).NewGaugeFunc(prometheus.GaugeOpts{ - Name: "thanos_store_cache_max_item_size_bytes", + Name: "thanos_cache_inmemory_max_item_size_bytes", Help: "Maximum number of bytes for single entry to be held in the inmemory cache.", }, func() float64 { return float64(c.maxItemSizeBytes) @@ -185,8 +186,8 @@ func NewInMemoryCacheWithConfig(name string, logger log.Logger, reg prometheus.R } func (c *InMemoryCache) onEvict(key, val interface{}) { - keySize := keyHeaderSize + uint64(len(key.(string))) - entrySize := sliceHeaderSize + uint64(len(val.(cacheDataWithTTLWrapper).data)) + uint64(unsafe.Sizeof(val.(cacheDataWithTTLWrapper).expiryTime)) + keySize := uint64(len(key.(string))) + entrySize := uint64(len(val.(cacheDataWithTTLWrapper).data)) c.evicted.Inc() c.current.Dec() @@ -217,9 +218,8 @@ func (c *InMemoryCache) get(key string) ([]byte, bool) { } func (c *InMemoryCache) set(key string, val []byte, ttl time.Duration) { - var size = sliceHeaderSize + uint64(len(val)) - keySize := keyHeaderSize + uint64(len(key)) - entrySize := sliceHeaderSize + uint64(len(val)) + uint64(unsafe.Sizeof(time.Now().Add(ttl))) + var size = uint64(len(val)) + keySize := uint64(len(key)) c.mtx.Lock() defer c.mtx.Unlock() @@ -241,7 +241,7 @@ func (c *InMemoryCache) set(key string, val []byte, ttl time.Duration) { c.added.Inc() c.currentSize.Add(float64(size)) - c.totalCurrentSize.Add(float64(keySize + entrySize)) + c.totalCurrentSize.Add(float64(keySize + size)) c.current.Inc() c.curSize += size } diff --git a/pkg/cache/memcached_test.go b/pkg/cache/memcached_test.go index 72ca7f91f62..5fcfbb936b0 100644 --- a/pkg/cache/memcached_test.go +++ b/pkg/cache/memcached_test.go @@ -15,7 +15,7 @@ import ( "github.com/thanos-io/thanos/pkg/testutil" ) -func TestMemcachedIndexCache(t *testing.T) { +func TestMemcachedCache(t *testing.T) { t.Parallel() // Init some data to conveniently define test cases later one.