Skip to content

Commit

Permalink
Address golint and go vet complaints
Browse files Browse the repository at this point in the history
Some immaterial code changes:

- Simplified Safecopy

- Replaced SafeMutex with empty warning parameters

- Removed unused functions

- Unexported functions (instead of commenting them)
  • Loading branch information
Sam Hughes committed Sep 8, 2017
1 parent cb5a769 commit e846b94
Show file tree
Hide file tree
Showing 20 changed files with 302 additions and 265 deletions.
5 changes: 3 additions & 2 deletions cmd/badger_info/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
* limitations under the License.
*/

/* badger_info
/*
badger_info
Usage: badger_info --dir x [--value-dir y]
Expand Down Expand Up @@ -118,7 +119,7 @@ func printInfo(dir, valueDir string) error {
return tableIDs[i] < tableIDs[j]
})
for _, tableID := range tableIDs {
tableFile := table.TableFilename(tableID)
tableFile := table.IDToFilename(tableID)
file, ok := fileinfoByName[tableFile]
if ok {
fileinfoMarked[tableFile] = true
Expand Down
7 changes: 3 additions & 4 deletions compaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -130,12 +130,11 @@ func (cs *compactStatus) delSize(l int) int64 {
return cs.levels[l].delSize
}

type thisAndNextLevelRLocked struct{}

// compareAndAdd will check whether we can run this compactDef. That it doesn't overlap with any
// other running compaction. If it can be run, it would store this run in the compactStatus state.
func (cs *compactStatus) compareAndAdd(cd compactDef) bool {
cd.thisLevel.AssertRLock()
cd.nextLevel.AssertRLock()

func (cs *compactStatus) compareAndAdd(_ thisAndNextLevelRLocked, cd compactDef) bool {
cs.Lock()
defer cs.Unlock()

Expand Down
2 changes: 1 addition & 1 deletion kv.go
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ func NewKV(optParam *Options) (out *KV, err error) {
if !(opt.ValueLogFileSize <= 2<<30 && opt.ValueLogFileSize >= 1<<20) {
return nil, ErrValueLogSize
}
manifestFile, manifest, err := OpenOrCreateManifestFile(opt.Dir)
manifestFile, manifest, err := openOrCreateManifestFile(opt.Dir)
if err != nil {
return nil, err
}
Expand Down
15 changes: 8 additions & 7 deletions level_handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"bytes"
"fmt"
"sort"
"sync"

"github.com/dgraph-io/badger/table"
"github.com/dgraph-io/badger/y"
Expand All @@ -28,7 +29,7 @@ import (

type levelHandler struct {
// Guards tables, totalSize.
y.SafeMutex
sync.RWMutex

// For level >= 1, tables are sorted by key ranges, which do not overlap.
// For level 0, tables are sorted by time.
Expand Down Expand Up @@ -122,7 +123,7 @@ func (s *levelHandler) replaceTables(newTables []*table.Table) error {
left: newTables[0].Smallest(),
right: newTables[len(newTables)-1].Biggest(),
}
left, right := s.overlappingTables(kr)
left, right := s.overlappingTables(levelHandlerRLocked{}, kr)

toDecr := make([]*table.Table, right-left)
// Update totalSize and reference counts.
Expand Down Expand Up @@ -273,12 +274,12 @@ func (s *levelHandler) appendIterators(iters []y.Iterator, reversed bool) []y.It
return append(iters, table.NewConcatIterator(s.tables, reversed))
}

// overlappingTables returns the tables that intersect with key range.
// Returns a half-interval.
// This function should already have acquired a read lock.
func (s *levelHandler) overlappingTables(kr keyRange) (int, int) {
s.AssertRLock()
type levelHandlerRLocked struct{}

// overlappingTables returns the tables that intersect with key range. Returns a half-interval.
// This function should already have acquired a read lock, and this is so important the caller must
// pass an empty parameter declaring such.
func (s *levelHandler) overlappingTables(_ levelHandlerRLocked, kr keyRange) (int, int) {
left := sort.Search(len(s.tables), func(i int) bool {
return bytes.Compare(kr.left, s.tables[i].Biggest()) <= 0
})
Expand Down
32 changes: 15 additions & 17 deletions levels.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ func (s *levelsController) compactBuildTables(
cd.elog.LazyPrintf("LOG Compact. Iteration to generate one table took: %v\n", time.Since(timeStart))

fileID := s.reserveFileID()
go func(builder *table.TableBuilder) {
go func(builder *table.Builder) {
defer builder.Close()

fd, err := y.CreateSyncedFile(table.NewFilename(fileID, s.kv.opt.Dir), true)
Expand Down Expand Up @@ -412,7 +412,7 @@ func (s *levelsController) fillTablesL0(cd *compactDef) bool {
cd.thisRange = infRange

kr := getKeyRange(cd.top)
left, right := cd.nextLevel.overlappingTables(kr)
left, right := cd.nextLevel.overlappingTables(levelHandlerRLocked{}, kr)
cd.bot = make([]*table.Table, right-left)
copy(cd.bot, cd.nextLevel.tables[left:right])

Expand All @@ -422,7 +422,7 @@ func (s *levelsController) fillTablesL0(cd *compactDef) bool {
cd.nextRange = getKeyRange(cd.bot)
}

if !s.cstatus.compareAndAdd(*cd) {
if !s.cstatus.compareAndAdd(thisAndNextLevelRLocked{}, *cd) {
return false
}

Expand Down Expand Up @@ -455,15 +455,15 @@ func (s *levelsController) fillTables(cd *compactDef) bool {
continue
}
cd.top = []*table.Table{t}
left, right := cd.nextLevel.overlappingTables(cd.thisRange)
left, right := cd.nextLevel.overlappingTables(levelHandlerRLocked{}, cd.thisRange)

cd.bot = make([]*table.Table, right-left)
copy(cd.bot, cd.nextLevel.tables[left:right])

if len(cd.bot) == 0 {
cd.bot = []*table.Table{}
cd.nextRange = cd.thisRange
if !s.cstatus.compareAndAdd(*cd) {
if !s.cstatus.compareAndAdd(thisAndNextLevelRLocked{}, *cd) {
continue
}
return true
Expand All @@ -474,7 +474,7 @@ func (s *levelsController) fillTables(cd *compactDef) bool {
continue
}

if !s.cstatus.compareAndAdd(*cd) {
if !s.cstatus.compareAndAdd(thisAndNextLevelRLocked{}, *cd) {
continue
}
return true
Expand All @@ -500,15 +500,13 @@ func (s *levelsController) runCompactDef(l int, cd compactDef) (err error) {
tbl := cd.top[0]

// We write to the manifest _before_ we delete files (and after we created files).
changeSet := protos.ManifestChangeSet{
Changes: []*protos.ManifestChange{
// The order matters here -- you can't temporarily have two copies of the same
// table id when reloading the manifest.
makeTableDeleteChange(tbl.ID()),
makeTableCreateChange(tbl.ID(), nextLevel.level),
},
changes := []*protos.ManifestChange{
// The order matters here -- you can't temporarily have two copies of the same
// table id when reloading the manifest.
makeTableDeleteChange(tbl.ID()),
makeTableCreateChange(tbl.ID(), nextLevel.level),
}
if err := s.kv.manifest.addChanges(changeSet); err != nil {
if err := s.kv.manifest.addChanges(changes); err != nil {
return err
}

Expand Down Expand Up @@ -545,7 +543,7 @@ func (s *levelsController) runCompactDef(l int, cd compactDef) (err error) {
changeSet := buildChangeSet(&cd, newTables)

// We write to the manifest _before_ we delete files (and after we created files)
if err := s.kv.manifest.addChanges(changeSet); err != nil {
if err := s.kv.manifest.addChanges(changeSet.Changes); err != nil {
return err
}

Expand Down Expand Up @@ -616,9 +614,9 @@ func (s *levelsController) addLevel0Table(t *table.Table) error {
// point it could get used in some compaction. This ensures the manifest file gets updated in
// the proper order. (That means this update happens before that of some compaction which
// deletes the table.)
err := s.kv.manifest.addChanges(protos.ManifestChangeSet{Changes: []*protos.ManifestChange{
err := s.kv.manifest.addChanges([]*protos.ManifestChange{
makeTableCreateChange(t.ID(), 0),
}})
})
if err != nil {
return err
}
Expand Down
9 changes: 5 additions & 4 deletions manifest.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,15 +104,15 @@ func (m *Manifest) asChanges() []*protos.ManifestChange {
}

func (m *Manifest) clone() Manifest {
changeSet := protos.ManifestChangeSet{m.asChanges()}
changeSet := protos.ManifestChangeSet{Changes: m.asChanges()}
ret := createManifest()
y.Check(applyChangeSet(&ret, &changeSet))
return ret
}

// OpenOrCreateManifestFile opens a Badger manifest file if it exists, or creates on if
// openOrCreateManifestFile opens a Badger manifest file if it exists, or creates on if
// one doesn’t.
func OpenOrCreateManifestFile(dir string) (ret *manifestFile, result Manifest, err error) {
func openOrCreateManifestFile(dir string) (ret *manifestFile, result Manifest, err error) {
return helpOpenOrCreateManifestFile(dir, manifestDeletionsRewriteThreshold)
}

Expand Down Expand Up @@ -172,7 +172,8 @@ func (mf *manifestFile) close() error {
// we replay the MANIFEST file, we'll either replay all the changes or none of them. (The truth of
// this depends on the filesystem -- some might append garbage data if a system crash happens at
// the wrong time.)
func (mf *manifestFile) addChanges(changes protos.ManifestChangeSet) error {
func (mf *manifestFile) addChanges(changesParam []*protos.ManifestChange) error {
changes := protos.ManifestChangeSet{Changes: changesParam}
buf, err := changes.Marshal()
if err != nil {
return err
Expand Down
13 changes: 9 additions & 4 deletions manifest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,12 @@ func buildTable(t *testing.T, keyValues [][]string) *os.File {
})
for i, kv := range keyValues {
y.AssertTrue(len(kv) == 2)
err := b.Add([]byte(kv[0]), y.ValueStruct{[]byte(kv[1]), 'A', 0, uint64(i)})
err := b.Add([]byte(kv[0]), y.ValueStruct{
Value: []byte(kv[1]),
Meta: 'A',
UserMeta: 0,
CASCounter: uint64(i),
})
if t != nil {
require.NoError(t, err)
} else {
Expand Down Expand Up @@ -215,17 +220,17 @@ func TestManifestRewrite(t *testing.T) {
require.Equal(t, 0, m.Creations)
require.Equal(t, 0, m.Deletions)

err = mf.addChanges(protos.ManifestChangeSet{[]*protos.ManifestChange{
err = mf.addChanges([]*protos.ManifestChange{
makeTableCreateChange(0, 0),
}})
})
require.NoError(t, err)

for i := uint64(0); i < uint64(deletionsThreshold*3); i++ {
ch := []*protos.ManifestChange{
makeTableCreateChange(i+1, 0),
makeTableDeleteChange(i),
}
err := mf.addChanges(protos.ManifestChangeSet{ch})
err := mf.addChanges(ch)
require.NoError(t, err)
}
err = mf.close()
Expand Down
20 changes: 10 additions & 10 deletions skl/arena.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,27 +28,27 @@ type Arena struct {
buf []byte
}

// NewArena returns a new arena.
func NewArena(n int64) *Arena {
// newArena returns a new arena.
func newArena(n int64) *Arena {
out := &Arena{
buf: make([]byte, n),
}
return out
}

func (s *Arena) Size() int64 {
func (s *Arena) size() int64 {
return int64(atomic.LoadUint32(&s.n))
}

func (s *Arena) Reset() {
func (s *Arena) reset() {
atomic.StoreUint32(&s.n, 0)
}

// Put will *copy* val into arena. To make better use of this, reuse your input
// val buffer. Returns an offset into buf. User is responsible for remembering
// size of val. We could also store this size inside arena but the encoding and
// decoding will incur some overhead.
func (s *Arena) PutVal(v y.ValueStruct) uint32 {
func (s *Arena) putVal(v y.ValueStruct) uint32 {
l := uint32(v.EncodedSize())
n := atomic.AddUint32(&s.n, l)
y.AssertTruef(int(n) <= len(s.buf),
Expand All @@ -59,7 +59,7 @@ func (s *Arena) PutVal(v y.ValueStruct) uint32 {
return m
}

func (s *Arena) PutKey(key []byte) uint32 {
func (s *Arena) putKey(key []byte) uint32 {
l := uint32(len(key))
n := atomic.AddUint32(&s.n, l)
y.AssertTruef(int(n) <= len(s.buf),
Expand All @@ -70,14 +70,14 @@ func (s *Arena) PutKey(key []byte) uint32 {
return m
}

// GetKey returns byte slice at offset.
func (s *Arena) GetKey(offset uint32, size uint16) []byte {
// getKey returns byte slice at offset.
func (s *Arena) getKey(offset uint32, size uint16) []byte {
return s.buf[offset : offset+uint32(size)]
}

// GetVal returns byte slice at offset. The given size should be just the value
// getVal returns byte slice at offset. The given size should be just the value
// size and should NOT include the meta bytes.
func (s *Arena) GetVal(offset uint32, size uint16) (ret y.ValueStruct) {
func (s *Arena) getVal(offset uint32, size uint16) (ret y.ValueStruct) {
ret.DecodeEntireSlice(s.buf[offset : offset+uint32(y.ValueStructSerializedSize(size))])
return
}
Loading

0 comments on commit e846b94

Please sign in to comment.