Skip to content
This repository has been archived by the owner on Aug 13, 2019. It is now read-only.

Don't write empty blocks #374

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
beb0d73
Dont write empty blocks
codesome Sep 8, 2018
bb76c82
Added unit test TestNoEmptyBlock
codesome Sep 12, 2018
c4edbcc
Fix infinite loop while compacting head.
codesome Sep 18, 2018
9de3926
Fix deletion of old blocks after no block is written.
codesome Sep 19, 2018
ab14c9c
Fix review comments
codesome Sep 19, 2018
268ae54
Merge remote-tracking branch 'upstream/master' into dont-write-empty-…
codesome Sep 21, 2018
281a493
Merge remote-tracking branch 'upstream/master' into dont-write-empty-…
codesome Sep 28, 2018
897dd33
Fixed infinite loop for head compaction
codesome Sep 28, 2018
9938162
Merge remote-tracking branch 'upstream/master' into dont-write-empty-…
codesome Oct 1, 2018
0faafec
Test fix attempt
codesome Oct 3, 2018
49631bb
Merge remote-tracking branch 'upstream/master' into dont-write-empty-…
codesome Nov 20, 2018
38a2c6b
Updated tests
codesome Nov 22, 2018
061971b
Fix review comments
codesome Nov 23, 2018
059dbd7
Merge remote-tracking branch 'upstream/master' into dont-write-empty-…
codesome Dec 1, 2018
2378d2b
Updated tests and added CHANGELOG entry
codesome Dec 1, 2018
98988fd
Merge remote-tracking branch 'upstream/master' into pull/374-review
Jan 16, 2019
e0bb757
rebased
Jan 16, 2019
efa23ce
Merge pull request #4 from krasi-georgiev/pull/374-review
codesome Jan 16, 2019
0eed036
Revert returning after empty block from head
codesome Jan 16, 2019
271b98f
Fix review comments
codesome Jan 16, 2019
a6a1779
Dont create empty blocks in tests
codesome Jan 16, 2019
2b509e7
nits
codesome Jan 17, 2019
ad9e3ed
simplified test
Jan 17, 2019
441ea45
Merge remote-tracking branch 'upstream/master' into pull/374-review
Jan 17, 2019
02dc732
solved the mistery for double compaction
Jan 17, 2019
162fa29
nit
Jan 17, 2019
9f24cde
less samples
Jan 17, 2019
2cb745f
Merge pull request #5 from krasi-georgiev/pull/374-review
codesome Jan 17, 2019
45bde4c
Nits
codesome Jan 17, 2019
6b87a56
revert some changes
Jan 18, 2019
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions block.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,10 @@ type BlockMetaCompaction struct {
Level int `json:"level"`
// ULIDs of all source head blocks that went into the block.
Sources []ulid.ULID `json:"sources,omitempty"`
// As we dont write empty blocks, we need this to mark
// if the block is deletable if in compaction this block
krasi-georgiev marked this conversation as resolved.
Show resolved Hide resolved
// resulted in an empty block.
krasi-georgiev marked this conversation as resolved.
Show resolved Hide resolved
Deletable bool `json:"deletable,omitempty"`
// Short descriptions of the direct blocks that were used to create
// this block.
Parents []BlockDesc `json:"parents,omitempty"`
Expand Down
46 changes: 34 additions & 12 deletions compact.go
Original file line number Diff line number Diff line change
Expand Up @@ -349,14 +349,29 @@ func (c *LeveledCompactor) Compact(dest string, dirs ...string) (uid ulid.ULID,
meta := compactBlockMetas(uid, metas...)
err = c.write(dest, meta, blocks...)
if err == nil {
level.Info(c.logger).Log(
"msg", "compact blocks",
"count", len(blocks),
"mint", meta.MinTime,
"maxt", meta.MaxTime,
"ulid", meta.ULID,
"sources", fmt.Sprintf("%v", uids),
)

if meta.Stats.NumSamples == 0 {
level.Info(c.logger).Log(
"msg", "compact blocks [resulted in empty block]",
gouthamve marked this conversation as resolved.
Show resolved Hide resolved
"count", len(blocks),
"sources", fmt.Sprintf("%v", uids),
)
for _, b := range bs {
b.meta.Compaction.Deletable = true
writeMetaFile(b.dir, &b.meta)
codesome marked this conversation as resolved.
Show resolved Hide resolved
}
uid = ulid.ULID{}
} else {
level.Info(c.logger).Log(
"msg", "compact blocks",
"count", len(blocks),
"mint", meta.MinTime,
"maxt", meta.MaxTime,
"ulid", meta.ULID,
"sources", fmt.Sprintf("%v", uids),
)
}

return uid, nil
}

Expand Down Expand Up @@ -471,17 +486,24 @@ func (c *LeveledCompactor) write(dest string, meta *BlockMeta, blocks ...BlockRe
return errors.Wrap(err, "write compaction")
}

if err = writeMetaFile(tmp, meta); err != nil {
return errors.Wrap(err, "write merged meta")
}

if err = chunkw.Close(); err != nil {
return errors.Wrap(err, "close chunk writer")
}
if err = indexw.Close(); err != nil {
return errors.Wrap(err, "close index writer")
}

if meta.Stats.NumSamples == 0 {
krasi-georgiev marked this conversation as resolved.
Show resolved Hide resolved
if err := os.RemoveAll(tmp); err != nil {
return errors.Wrap(err, "remove tmp folder after empty block failed")
}
return nil
}

if err = writeMetaFile(tmp, meta); err != nil {
return errors.Wrap(err, "write merged meta")
}

// Create an empty tombstones file.
if err := writeTombstoneFile(tmp, NewMemTombstones()); err != nil {
return errors.Wrap(err, "write new tombstones file")
Expand Down
19 changes: 19 additions & 0 deletions db.go
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,22 @@ func (db *DB) compact() (changes bool, err error) {
if err := db.reload(); err != nil {
return changes, errors.Wrap(err, "reload blocks")
}
// After this is fixed https://github.com/prometheus/tsdb/issues/309,
// if the compaction of head did not create any new blocks (possible
// if the samples were deleted in that range), and if number of
// blocks are 0, db.reload() does not truncate head. Hence MinTime
// of head is not set properly. So only for that situation, we need
// to call db.head.Truncate(..) manually.
krasi-georgiev marked this conversation as resolved.
Show resolved Hide resolved
db.mtx.RLock()
l := len(db.blocks)
db.mtx.RUnlock()
if l == 0 {
err = db.head.Truncate(maxt)
if err != nil {
return changes, errors.Wrap(err, "head truncate failed (in compact)")

}
}
gouthamve marked this conversation as resolved.
Show resolved Hide resolved
runtime.GC()
}

Expand Down Expand Up @@ -468,6 +484,9 @@ func (db *DB) reload() (err error) {
corrupted[ulid] = err
continue
}
if meta.Compaction.Deletable {
deleteable[meta.ULID] = struct{}{}
}
if db.beyondRetention(meta) {
deleteable[meta.ULID] = struct{}{}
continue
Expand Down
59 changes: 59 additions & 0 deletions db_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1301,3 +1301,62 @@ func TestInitializeHeadTimestamp(t *testing.T) {
testutil.Equals(t, int64(15000), db.head.MaxTime())
})
}

func TestNoEmptyBlock(t *testing.T) {
krasi-georgiev marked this conversation as resolved.
Show resolved Hide resolved
db, close := openTestDB(t, nil)
defer close()
defer db.Close()

// Test no blocks after compact with empty head.
_, err := db.compact()
testutil.Ok(t, err)
testutil.Equals(t, 0, len(db.blocks))
krasi-georgiev marked this conversation as resolved.
Show resolved Hide resolved

// Test no blocks after deleting all samples from head.
blockRange := DefaultOptions.BlockRanges[0]
label := labels.FromStrings("foo", "bar")
codesome marked this conversation as resolved.
Show resolved Hide resolved

app := db.Appender()
for i := int64(0); i < 3; i++ {
_, err := app.Add(label, i*blockRange, 0)
testutil.Ok(t, err)
_, err = app.Add(label, i*blockRange+1000, 0)
testutil.Ok(t, err)
}
err = app.Commit()
testutil.Ok(t, err)

testutil.Ok(t, db.Delete(math.MinInt64, math.MaxInt64, labels.NewEqualMatcher("foo", "bar")))
_, err = db.compact()
testutil.Ok(t, err)
// No blocks created.
testutil.Equals(t, 0, len(db.blocks))
krasi-georgiev marked this conversation as resolved.
Show resolved Hide resolved

// Test no blocks remaining after small samples are deleted from disk.
krasi-georgiev marked this conversation as resolved.
Show resolved Hide resolved
app = db.Appender()
for i := int64(3); i < 20; i++ {
for j := int64(0); j < 10; j++ {
_, err := app.Add(label, i*blockRange+j, 0)
testutil.Ok(t, err)
}
}
err = app.Commit()
testutil.Ok(t, err)

_, err = db.compact()
testutil.Ok(t, err)
testutil.Assert(t, len(db.blocks) > 0, "No blocks created")
krasi-georgiev marked this conversation as resolved.
Show resolved Hide resolved

testutil.Ok(t, db.Delete(math.MinInt64, math.MaxInt64, labels.NewEqualMatcher("foo", "bar")))

// Mimicking small part of compaction.
krasi-georgiev marked this conversation as resolved.
Show resolved Hide resolved
plan := []string{}
for _, b := range db.blocks {
plan = append(plan, b.Dir())
}
_, err = db.compactor.Compact(db.dir, plan...)
testutil.Ok(t, err)
testutil.Ok(t, db.reload())
// All samples are deleted. No blocks should be remeianing after compact.
testutil.Equals(t, 0, len(db.blocks))
krasi-georgiev marked this conversation as resolved.
Show resolved Hide resolved
}