Skip to content

Commit

Permalink
Dont use pool for allocating tag slices (#1300)
Browse files Browse the repository at this point in the history
  • Loading branch information
richardartoul authored Jan 17, 2019
1 parent 8b04e5d commit 8e318ef
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 9 deletions.
28 changes: 21 additions & 7 deletions src/dbnode/storage/index/convert/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -183,11 +183,15 @@ func TagsFromTagsIter(
iter ident.TagIterator,
idPool ident.Pool,
) (ident.Tags, error) {
var (
seriesIDBytes = ident.BytesID(seriesID.Bytes())
tags = idPool.Tags()
)
var tags ident.Tags
if idPool != nil {
tags = idPool.Tags()
} else {
tagSlice := make([]ident.Tag, 0, iter.Len())
tags = ident.NewTags(tagSlice...)
}

seriesIDBytes := ident.BytesID(seriesID.Bytes())
for iter.Next() {
curr := iter.Current()

Expand All @@ -200,17 +204,27 @@ func TagsFromTagsIter(
tag.Name = seriesIDBytes[idx : idx+len(nameBytes)]
idRef = true
} else {
tag.Name = idPool.Clone(curr.Name)
if idPool != nil {
tag.Name = idPool.Clone(curr.Name)
} else {
copiedBytes := append([]byte(nil), curr.Name.Bytes()...)
tag.Name = ident.BytesID(copiedBytes)
}
}
if idx := bytes.Index(seriesIDBytes, valueBytes); idx != -1 {
tag.Value = seriesIDBytes[idx : idx+len(valueBytes)]
idRef = true
} else {
tag.Value = idPool.Clone(curr.Value)
if idPool != nil {
tag.Value = idPool.Clone(curr.Value)
} else {
copiedBytes := append([]byte(nil), curr.Value.Bytes()...)
tag.Value = ident.BytesID(copiedBytes)
}
}

if idRef {
tag.NoFinalize() // Taken ref, cannot finalize this
tag.NoFinalize() // Taken ref, cannot finalize this.
}

tags.Append(tag)
Expand Down
15 changes: 15 additions & 0 deletions src/dbnode/storage/index/convert/convert_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,21 @@ func TestTagsFromTagsIter(t *testing.T) {
require.True(t, true, expectedTags.Equal(tags))
}

func TestTagsFromTagsIterNoPool(t *testing.T) {
var (
id = ident.StringID("foo")
expectedTags = ident.NewTags(
ident.StringTag("bar", "baz"),
ident.StringTag("foo", "m3"),
)
tagsIter = ident.NewTagsIterator(expectedTags)
)

tags, err := convert.TagsFromTagsIter(id, tagsIter, nil)
require.NoError(t, err)
require.True(t, true, expectedTags.Equal(tags))
}

func TestToMetricInvalidID(t *testing.T) {
d := doc.Document{
Fields: []doc.Field{
Expand Down
7 changes: 5 additions & 2 deletions src/dbnode/storage/shard.go
Original file line number Diff line number Diff line change
Expand Up @@ -1013,8 +1013,11 @@ func (s *dbShard) newShardEntry(
if tagsIter.CurrentIndex() != 0 {
return nil, errNewShardEntryTagsIterNotAtIndexZero
}
seriesTags, err = convert.TagsFromTagsIter(
seriesID, tagsIter, s.identifierPool)

// Pass nil for the identifier pool because the pool will force us to use an array
// with a large capacity to store the tags. Since these tags are long-lived, it's
// better to allocate an array of the exact size to save memory.
seriesTags, err = convert.TagsFromTagsIter(seriesID, tagsIter, nil)
tagsIter.Close()
if err != nil {
return nil, err
Expand Down
1 change: 1 addition & 0 deletions src/dbnode/storage/shard_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1097,6 +1097,7 @@ func TestShardNewInvalidShardEntry(t *testing.T) {
gomock.InOrder(
iter.EXPECT().Duplicate().Return(iter),
iter.EXPECT().CurrentIndex().Return(0),
iter.EXPECT().Len().Return(0),
iter.EXPECT().Next().Return(false),
iter.EXPECT().Err().Return(fmt.Errorf("random err")),
iter.EXPECT().Close(),
Expand Down

0 comments on commit 8e318ef

Please sign in to comment.