From c5e8fe6b71f5dcfee8c20f0897561b06040ba4d3 Mon Sep 17 00:00:00 2001
From: Ganesh Vernekar <ganeshvern@gmail.com>
Date: Mon, 26 Aug 2024 12:57:26 -0400
Subject: [PATCH] TSDB: Demonstrating Chunk pool is unnecessary

We only Put chunks in one place, that isn't called very often, so almost
every Get results in allocating fresh memory.

Signed-off-by: Bryan Boreham <bjboreham@gmail.com>
---
 tsdb/chunkenc/chunk.go      | 29 ----------------------------
 tsdb/chunkenc/chunk_test.go | 38 -------------------------------------
 tsdb/compact.go             |  6 ------
 3 files changed, 73 deletions(-)

diff --git a/tsdb/chunkenc/chunk.go b/tsdb/chunkenc/chunk.go
index 7082f34c3f..bef7dfe6e9 100644
--- a/tsdb/chunkenc/chunk.go
+++ b/tsdb/chunkenc/chunk.go
@@ -273,7 +273,6 @@ func (nopIterator) Err() error { return nil }
 
 // Pool is used to create and reuse chunk references to avoid allocations.
 type Pool interface {
-	Put(Chunk) error
 	Get(e Encoding, b []byte) (Chunk, error)
 }
 
@@ -322,34 +321,6 @@ func (p *pool) Get(e Encoding, b []byte) (Chunk, error) {
 	return c, nil
 }
 
-func (p *pool) Put(c Chunk) error {
-	var sp *sync.Pool
-	var ok bool
-	switch c.Encoding() {
-	case EncXOR:
-		_, ok = c.(*XORChunk)
-		sp = &p.xor
-	case EncHistogram:
-		_, ok = c.(*HistogramChunk)
-		sp = &p.histogram
-	case EncFloatHistogram:
-		_, ok = c.(*FloatHistogramChunk)
-		sp = &p.floatHistogram
-	default:
-		return fmt.Errorf("invalid chunk encoding %q", c.Encoding())
-	}
-	if !ok {
-		// This may happen often with wrapped chunks. Nothing we can really do about
-		// it but returning an error would cause a lot of allocations again. Thus,
-		// we just skip it.
-		return nil
-	}
-
-	c.Reset(nil)
-	sp.Put(c)
-	return nil
-}
-
 // FromData returns a chunk from a byte slice of chunk data.
 // This is there so that users of the library can easily create chunks from
 // bytes.
diff --git a/tsdb/chunkenc/chunk_test.go b/tsdb/chunkenc/chunk_test.go
index a5e75ca32b..9c11ca5c13 100644
--- a/tsdb/chunkenc/chunk_test.go
+++ b/tsdb/chunkenc/chunk_test.go
@@ -158,46 +158,8 @@ func TestPool(t *testing.T) {
 				stream: []byte("test"),
 				count:  0,
 			}, b)
-
-			b.count = 1
-			require.NoError(t, p.Put(c))
-			require.Equal(t, &bstream{
-				stream: nil,
-				count:  0,
-			}, b)
 		})
 	}
-
-	t.Run("put bad chunk wrapper", func(t *testing.T) {
-		// When a wrapping chunk poses as an encoding it can't be converted to, Put should skip it.
-		c := fakeChunk{
-			encoding: EncXOR,
-			t:        t,
-		}
-		require.NoError(t, p.Put(c))
-	})
-	t.Run("put invalid encoding", func(t *testing.T) {
-		c := fakeChunk{
-			encoding: EncNone,
-			t:        t,
-		}
-		require.EqualError(t, p.Put(c), `invalid chunk encoding "none"`)
-	})
-}
-
-type fakeChunk struct {
-	Chunk
-
-	encoding Encoding
-	t        *testing.T
-}
-
-func (c fakeChunk) Encoding() Encoding {
-	return c.encoding
-}
-
-func (c fakeChunk) Reset([]byte) {
-	c.t.Fatal("Reset should not be called")
 }
 
 func benchmarkIterator(b *testing.B, newChunk func() Chunk) {
diff --git a/tsdb/compact.go b/tsdb/compact.go
index 31b445f227..83143e6ef2 100644
--- a/tsdb/compact.go
+++ b/tsdb/compact.go
@@ -882,12 +882,6 @@ func (c DefaultBlockPopulator) PopulateBlock(ctx context.Context, metrics *Compa
 		for _, chk := range chks {
 			meta.Stats.NumSamples += uint64(chk.Chunk.NumSamples())
 		}
-
-		for _, chk := range chks {
-			if err := chunkPool.Put(chk.Chunk); err != nil {
-				return fmt.Errorf("put chunk: %w", err)
-			}
-		}
 		ref++
 	}
 	if err := set.Err(); err != nil {