Move buffer pool management into FileReader.

pkg/storegateway/indexheader/encoding/factory.go

@@ -3,11 +3,9 @@
 package encoding
 
 import (
-	"bufio"
 	"encoding/binary"
 	"hash/crc32"
 	"os"
-	"sync"
 
 	"github.com/grafana/dskit/multierror"
 	"github.com/pkg/errors"
@@ -22,12 +20,6 @@ type DecbufFactory struct {
 	path string
 }
 
-var bufferPool = sync.Pool{
-	New: func() any {
-		return bufio.NewReaderSize(nil, readerBufferSize)
-	},
-}
-
 func NewDecbufFactory(path string) *DecbufFactory {
 	return &DecbufFactory{
 		path: path,
@@ -65,10 +57,9 @@ func (df *DecbufFactory) NewDecbufAtChecked(offset int, table *crc32.Table) Decb
 		return Decbuf{E: errors.Wrapf(ErrInvalidSize, "insufficient bytes read for size (got %d, wanted %d)", n, 4)}
 	}
 
-	bufReader := bufferPool.Get().(*bufio.Reader)
 	contentLength := int(binary.BigEndian.Uint32(lengthBytes))
 	bufferLength := len(lengthBytes) + contentLength + crc32.Size
-	r, err := NewFileReader(f, offset, bufferLength, bufReader)
+	r, err := NewFileReader(f, offset, bufferLength)
 	if err != nil {
 		return Decbuf{E: errors.Wrap(err, "create file reader")}
 	}
@@ -124,9 +115,8 @@ func (df *DecbufFactory) NewRawDecbuf() Decbuf {
 		return Decbuf{E: errors.Wrap(err, "stat file for decbuf")}
 	}
 
-	bufReader := bufferPool.Get().(*bufio.Reader)
 	fileSize := stat.Size()
-	reader, err := NewFileReader(f, 0, int(fileSize), bufReader)
+	reader, err := NewFileReader(f, 0, int(fileSize))
 	if err != nil {
 		return Decbuf{E: errors.Wrap(err, "file reader for decbuf")}
 	}
@@ -137,10 +127,6 @@ func (df *DecbufFactory) NewRawDecbuf() Decbuf {
 
 // Close cleans up any resources associated with the Decbuf
 func (df *DecbufFactory) Close(d Decbuf) error {
-	if d.r != nil {
-		bufferPool.Put(d.r.buf)
-	}
-
 	return d.close()
 }
 

pkg/storegateway/indexheader/encoding/reader.go

@@ -8,6 +8,7 @@ import (
 	"fmt"
 	"io"
 	"os"
+	"sync"
 )
 
 type FileReader struct {
@@ -18,12 +19,18 @@ type FileReader struct {
 	pos    int
 }
 
+var bufferPool = sync.Pool{
+	New: func() any {
+		return bufio.NewReaderSize(nil, readerBufferSize)
+	},
+}
+
 // NewFileReader creates a new FileReader for the segment of file beginning at base bytes
 // extending length bytes using the supplied buffered reader.
-func NewFileReader(file *os.File, base, length int, buf *bufio.Reader) (*FileReader, error) {
+func NewFileReader(file *os.File, base, length int) (*FileReader, error) {
 	f := &FileReader{
 		file:   file,
-		buf:    buf,
+		buf:    bufferPool.Get().(*bufio.Reader),
 		base:   base,
 		length: length,
 	}
@@ -143,5 +150,7 @@ func (f *FileReader) Len() int {
 // is unexported to ensure that all resource management is handled by DecbufFactory
 // which pools resources.
 func (f *FileReader) close() error {
+	bufferPool.Put(f.buf)
+
 	return f.file.Close()
 }

pkg/storegateway/indexheader/encoding/reader_test.go

@@ -3,7 +3,6 @@
 package encoding
 
 import (
-	"bufio"
 	"os"
 	"path"
 	"testing"
@@ -185,7 +184,7 @@ func TestReaders_CreationWithEmptyContents(t *testing.T) {
 			require.NoError(t, f.Close())
 		})
 
-		r, err := NewFileReader(f, 0, 0, bufio.NewReader(f))
+		r, err := NewFileReader(f, 0, 0)
 		require.NoError(t, err)
 		require.ErrorIs(t, r.Skip(1), ErrInvalidSize)
 		require.ErrorIs(t, r.ResetAt(1), ErrInvalidSize)
@@ -206,7 +205,7 @@ func testReaders(t *testing.T, test func(t *testing.T, r *FileReader)) {
 			require.NoError(t, f.Close())
 		})
 
-		r, err := NewFileReader(f, 0, len(testReaderContents), bufio.NewReader(f))
+		r, err := NewFileReader(f, 0, len(testReaderContents))
 		require.NoError(t, err)
 
 		test(t, r)
@@ -226,7 +225,7 @@ func testReaders(t *testing.T, test func(t *testing.T, r *FileReader)) {
 			require.NoError(t, f.Close())
 		})
 
-		r, err := NewFileReader(f, len(offsetBytes), len(testReaderContents), bufio.NewReader(f))
+		r, err := NewFileReader(f, len(offsetBytes), len(testReaderContents))
 		require.NoError(t, err)
 
 		test(t, r)