Allow to configure min/max bucket size for the store-gateway chunks pool (#4119)

* Allow to configure min/max bucket size for the store-gateway chunks pool

Signed-off-by: Marco Pracucci <marco@pracucci.com>

* Fixed unit test

Signed-off-by: Marco Pracucci <marco@pracucci.com>

Author: pracucci

docs/blocks-storage/querier.md

@@ -435,11 +435,6 @@ blocks_storage:
     # CLI flag: -blocks-storage.bucket-store.sync-interval
     [sync_interval: <duration> | default = 15m]
 
-    # Max size - in bytes - of a chunks pool, used to reduce memory allocations.
-    # The pool is shared across all tenants. 0 to disable the limit.
-    # CLI flag: -blocks-storage.bucket-store.max-chunk-pool-bytes
-    [max_chunk_pool_bytes: <int> | default = 2147483648]
-
     # Max number of concurrent queries to execute against the long-term storage.
     # The limit is shared across all tenants.
     # CLI flag: -blocks-storage.bucket-store.max-concurrent
@@ -717,6 +712,11 @@ blocks_storage:
       # CLI flag: -blocks-storage.bucket-store.bucket-index.max-stale-period
       [max_stale_period: <duration> | default = 1h]
 
+    # Max size - in bytes - of a chunks pool, used to reduce memory allocations.
+    # The pool is shared across all tenants. 0 to disable the limit.
+    # CLI flag: -blocks-storage.bucket-store.max-chunk-pool-bytes
+    [max_chunk_pool_bytes: <int> | default = 2147483648]
+
     # If enabled, store-gateway will lazy load an index-header only once
     # required by a query.
     # CLI flag: -blocks-storage.bucket-store.index-header-lazy-loading-enabled

docs/blocks-storage/store-gateway.md

@@ -481,11 +481,6 @@ blocks_storage:
     # CLI flag: -blocks-storage.bucket-store.sync-interval
     [sync_interval: <duration> | default = 15m]
 
-    # Max size - in bytes - of a chunks pool, used to reduce memory allocations.
-    # The pool is shared across all tenants. 0 to disable the limit.
-    # CLI flag: -blocks-storage.bucket-store.max-chunk-pool-bytes
-    [max_chunk_pool_bytes: <int> | default = 2147483648]
-
     # Max number of concurrent queries to execute against the long-term storage.
     # The limit is shared across all tenants.
     # CLI flag: -blocks-storage.bucket-store.max-concurrent
@@ -763,6 +758,11 @@ blocks_storage:
       # CLI flag: -blocks-storage.bucket-store.bucket-index.max-stale-period
       [max_stale_period: <duration> | default = 1h]
 
+    # Max size - in bytes - of a chunks pool, used to reduce memory allocations.
+    # The pool is shared across all tenants. 0 to disable the limit.
+    # CLI flag: -blocks-storage.bucket-store.max-chunk-pool-bytes
+    [max_chunk_pool_bytes: <int> | default = 2147483648]
+
     # If enabled, store-gateway will lazy load an index-header only once
     # required by a query.
     # CLI flag: -blocks-storage.bucket-store.index-header-lazy-loading-enabled

docs/configuration/config-file-reference.md

@@ -4565,11 +4565,6 @@ bucket_store:
   # CLI flag: -blocks-storage.bucket-store.sync-interval
   [sync_interval: <duration> | default = 15m]
 
-  # Max size - in bytes - of a chunks pool, used to reduce memory allocations.
-  # The pool is shared across all tenants. 0 to disable the limit.
-  # CLI flag: -blocks-storage.bucket-store.max-chunk-pool-bytes
-  [max_chunk_pool_bytes: <int> | default = 2147483648]
-
   # Max number of concurrent queries to execute against the long-term storage.
   # The limit is shared across all tenants.
   # CLI flag: -blocks-storage.bucket-store.max-concurrent
@@ -4846,6 +4841,11 @@ bucket_store:
     # CLI flag: -blocks-storage.bucket-store.bucket-index.max-stale-period
     [max_stale_period: <duration> | default = 1h]
 
+  # Max size - in bytes - of a chunks pool, used to reduce memory allocations.
+  # The pool is shared across all tenants. 0 to disable the limit.
+  # CLI flag: -blocks-storage.bucket-store.max-chunk-pool-bytes
+  [max_chunk_pool_bytes: <int> | default = 2147483648]
+
   # If enabled, store-gateway will lazy load an index-header only once required
   # by a query.
   # CLI flag: -blocks-storage.bucket-store.index-header-lazy-loading-enabled

pkg/cortex/cortex_test.go

@@ -63,6 +63,8 @@ func TestCortex(t *testing.T) {
 				},
 			},
 			BucketStore: tsdb.BucketStoreConfig{
+				ChunkPoolMinBucketSizeBytes: tsdb.ChunkPoolDefaultMinBucketSize,
+				ChunkPoolMaxBucketSizeBytes: tsdb.ChunkPoolDefaultMaxBucketSize,
 				IndexCache: tsdb.IndexCacheConfig{
 					Backend: tsdb.IndexCacheBackendInMemory,
 				},

pkg/storage/tsdb/config.go

@@ -33,6 +33,12 @@ const (
 
 	// How often to check for tenant deletion mark.
 	DeletionMarkCheckInterval = 1 * time.Hour
+
+	// Default minimum bucket size (bytes) of the chunk pool.
+	ChunkPoolDefaultMinBucketSize = store.EstimatedMaxChunkSize
+
+	// Default maximum bucket size (bytes) of the chunk pool.
+	ChunkPoolDefaultMaxBucketSize = 50e6
 )
 
 // Validation errors
@@ -215,7 +221,6 @@ func (cfg *TSDBConfig) IsBlocksShippingEnabled() bool {
 type BucketStoreConfig struct {
 	SyncDir                  string              `yaml:"sync_dir"`
 	SyncInterval             time.Duration       `yaml:"sync_interval"`
-	MaxChunkPoolBytes        uint64              `yaml:"max_chunk_pool_bytes"`
 	MaxConcurrent            int                 `yaml:"max_concurrent"`
 	TenantSyncConcurrency    int                 `yaml:"tenant_sync_concurrency"`
 	BlockSyncConcurrency     int                 `yaml:"block_sync_concurrency"`
@@ -227,6 +232,11 @@ type BucketStoreConfig struct {
 	IgnoreDeletionMarksDelay time.Duration       `yaml:"ignore_deletion_mark_delay"`
 	BucketIndex              BucketIndexConfig   `yaml:"bucket_index"`
 
+	// Chunk pool.
+	MaxChunkPoolBytes           uint64 `yaml:"max_chunk_pool_bytes"`
+	ChunkPoolMinBucketSizeBytes int    `yaml:"chunk_pool_min_bucket_size_bytes" doc:"hidden"`
+	ChunkPoolMaxBucketSizeBytes int    `yaml:"chunk_pool_max_bucket_size_bytes" doc:"hidden"`
+
 	// Controls whether index-header lazy loading is enabled.
 	IndexHeaderLazyLoadingEnabled     bool          `yaml:"index_header_lazy_loading_enabled"`
 	IndexHeaderLazyLoadingIdleTimeout time.Duration `yaml:"index_header_lazy_loading_idle_timeout"`
@@ -253,6 +263,8 @@ func (cfg *BucketStoreConfig) RegisterFlags(f *flag.FlagSet) {
 	f.StringVar(&cfg.SyncDir, "blocks-storage.bucket-store.sync-dir", "tsdb-sync", "Directory to store synchronized TSDB index headers.")
 	f.DurationVar(&cfg.SyncInterval, "blocks-storage.bucket-store.sync-interval", 15*time.Minute, "How frequently to scan the bucket, or to refresh the bucket index (if enabled), in order to look for changes (new blocks shipped by ingesters and blocks deleted by retention or compaction).")
 	f.Uint64Var(&cfg.MaxChunkPoolBytes, "blocks-storage.bucket-store.max-chunk-pool-bytes", uint64(2*units.Gibibyte), "Max size - in bytes - of a chunks pool, used to reduce memory allocations. The pool is shared across all tenants. 0 to disable the limit.")
+	f.IntVar(&cfg.ChunkPoolMinBucketSizeBytes, "blocks-storage.bucket-store.chunk-pool-min-bucket-size-bytes", ChunkPoolDefaultMinBucketSize, "Size - in bytes - of the smallest chunks pool bucket.")
+	f.IntVar(&cfg.ChunkPoolMaxBucketSizeBytes, "blocks-storage.bucket-store.chunk-pool-max-bucket-size-bytes", ChunkPoolDefaultMaxBucketSize, "Size - in bytes - of the largest chunks pool bucket.")
 	f.IntVar(&cfg.MaxConcurrent, "blocks-storage.bucket-store.max-concurrent", 100, "Max number of concurrent queries to execute against the long-term storage. The limit is shared across all tenants.")
 	f.IntVar(&cfg.TenantSyncConcurrency, "blocks-storage.bucket-store.tenant-sync-concurrency", 10, "Maximum number of concurrent tenants synching blocks.")
 	f.IntVar(&cfg.BlockSyncConcurrency, "blocks-storage.bucket-store.block-sync-concurrency", 20, "Maximum number of concurrent blocks synching per tenant.")

pkg/storegateway/bucket_stores.go

@@ -126,7 +126,7 @@ func NewBucketStores(cfg tsdb.BlocksStorageConfig, shardingStrategy ShardingStra
 	}
 
 	// Init the chunks bytes pool.
-	if u.chunksPool, err = newChunkBytesPool(cfg.BucketStore.MaxChunkPoolBytes, reg); err != nil {
+	if u.chunksPool, err = newChunkBytesPool(cfg.BucketStore.ChunkPoolMinBucketSizeBytes, cfg.BucketStore.ChunkPoolMaxBucketSizeBytes, cfg.BucketStore.MaxChunkPoolBytes, reg); err != nil {
 		return nil, errors.Wrap(err, "create chunks bytes pool")
 	}
 

pkg/storegateway/chunk_bytes_pool.go

@@ -4,7 +4,6 @@ import (
 	"github.com/prometheus/client_golang/prometheus"
 	"github.com/prometheus/client_golang/prometheus/promauto"
 	"github.com/thanos-io/thanos/pkg/pool"
-	"github.com/thanos-io/thanos/pkg/store"
 )
 
 type chunkBytesPool struct {
@@ -15,8 +14,8 @@ type chunkBytesPool struct {
 	returnedBytes  prometheus.Counter
 }
 
-func newChunkBytesPool(maxChunkPoolBytes uint64, reg prometheus.Registerer) (*chunkBytesPool, error) {
-	upstream, err := pool.NewBucketedBytes(store.EstimatedMaxChunkSize, 50e6, 2, maxChunkPoolBytes)
+func newChunkBytesPool(minBucketSize, maxBucketSize int, maxChunkPoolBytes uint64, reg prometheus.Registerer) (*chunkBytesPool, error) {
+	upstream, err := pool.NewBucketedBytes(minBucketSize, maxBucketSize, 2, maxChunkPoolBytes)
 	if err != nil {
 		return nil, err
 	}

pkg/storegateway/chunk_bytes_pool_test.go

@@ -10,11 +10,13 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"github.com/thanos-io/thanos/pkg/store"
+
+	cortex_tsdb "github.com/cortexproject/cortex/pkg/storage/tsdb"
 )
 
 func TestChunkBytesPool_Get(t *testing.T) {
 	reg := prometheus.NewPedanticRegistry()
-	p, err := newChunkBytesPool(0, reg)
+	p, err := newChunkBytesPool(cortex_tsdb.ChunkPoolDefaultMinBucketSize, cortex_tsdb.ChunkPoolDefaultMaxBucketSize, 0, reg)
 	require.NoError(t, err)
 
 	_, err = p.Get(store.EstimatedMaxChunkSize - 1)