Skip to content

Commit

Permalink
Revert "store: fix marshaling with sync.Pool (thanos-io#4593)"
Browse files Browse the repository at this point in the history
This reverts commit 8b4c3c9.

Signed-off-by: Giedrius Statkevičius <giedrius.statkevicius@vinted.com>
  • Loading branch information
GiedriusS committed Aug 27, 2021
1 parent 38a1bc6 commit 0f4e046
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 78 deletions.
53 changes: 20 additions & 33 deletions pkg/store/storepb/custom.go
Original file line number Diff line number Diff line change
Expand Up @@ -497,45 +497,32 @@ func (m *SeriesResponse) Marshal() (dAtA []byte, err error) {

var respBuf []byte

// Slow path with no sync.Pool.
// No pool defined, allocate directly.
if m.respPool == nil {
respBuf = make([]byte, size)

n, err := m.MarshalToSizedBuffer(respBuf)
if err != nil {
return nil, err
}
return respBuf[len(respBuf)-n:], nil
}

// Fast path with sync.Pool.
// m.respBuf must not be nil so that it would be returned to the pool.

// If no pre-allocated buffer has been passed then try to get a new one.
if m.respBuf == nil {
poolBuf := m.respPool.Get()
// No previous buffer found in the pool, try to allocate.
if poolBuf == nil {
respBuf = make([]byte, size)
} else {
// Found something, let's see if it is big enough.
respBuf = *(poolBuf.(*[]byte))
}
} else {
respBuf = *m.respBuf
}

// Last sanity check of the size before the marshaling.
if cap(respBuf) < size {
if m.respPool != nil {
m.respPool.Put(&respBuf)
if m.respBuf == nil {
poolBuf := m.respPool.Get()
if poolBuf == nil {
respBuf = make([]byte, size)
m.respBuf = &respBuf
} else {
m.respBuf = poolBuf.(*[]byte)
respBuf = *m.respBuf
}
} else {
if cap(*m.respBuf) < size {
if m.respPool != nil {
m.respPool.Put(m.respBuf)
}
respBuf = make([]byte, size)
m.respBuf = &respBuf
} else {
respBuf = *m.respBuf
}
}
respBuf = make([]byte, size)
}
m.respBuf = &respBuf

// Possibly trim it so that there wouldn't be left-over "garbage" in the slice.
// TODO: check if it is needed to always trim this.
marshalBuf := respBuf[:size]
n, err := m.MarshalToSizedBuffer(marshalBuf)
if err != nil {
Expand Down
45 changes: 0 additions & 45 deletions pkg/store/storepb/custom_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (
"fmt"
"path/filepath"
"sort"
"sync"
"testing"

"github.com/pkg/errors"
Expand Down Expand Up @@ -526,47 +525,3 @@ func TestMatchersToString_Translate(t *testing.T) {

}
}

// Tests whether the Marshal() function properly checks the size
// of a slice returned from a sync.Pool.
// Regression test against https://github.com/thanos-io/thanos/issues/4591.
func TestMarshalChecksSize(t *testing.T) {
var s Series

rawData := []rawSeries{
{
lset: labels.Labels{labels.Label{Name: "a", Value: "c"}},
chunks: [][]sample{
{{t: 11, v: 11}, {t: 12, v: 12}, {t: 13, v: 13}, {t: 14, v: 14}},
{{t: 1, v: 1}, {t: 2, v: 2}, {t: 3, v: 3}, {t: 4, v: 4}},
{{t: 20, v: 20}, {t: 21, v: 21}, {t: 22, v: 22}, {t: 24, v: 24}},
{{t: 11, v: 11}, {t: 12, v: 12}, {t: 13, v: 13}, {t: 14, v: 14}},
{{t: 15, v: 15}, {t: 16, v: 16}, {t: 17, v: 17}, {t: 18, v: 18}},
{{t: 20, v: 20}, {t: 21, v: 21}, {t: 22, v: 23}, {t: 24, v: 24}},
{{t: 11, v: 11}, {t: 12, v: 12}, {t: 13, v: 13}, {t: 14, v: 14}},
},
},
}

listSS := newListSeriesSet(t, rawData)
lset, chks := listSS.At()

s.Chunks = chks
s.Labels = labelpb.ZLabelsFromPromLabels(lset)

resp := NewSeriesResponse(&s)

smallPool := sync.Pool{
New: func() interface{} {
b := make([]byte, 1)
return &b
},
}
resp.respPool = &smallPool

d, err := resp.Marshal()
testutil.Ok(t, err)
testutil.Assert(t, d != nil)

resp.Close()
}

0 comments on commit 0f4e046

Please sign in to comment.