Skip to content

Commit

Permalink
Do not return error from builder.Add method (#958)
Browse files Browse the repository at this point in the history
The builder.Add method doesn't need to return an error. We always return
nil and it has to be handled everywhere. This commit removes it.

(cherry picked from commit e627d49)
  • Loading branch information
Ibrahim Jarif committed Mar 10, 2020
1 parent 68da170 commit f34085e
Show file tree
Hide file tree
Showing 11 changed files with 44 additions and 84 deletions.
4 changes: 1 addition & 3 deletions db.go
Original file line number Diff line number Diff line change
Expand Up @@ -843,9 +843,7 @@ func writeLevel0Table(ft flushTask, f io.Writer) error {
if len(ft.dropPrefix) > 0 && bytes.HasPrefix(iter.Key(), ft.dropPrefix) {
continue
}
if err := b.Add(iter.Key(), iter.Value()); err != nil {
return err
}
b.Add(iter.Key(), iter.Value())
}
_, err := f.Write(b.Finish())
return err
Expand Down
2 changes: 1 addition & 1 deletion db2_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -503,7 +503,7 @@ func createTableWithRange(t *testing.T, db *DB, start, end int) *table.Table {
binary.BigEndian.PutUint64(key[:], uint64(i))
key = y.KeyWithTs(key, uint64(0))
val := y.ValueStruct{Value: []byte(fmt.Sprintf("%d", i))}
require.NoError(t, b.Add(key, val))
b.Add(key, val)
}

fileID := db.lc.reserveFileID()
Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,12 @@ go 1.12

require (
github.com/AndreasBriese/bbloom v0.0.0-20190306092124-e2d15f34fcf9
github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2
github.com/dgraph-io/ristretto v0.0.2
github.com/dustin/go-humanize v1.0.0
github.com/golang/protobuf v1.3.1
github.com/pkg/errors v0.8.1
github.com/spf13/cobra v0.0.5
github.com/stretchr/testify v1.3.0
github.com/stretchr/testify v1.4.0
golang.org/x/net v0.0.0-20190620200207-3b0461eec859
golang.org/x/sys v0.0.0-20190626221950-04f50cda93cb
)
14 changes: 12 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
@@ -1,14 +1,20 @@
github.com/AndreasBriese/bbloom v0.0.0-20190306092124-e2d15f34fcf9 h1:HD8gA2tkByhMAwYaFAX9w2l7vxvBQ5NMoxDrkhqhtn4=
github.com/AndreasBriese/bbloom v0.0.0-20190306092124-e2d15f34fcf9/go.mod h1:bOvUY6CB00SOBii9/FifXqc0awNKxLFCL/+pkDPuyl8=
github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU=
github.com/OneOfOne/xxhash v1.2.2 h1:KMrpdQIwFcEqXDklaen+P1axHaj9BSKzvpUUfnHldSE=
github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU=
github.com/armon/consul-api v0.0.0-20180202201655-eb2c6b5be1b6/go.mod h1:grANhF5doyWs3UAsr3K4I6qtAmlQcZDesFNEHPZAzj8=
github.com/cespare/xxhash v1.1.0 h1:a6HrQnmkObjyL+Gs60czilIUGqrzKutQD6XZog3p+ko=
github.com/cespare/xxhash v1.1.0/go.mod h1:XrSqR1VqqWfGrhpAt58auRo0WTKS1nRRg3ghfAqPWnc=
github.com/coreos/etcd v3.3.10+incompatible/go.mod h1:uF7uidLiAD3TWHmW31ZFd/JWoc32PjwdhPthX9715RE=
github.com/coreos/go-etcd v2.0.0+incompatible/go.mod h1:Jez6KQU2B/sWsbdaef3ED8NzMklzPG4d5KIOhIy30Tk=
github.com/coreos/go-semver v0.2.0/go.mod h1:nnelYz7RCh+5ahJtPPxZlU+153eP4D4r3EedlOD2RNk=
github.com/cpuguy83/go-md2man v1.0.10/go.mod h1:SmD6nW6nTyfqj6ABTjUi3V3JVMnlJmwcJI5acqYI6dE=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/dgraph-io/ristretto v0.0.2 h1:a5WaUrDa0qm0YrAAS1tUykT5El3kt62KNZZeMxQn3po=
github.com/dgraph-io/ristretto v0.0.2/go.mod h1:KPxhHT9ZxKefz+PCeOGsrHpl1qZ7i70dGTu2u+Ahh6E=
github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2 h1:tdlZCpZ/P9DhczCTSixgIKmwPv6+wP5DGjqLYw5SUiA=
github.com/dgryski/go-farm v0.0.0-20190423205320-6a90982ecee2/go.mod h1:SqUrOPUnsFjfmXRMNPybcSiG0BgUW2AuFH8PAnS2iTw=
github.com/dustin/go-humanize v1.0.0 h1:VSnTsYCnlFHaM2/igO1h6X3HA71jcobQuxemgkq4zYo=
Expand All @@ -28,6 +34,8 @@ github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINE
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/russross/blackfriday v1.5.2/go.mod h1:JO/DiYxRf+HjHt06OyowR9PTA263kcR/rfWxYHBV53g=
github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72 h1:qLC7fQah7D6K1B0ujays3HV9gkFtllcxhzImRR7ArPQ=
github.com/spaolacci/murmur3 v0.0.0-20180118202830-f09979ecbc72/go.mod h1:JwIasOWyU6f++ZhiEuf87xNszmSA2myDM2Kzu9HwQUA=
github.com/spf13/afero v1.1.2/go.mod h1:j4pytiNVoe2o6bmDsKpLACNPDBIoEAkihy7loJ1B0CQ=
github.com/spf13/cast v1.3.0/go.mod h1:Qx5cxh0v+4UWYiBimWS+eyWzqEqokIECu5etghLkUJE=
github.com/spf13/cobra v0.0.5 h1:f0B+LkLX6DtmRH1isoNA9VTtNUK9K8xYd28JNNfOv/s=
Expand All @@ -38,8 +46,8 @@ github.com/spf13/pflag v1.0.3/go.mod h1:DYY7MBk1bdzusC3SYhjObp+wFpr4gzcvqqNjLnIn
github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.2.2/go.mod h1:a8OnRcib4nhh0OaRAV+Yts87kKdq0PP7pXfy6kDkUVs=
github.com/stretchr/testify v1.3.0 h1:TivCn/peBQ7UY8ooIcPgZFpTNSz0Q2U6UrFlUfqbe0Q=
github.com/stretchr/testify v1.3.0/go.mod h1:M5WIy9Dh21IEIfnGCwXGc5bZfKNJtfHm1UVUgZn+9EI=
github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0=
github.com/xordataexchange/crypt v0.0.3-0.20170626215501-b2862e3d0a77/go.mod h1:aYKd//L2LvnjZzWKhF00oedf4jCCReLcmhLdhm1A27Q=
golang.org/x/crypto v0.0.0-20181203042331-505ab145d0a9/go.mod h1:6SG95UA2DQfeDnfUPMdvaQW0Q7yPrPDi9nlGo2tz2b4=
Expand All @@ -51,5 +59,7 @@ golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5h
golang.org/x/sys v0.0.0-20190626221950-04f50cda93cb h1:fgwFCsaw9buMuxNd6+DQfAuSFqbNiQZpcgJQAgJsK6k=
golang.org/x/sys v0.0.0-20190626221950-04f50cda93cb/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
2 changes: 1 addition & 1 deletion levels.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,7 +556,7 @@ func (s *levelsController) compactBuildTables(
}
}
numKeys++
y.Check(builder.Add(it.Key(), it.Value()))
builder.Add(it.Key(), it.Value())
}
// It was true that it.Valid() at least once in the loop above, which means we
// called Add() at least once, and builder is not Empty().
Expand Down
7 changes: 1 addition & 6 deletions manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,16 +143,11 @@ func buildTable(t *testing.T, keyValues [][]string) *os.File {
})
for _, kv := range keyValues {
y.AssertTrue(len(kv) == 2)
err := b.Add(y.KeyWithTs([]byte(kv[0]), 10), y.ValueStruct{
b.Add(y.KeyWithTs([]byte(kv[0]), 10), y.ValueStruct{
Value: []byte(kv[1]),
Meta: 'A',
UserMeta: 0,
})
if t != nil {
require.NoError(t, err)
} else {
y.Check(err)
}
}
f.Write(b.Finish())
f.Close()
Expand Down
3 changes: 2 additions & 1 deletion stream_writer.go
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,8 @@ func (w *sortedWriter) Add(key []byte, vs y.ValueStruct) error {
}

w.lastKey = y.SafeCopy(w.lastKey, key)
return w.builder.Add(key, vs)
w.builder.Add(key, vs)
return nil
}

func (w *sortedWriter) send() error {
Expand Down
3 changes: 1 addition & 2 deletions table/builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ func (b *Builder) finishBlock() {

// Add adds a key-value pair to the block.
// If doNotRestart is true, we will not restart even if b.counter >= restartInterval.
func (b *Builder) Add(key []byte, value y.ValueStruct) error {
func (b *Builder) Add(key []byte, value y.ValueStruct) {
if b.counter >= restartInterval {
b.finishBlock()
// Start a new block. Initialize the block.
Expand All @@ -167,7 +167,6 @@ func (b *Builder) Add(key []byte, value y.ValueStruct) error {
b.prevOffset = math.MaxUint32 // First key-value pair of block has header.prev=MaxInt.
}
b.addHelper(key, value)
return nil // Currently, there is no meaningful error.
}

// TODO: vvv this was the comment on ReachedCapacity.
Expand Down
29 changes: 21 additions & 8 deletions table/table_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"math/rand"
"os"
"sort"
"strings"
"testing"
"time"

Expand Down Expand Up @@ -64,12 +65,7 @@ func buildTable(t *testing.T, keyValues [][]string) *os.File {
})
for _, kv := range keyValues {
y.AssertTrue(len(kv) == 2)
err := b.Add(y.KeyWithTs([]byte(kv[0]), 0), y.ValueStruct{Value: []byte(kv[1]), Meta: 'A', UserMeta: 0})
if t != nil {
require.NoError(t, err)
} else {
y.Check(err)
}
b.Add(y.KeyWithTs([]byte(kv[0]), 0), y.ValueStruct{Value: []byte(kv[1]), Meta: 'A', UserMeta: 0})
}
f.Write(b.Finish())
f.Close()
Expand Down Expand Up @@ -624,6 +620,23 @@ func TestMergingIteratorTakeTwo(t *testing.T) {
require.False(t, it.Valid())
}

// This test is for verifying checksum failure during table open.
func TestTableChecksum(t *testing.T) {
rand.Seed(time.Now().Unix())
// we are going to write random byte at random location in table file.
rb := make([]byte, 1)
rand.Read(rb)
f := buildTestTable(t, "k", 10000)
fi, err := f.Stat()
require.NoError(t, err, "unable to get file information")
f.WriteAt(rb, rand.Int63n(fi.Size()))

_, err = OpenTable(f, options.LoadToRAM, []byte("wrong"))
if err == nil || !strings.Contains(err.Error(), "checksum") {
t.Fatal("Test should have been failed with checksum mismatch error")
}
}

func BenchmarkRead(b *testing.B) {
n := int(5 * 1e6)
tbl := getTableForBenchmarks(b, n)
Expand Down Expand Up @@ -678,7 +691,7 @@ func BenchmarkReadMerged(b *testing.B) {
// id := i*tableSize+j (not interleaved)
k := fmt.Sprintf("%016x", id)
v := fmt.Sprintf("%d", id)
y.Check(builder.Add([]byte(k), y.ValueStruct{Value: []byte(v), Meta: 123, UserMeta: 0}))
builder.Add([]byte(k), y.ValueStruct{Value: []byte(v), Meta: 123, UserMeta: 0})
}
f.Write(builder.Finish())
tbl, err := OpenTable(f, options.LoadToRAM, nil)
Expand Down Expand Up @@ -739,7 +752,7 @@ func getTableForBenchmarks(b *testing.B, count int) *Table {
for i := 0; i < count; i++ {
k := fmt.Sprintf("%016x", i)
v := fmt.Sprintf("%d", i)
y.Check(builder.Add([]byte(k), y.ValueStruct{Value: []byte(v)}))
builder.Add([]byte(k), y.ValueStruct{Value: []byte(v)})
}

f.Write(builder.Finish())
Expand Down
4 changes: 2 additions & 2 deletions txn.go
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ func (txn *Txn) modify(e *Entry) error {
if err := txn.checkSize(e); err != nil {
return err
}
fp := z.AESHash(e.Key) // Avoid dealing with byte arrays.
fp := z.MemHash(e.Key) // Avoid dealing with byte arrays.
txn.writes = append(txn.writes, fp)
txn.pendingWrites[string(e.Key)] = e
return nil
Expand Down Expand Up @@ -428,7 +428,7 @@ func (txn *Txn) Get(key []byte) (item *Item, rerr error) {

func (txn *Txn) addReadKey(key []byte) {
if txn.update {
fp := z.AESHash(key)
fp := z.MemHash(key)
txn.reads = append(txn.reads, fp)
}
}
Expand Down
56 changes: 0 additions & 56 deletions value_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1070,59 +1070,3 @@ func TestTruncatedDiscardStat(t *testing.T) {
require.NoError(t, err)
require.NoError(t, db.Close())
}

// Regression test for https://github.com/dgraph-io/badger/issues/926
func TestDiscardStatsMove(t *testing.T) {
dir, err := ioutil.TempDir("", "badger-test")
require.NoError(t, err)
ops := getTestOptions(dir)
ops.ValueLogMaxEntries = 1
db, err := Open(ops)
require.NoError(t, err)

stat := make(map[uint32]int64, ops.ValueThreshold+10)
for i := uint32(0); i < uint32(ops.ValueThreshold+10); i++ {
stat[i] = 0
}

// Set discard stats.
db.vlog.lfDiscardStats = &lfDiscardStats{
m: stat,
}
entries := []*Entry{{
Key: y.KeyWithTs(lfDiscardStatsKey, 1),
// The discard stat value is more than value threshold.
Value: db.vlog.encodedDiscardStats(),
}}
// Push discard stats entry to the write channel.
req, err := db.sendToWriteCh(entries)
require.NoError(t, err)
req.Wait()

// Unset discard stats. We've already pushed the stats. If we don't unset it then it will be
// pushed again on DB close. Also, the first insertion was in vlog file 1, this insertion would
// be in value log file 3.
db.vlog.lfDiscardStats.m = nil

// Push more entries so that we get more than 1 value log files.
require.NoError(t, db.Update(func(txn *Txn) error {
e := NewEntry([]byte("f"), []byte("1"))
return txn.SetEntry(e)
}))
require.NoError(t, db.Update(func(txn *Txn) error {
e := NewEntry([]byte("ff"), []byte("1"))
return txn.SetEntry(e)

}))

tr := trace.New("Badger.ValueLog", "GC")
// Use first value log file for GC. This value log file contains the discard stats.
lf := db.vlog.filesMap[0]
require.NoError(t, db.vlog.rewrite(lf, tr))
require.NoError(t, db.Close())

db, err = Open(ops)
require.NoError(t, err)
require.Equal(t, stat, db.vlog.lfDiscardStats.m)
require.NoError(t, db.Close())
}

0 comments on commit f34085e

Please sign in to comment.