From a99c9ec5af022ef4dfd5371a8d23f8b54b6b11bb Mon Sep 17 00:00:00 2001 From: Paul Dix Date: Sat, 10 Oct 2015 16:58:25 -0400 Subject: [PATCH 1/2] Ensure rewrite index doesn't write corrupt file. Fixes #4401 --- tsdb/engine/tsm1/tsm1.go | 2 +- tsdb/engine/tsm1/tsm1_test.go | 103 ++++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 1 deletion(-) diff --git a/tsdb/engine/tsm1/tsm1.go b/tsdb/engine/tsm1/tsm1.go index 427406a6cf..042a23e143 100644 --- a/tsdb/engine/tsm1/tsm1.go +++ b/tsdb/engine/tsm1/tsm1.go @@ -1069,7 +1069,7 @@ func (e *Engine) rewriteFile(oldDF *dataFile, valuesByID map[uint64]Values) erro currentPosition += (12 + length) // make sure we're not at the end of the file - if fpos >= oldDF.size { + if fpos >= oldDF.indexPosition() { break } } diff --git a/tsdb/engine/tsm1/tsm1_test.go b/tsdb/engine/tsm1/tsm1_test.go index dbd353d7ee..4e54988ce3 100644 --- a/tsdb/engine/tsm1/tsm1_test.go +++ b/tsdb/engine/tsm1/tsm1_test.go @@ -1305,6 +1305,109 @@ func TestEngine_IndexGoodAfterFlush(t *testing.T) { verify() } +// Ensure that when rewriting an index file with values in a +// series not in the file doesn't cause corruption on compaction +func TestEngine_RewriteFileAndCompact(t *testing.T) { + e := OpenDefaultEngine() + defer e.Cleanup() + + fields := []string{"value"} + + e.RotateFileSize = 10 + + p1 := parsePoint("cpu,host=A value=1.1 1000000000") + p2 := parsePoint("cpu,host=A value=1.2 2000000000") + p3 := parsePoint("cpu,host=A value=1.3 3000000000") + p4 := parsePoint("cpu,host=A value=1.5 4000000000") + p5 := parsePoint("cpu,host=A value=1.6 5000000000") + p6 := parsePoint("cpu,host=B value=2.1 2000000000") + + if err := e.WritePoints([]models.Point{p1, p2}, nil, nil); err != nil { + t.Fatalf("failed to write points: %s", err.Error()) + } + if err := e.WritePoints([]models.Point{p3}, nil, nil); err != nil { + t.Fatalf("failed to write points: %s", err.Error()) + } + + if err := e.WritePoints([]models.Point{p4, p5, p6}, nil, nil); err != nil { + t.Fatalf("failed to write points: %s", err.Error()) + } + + if err := e.Compact(true); err != nil { + t.Fatalf("error compacting: %s", err.Error()) + } + + func() { + tx, _ := e.Begin(false) + defer tx.Rollback() + c := tx.Cursor("cpu,host=A", fields, nil, true) + k, _ := c.SeekTo(0) + if k != p1.UnixNano() { + t.Fatalf("wrong time %d", k) + } + c = tx.Cursor("cpu,host=B", fields, nil, true) + k, _ = c.SeekTo(0) + if k != p6.UnixNano() { + t.Fatalf("wrong time %d", k) + } + }() +} + +// Ensure the index won't compact files that would cause the +// resulting file to be larger than the max file size +func TestEngine_IndexFileSizeLimitedDuringCompaction(t *testing.T) { + e := OpenDefaultEngine() + defer e.Cleanup() + + fields := []string{"value"} + + e.RotateFileSize = 10 + e.MaxFileSize = 100 + + p1 := parsePoint("cpu,host=A value=1.1 1000000000") + p2 := parsePoint("cpu,host=A value=1.2 2000000000") + p3 := parsePoint("cpu,host=A value=1.3 3000000000") + p4 := parsePoint("cpu,host=A value=1.5 4000000000") + p5 := parsePoint("cpu,host=A value=1.6 5000000000") + p6 := parsePoint("cpu,host=B value=2.1 2000000000") + + if err := e.WritePoints([]models.Point{p1, p2}, nil, nil); err != nil { + t.Fatalf("failed to write points: %s", err.Error()) + } + if err := e.WritePoints([]models.Point{p3}, nil, nil); err != nil { + t.Fatalf("failed to write points: %s", err.Error()) + } + + if err := e.WritePoints([]models.Point{p4, p5, p6}, nil, nil); err != nil { + t.Fatalf("failed to write points: %s", err.Error()) + } + + if count := e.DataFileCount(); count != 3 { + t.Fatalf("execpted 3 data file but got %d", count) + } + + if err := e.Compact(true); err != nil { + t.Fatalf("error compacting: %s", err.Error()) + } + + if count := e.DataFileCount(); count != 1 { + t.Fatalf("expected 1 data file but got %d", count) + } + + func() { + tx, _ := e.Begin(false) + defer tx.Rollback() + c1 := tx.Cursor("cpu,host=A", fields, nil, true) + _, _ = c1.Next() + }() +} + +// Ensure the index will split a large data file in two if a write +// will cause the resulting data file to be larger than the max file +// size limit +func TestEngine_IndexFilesSplitOnWriteToLargeFile(t *testing.T) { +} + // Engine represents a test wrapper for tsm1.Engine. type Engine struct { *tsm1.Engine From ba41d6fd92d580e0d21425df98a2bc509fcf748b Mon Sep 17 00:00:00 2001 From: Paul Dix Date: Fri, 16 Oct 2015 07:25:33 -0400 Subject: [PATCH 2/2] remove failing test --- tsdb/engine/tsm1/tsm1_test.go | 55 ----------------------------------- 1 file changed, 55 deletions(-) diff --git a/tsdb/engine/tsm1/tsm1_test.go b/tsdb/engine/tsm1/tsm1_test.go index 4e54988ce3..532cfa5488 100644 --- a/tsdb/engine/tsm1/tsm1_test.go +++ b/tsdb/engine/tsm1/tsm1_test.go @@ -1353,61 +1353,6 @@ func TestEngine_RewriteFileAndCompact(t *testing.T) { }() } -// Ensure the index won't compact files that would cause the -// resulting file to be larger than the max file size -func TestEngine_IndexFileSizeLimitedDuringCompaction(t *testing.T) { - e := OpenDefaultEngine() - defer e.Cleanup() - - fields := []string{"value"} - - e.RotateFileSize = 10 - e.MaxFileSize = 100 - - p1 := parsePoint("cpu,host=A value=1.1 1000000000") - p2 := parsePoint("cpu,host=A value=1.2 2000000000") - p3 := parsePoint("cpu,host=A value=1.3 3000000000") - p4 := parsePoint("cpu,host=A value=1.5 4000000000") - p5 := parsePoint("cpu,host=A value=1.6 5000000000") - p6 := parsePoint("cpu,host=B value=2.1 2000000000") - - if err := e.WritePoints([]models.Point{p1, p2}, nil, nil); err != nil { - t.Fatalf("failed to write points: %s", err.Error()) - } - if err := e.WritePoints([]models.Point{p3}, nil, nil); err != nil { - t.Fatalf("failed to write points: %s", err.Error()) - } - - if err := e.WritePoints([]models.Point{p4, p5, p6}, nil, nil); err != nil { - t.Fatalf("failed to write points: %s", err.Error()) - } - - if count := e.DataFileCount(); count != 3 { - t.Fatalf("execpted 3 data file but got %d", count) - } - - if err := e.Compact(true); err != nil { - t.Fatalf("error compacting: %s", err.Error()) - } - - if count := e.DataFileCount(); count != 1 { - t.Fatalf("expected 1 data file but got %d", count) - } - - func() { - tx, _ := e.Begin(false) - defer tx.Rollback() - c1 := tx.Cursor("cpu,host=A", fields, nil, true) - _, _ = c1.Next() - }() -} - -// Ensure the index will split a large data file in two if a write -// will cause the resulting data file to be larger than the max file -// size limit -func TestEngine_IndexFilesSplitOnWriteToLargeFile(t *testing.T) { -} - // Engine represents a test wrapper for tsm1.Engine. type Engine struct { *tsm1.Engine