diff --git a/src/m3ninx/index/segment/builder/multi_segments_builder.go b/src/m3ninx/index/segment/builder/multi_segments_builder.go index 459b93f524..ef890fd30a 100644 --- a/src/m3ninx/index/segment/builder/multi_segments_builder.go +++ b/src/m3ninx/index/segment/builder/multi_segments_builder.go @@ -138,6 +138,12 @@ func (b *builderFromSegments) AddSegments(segments []segment.Segment) error { b.segmentsOffset += postings.ID(added) } + // Sort segments in descending order in terms of size so the multi segments + // terms iter more efficiently builds its postings list. + sort.Slice(b.segments, func(i, j int) bool { + return b.segments[i].segment.Size() > b.segments[j].segment.Size() + }) + // Make sure the terms iter has all the segments to combine data from b.termsIter.reset(b.segments) diff --git a/src/m3ninx/index/segment/builder/multi_segments_builder_test.go b/src/m3ninx/index/segment/builder/multi_segments_builder_test.go new file mode 100644 index 0000000000..b11097ecdd --- /dev/null +++ b/src/m3ninx/index/segment/builder/multi_segments_builder_test.go @@ -0,0 +1,79 @@ +// Copyright (c) 2020 Uber Technologies, Inc. +// +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to deal +// in the Software without restriction, including without limitation the rights +// to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +// copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +// OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +// THE SOFTWARE. + +package builder + +import ( + "sort" + "testing" + + "github.com/golang/mock/gomock" + "github.com/stretchr/testify/require" + + "github.com/m3db/m3/src/m3ninx/index" + "github.com/m3db/m3/src/m3ninx/index/segment" + "github.com/m3db/m3/src/m3ninx/postings" +) + +// TestMultiSegmentsBuilderSortedBySizeDescending ensures that segments in the +// multi segments builder are in descending order by size for optimal compaction. +func TestMultiSegmentsBuilderSortedBySizeDescending(t *testing.T) { + ctrl := gomock.NewController(t) + defer ctrl.Finish() + + b := NewBuilderFromSegments(NewOptions()).(*builderFromSegments) + + numSegments := 5 + segs := make([]segment.Segment, 0, numSegments) + for i := 1; i <= numSegments; i++ { + pi := postings.NewMockIterator(ctrl) + gomock.InOrder( + pi.EXPECT().Next().Return(false), + pi.EXPECT().Close().Return(nil), + ) + r := segment.NewMockReader(ctrl) + it := index.NewIDDocIterator(nil, pi) + gomock.InOrder( + r.EXPECT().AllDocs().Return(it, nil), + r.EXPECT().Close().Return(nil), + ) + + seg := segment.NewMockSegment(ctrl) + seg.EXPECT().Reader().Return(r, nil) + seg.EXPECT().Size().Return(int64(i * 10)).AnyTimes() + seg.EXPECT().TermsIterable().Return(nil) + segs = append(segs, seg) + } + require.NoError(t, b.AddSegments(segs)) + + // Sort segments in descending order by size. + sort.Slice(segs, func(i, j int) bool { + return segs[i].Size() > segs[j].Size() + }) + + actualSegs := make([]segment.Segment, 0, numSegments) + for _, segMD := range b.segments { + actualSegs = append(actualSegs, segMD.segment) + } + + for i := 0; i < numSegments; i++ { + require.Equal(t, segs[i].Size(), actualSegs[i].Size()) + } +}