Skip to content

Commit

Permalink
fix[mimirtool]: order samples when performing export (#5700)
Browse files Browse the repository at this point in the history
* fix[mimirtool]: prevent out of bounds on export

* Update pkg/mimirtool/commands/remote_read_test.go

---------

Co-authored-by: Dimitar Dimitrov <dimitar.dimitrov@grafana.com>
  • Loading branch information
theSuess and dimitarvdimitrov committed Sep 1, 2023
1 parent 74e3389 commit cf87d8f
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 5 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
### Jsonnet

### Mimirtool
* [BUGFIX] Fix out of bounds error on export with large timespans and/or series count. #5700

### Mimir Continuous Test

Expand Down
4 changes: 2 additions & 2 deletions pkg/mimirtool/backfill/backfill.go
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ func CreateBlocks(input IteratorCreator, mint, maxt int64, maxSamplesInAppender

var wroteHeader bool

for t := mint; t <= maxt; t = t + blockDuration {
for t := mint; t <= maxt; t = t + blockDuration/2 {
err := func() error {
w, err := tsdb.NewBlockWriter(log.NewNopLogger(), outputDir, blockDuration)
if err != nil {
Expand All @@ -106,7 +106,7 @@ func CreateBlocks(input IteratorCreator, mint, maxt int64, maxSamplesInAppender
ctx := context.Background()
app := w.Appender(ctx)
i := input()
tsUpper := t + blockDuration
tsUpper := t + blockDuration/2
samplesCount := 0
for {
err := i.Next()
Expand Down
5 changes: 2 additions & 3 deletions pkg/mimirtool/commands/remote_read.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,8 +416,7 @@ func (c *RemoteReadCommand) export(_ *kingpin.ParseContext) error {
if err != nil {
return err
}

iterator := func() backfill.Iterator {
iteratorCreator := func() backfill.Iterator {
return newTimeSeriesIterator(timeseries)
}

Expand All @@ -434,7 +433,7 @@ func (c *RemoteReadCommand) export(_ *kingpin.ParseContext) error {
defer pipeR.Close()

log.Infof("Store TSDB blocks in '%s'", c.tsdbPath)
if err := backfill.CreateBlocks(iterator, int64(mint), int64(maxt), 1000, c.tsdbPath, true, pipeW); err != nil {
if err := backfill.CreateBlocks(iteratorCreator, int64(mint), int64(maxt), 1000, c.tsdbPath, true, pipeW); err != nil {
return err
}

Expand Down
42 changes: 42 additions & 0 deletions pkg/mimirtool/commands/remote_read_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,16 @@
package commands

import (
"fmt"
"io"
"testing"
"time"

"github.com/pkg/errors"
"github.com/prometheus/prometheus/prompb"
"github.com/stretchr/testify/assert"

"github.com/grafana/mimir/pkg/mimirtool/backfill"
)

func TestTimeSeriesIterator(t *testing.T) {
Expand Down Expand Up @@ -150,3 +154,41 @@ func TestTimeSeriesIterator(t *testing.T) {
}

}

// TestEarlyCommit writes samples of many series that don't fit into the same
// append commit. It makes sure that batching the samples into many commits
// doesn't cause the appends to advance the head block too far and make future
// appends invalid.
func TestEarlyCommit(t *testing.T) {
maxSamplesPerBlock := 1000
series := 100
samples := 140

start := int64(time.Date(2023, 8, 30, 11, 42, 17, 0, time.UTC).UnixNano())
inc := int64(time.Minute / time.Millisecond)
end := start + (inc * int64(samples))
ts := make([]*prompb.TimeSeries, series)
for i := 0; i < series; i++ {
s := &prompb.TimeSeries{
Labels: []prompb.Label{
{
Name: "__name__",
Value: fmt.Sprintf("metric_%d", i),
},
},
Samples: make([]prompb.Sample, samples),
}
for j := 0; j < samples; j++ {
s.Samples[j] = prompb.Sample{
Value: float64(j),
Timestamp: start + (inc * int64(j)),
}
}
ts[i] = s
}
iterator := func() backfill.Iterator {
return newTimeSeriesIterator(ts)
}
err := backfill.CreateBlocks(iterator, start, end, maxSamplesPerBlock, t.TempDir(), true, io.Discard)
assert.NoError(t, err)
}

0 comments on commit cf87d8f

Please sign in to comment.