Skip to content

Commit

Permalink
Revert "Fix(builder): Too many small tables when compression is enabl…
Browse files Browse the repository at this point in the history
…ed (#1549)"

This reverts commit 5d1bab4.
We'reverting this commit because it seems to cause a strange issue while
writing data. Running `go run -tags main.go benchmark write --sorted
--compression=true --block-cache-mb=100` creates a directory which does
not have all the keys. I don't know why this would fix the issue but the
test works fine after reverting this commit.
  • Loading branch information
Ibrahim Jarif committed Oct 5, 2020
1 parent d74698d commit 76a893f
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 14 deletions.
2 changes: 1 addition & 1 deletion levels.go
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,7 @@ nextTable:
}

if !y.SameKey(it.Key(), lastKey) {
if builder.ReachedCapacity(uint64(float64(s.kv.opt.MaxTableSize) * 0.9)) {
if builder.ReachedCapacity(s.kv.opt.MaxTableSize) {
// Only break if we are on a different key, and have reached capacity. We want
// to ensure that all versions of the key are stored in the same sstable, and
// not divided across multiple tables at the same level.
Expand Down
2 changes: 1 addition & 1 deletion stream_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,7 @@ func (w *sortedWriter) Add(key []byte, vs y.ValueStruct) error {

sameKey := y.SameKey(key, w.lastKey)
// Same keys should go into the same SSTable.
if !sameKey && w.builder.ReachedCapacity(uint64(float64(w.db.opt.MaxTableSize)*0.9)) {
if !sameKey && w.builder.ReachedCapacity(w.db.opt.MaxTableSize) {
if err := w.send(false); err != nil {
return err
}
Expand Down
18 changes: 6 additions & 12 deletions table/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import (
"math"
"runtime"
"sync"
"sync/atomic"
"unsafe"

"github.com/dgraph-io/badger/v2/fb"
Expand Down Expand Up @@ -77,10 +76,9 @@ type bblock struct {
// Builder is used in building a table.
type Builder struct {
// Typically tens or hundreds of meg. This is for one single file.
buf []byte
sz uint32
bufLock sync.Mutex // This lock guards the buf. We acquire lock when we resize the buf.
actualSize uint32 // Used to store the sum of sizes of blocks after compression/encryption.
buf []byte
sz uint32
bufLock sync.Mutex // This lock guards the buf. We acquire lock when we resize the buf.

baseKey []byte // Base key for the current block.
baseOffset uint32 // Offset for the current block.
Expand Down Expand Up @@ -158,9 +156,6 @@ func (b *Builder) handleBlock() {
copy(b.buf[item.start:], blockBuf)
b.bufLock.Unlock()

// Add the actual size of current block.
atomic.AddUint32(&b.actualSize, uint32(len(blockBuf)))

// Fix the boundary of the block.
item.end = item.start + uint32(len(blockBuf))

Expand Down Expand Up @@ -290,7 +285,6 @@ func (b *Builder) finishBlock() {
// If compression/encryption is disabled, no need to send the block to the blockChan.
// There's nothing to be done.
if b.blockChan == nil {
atomic.StoreUint32(&b.actualSize, b.sz)
b.addBlockToIndex()
return
}
Expand Down Expand Up @@ -371,8 +365,8 @@ func (b *Builder) Add(key []byte, value y.ValueStruct, valueLen uint32) {
// at the end. The diff can vary.

// ReachedCapacity returns true if we... roughly (?) reached capacity?
func (b *Builder) ReachedCapacity(capacity uint64) bool {
blocksSize := atomic.LoadUint32(&b.actualSize) + // actual length of current buffer
func (b *Builder) ReachedCapacity(cap int64) bool {
blocksSize := b.sz + // length of current buffer
uint32(len(b.entryOffsets)*4) + // all entry offsets size
4 + // count of all entry offsets
8 + // checksum bytes
Expand All @@ -382,7 +376,7 @@ func (b *Builder) ReachedCapacity(capacity uint64) bool {
4 + // Index length
uint32(b.offsets.Len())

return uint64(estimateSz) > capacity
return int64(estimateSz) > cap
}

// Finish finishes the table by appending the index.
Expand Down

0 comments on commit 76a893f

Please sign in to comment.