From d6e03d2368e9d1fa11ed19f9a4d54e4c578d727c Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Fri, 22 Sep 2017 11:42:29 -0400 Subject: [PATCH 01/26] linting: add to Makefile & do some fixes --- Makefile | 43 ++++++++++++++++++++++++++++++++++++- autofile/autofile_test.go | 4 +++- autofile/group.go | 4 +--- autofile/group_test.go | 3 +-- autofile/sighup_watcher.go | 2 +- cli/flags/log_level_test.go | 2 -- clist/clist_test.go | 6 ++++-- common/cmap.go | 4 ++-- common/errors.go | 2 +- common/http_test.go | 2 +- common/os.go | 10 +++------ common/string.go | 5 +---- events/events_test.go | 15 ++++++------- flowrate/io_test.go | 5 +---- log/filter_test.go | 6 ++---- log/tmfmt_logger.go | 7 +++--- log/tracing_logger_test.go | 3 +-- merkle/simple_tree.go | 3 ++- process/util.go | 4 ++-- pubsub/query/query.peg.go | 3 +++ 20 files changed, 82 insertions(+), 51 deletions(-) diff --git a/Makefile b/Makefile index 8e43dd11a..ba164ec1d 100644 --- a/Makefile +++ b/Makefile @@ -1,11 +1,15 @@ .PHONY: all test get_vendor_deps ensure_tools GOTOOLS = \ - github.com/Masterminds/glide + github.com/Masterminds/glide \ + github.com/alecthomas/gometalinter + REPO:=github.com/tendermint/tmlibs all: test +NOVENDOR = go list github.com/tendermint/tmlibs/... | grep -v /vendor/ + test: go test `glide novendor` @@ -16,3 +20,40 @@ get_vendor_deps: ensure_tools ensure_tools: go get $(GOTOOLS) + +metalinter: ensure_tools + @gometalinter --install + gometalinter --vendor --deadline=600s --enable-all --disable=lll ./... + +metalinter_test: ensure_tools + @gometalinter --install + gometalinter --vendor --deadline=600s --disable-all \ + --enable=deadcode \ + --enable=gas \ + --enable=goconst \ + --enable=goimports \ + --enable=gosimple \ + --enable=gotype \ + --enable=ineffassign \ + --enable=megacheck \ + --enable=misspell \ + --enable=staticcheck \ + --enable=safesql \ + --enable=structcheck \ + --enable=unconvert \ + --enable=unused \ + --enable=varcheck \ + --enable=vetshadow \ + --enable=interfacer \ + --enable=unparam \ + --enable=vet \ + ./... + + #--enable=aligncheck \ + #--enable=dupl \ + #--enable=errcheck \ + #--enable=gocyclo \ + #--enable=golint \ <== comments on anything exported + #--enable=interfacer \ + #--enable=unparam \ + #--enable=vet \ diff --git a/autofile/autofile_test.go b/autofile/autofile_test.go index 8f8017e1b..c7aa93beb 100644 --- a/autofile/autofile_test.go +++ b/autofile/autofile_test.go @@ -1,12 +1,14 @@ +// nolint: goimports package autofile import ( - . "github.com/tendermint/tmlibs/common" "os" "sync/atomic" "syscall" "testing" "time" + + . "github.com/tendermint/tmlibs/common" ) func TestSIGHUP(t *testing.T) { diff --git a/autofile/group.go b/autofile/group.go index ce3e30009..689b1cb92 100644 --- a/autofile/group.go +++ b/autofile/group.go @@ -107,7 +107,6 @@ func (g *Group) OnStart() error { func (g *Group) OnStop() { g.BaseService.OnStop() g.ticker.Stop() - return } func (g *Group) SetHeadSizeLimit(limit int64) { @@ -568,8 +567,7 @@ func (gr *GroupReader) ReadLine() (string, error) { bytesRead, err := gr.curReader.ReadBytes('\n') if err == io.EOF { // Open the next file - err := gr.openFile(gr.curIndex + 1) - if err != nil { + if err := gr.openFile(gr.curIndex + 1); err != nil { return "", err } if len(bytesRead) > 0 && bytesRead[len(bytesRead)-1] == byte('\n') { diff --git a/autofile/group_test.go b/autofile/group_test.go index 92e259701..91c6a0bbd 100644 --- a/autofile/group_test.go +++ b/autofile/group_test.go @@ -100,8 +100,7 @@ func TestCheckHeadSizeLimit(t *testing.T) { // Write 1000 bytes 999 times. for i := 0; i < 999; i++ { - err := g.WriteLine(RandStr(999)) - if err != nil { + if err := g.WriteLine(RandStr(999)); err != nil { t.Fatal("Error appending to head", err) } } diff --git a/autofile/sighup_watcher.go b/autofile/sighup_watcher.go index facc238d5..56fbd4d86 100644 --- a/autofile/sighup_watcher.go +++ b/autofile/sighup_watcher.go @@ -22,7 +22,7 @@ func initSighupWatcher() { signal.Notify(c, syscall.SIGHUP) go func() { - for _ = range c { + for range c { sighupWatchers.closeAll() atomic.AddInt32(&sighupCounter, 1) } diff --git a/cli/flags/log_level_test.go b/cli/flags/log_level_test.go index 458a9e24d..faf9b19db 100644 --- a/cli/flags/log_level_test.go +++ b/cli/flags/log_level_test.go @@ -49,8 +49,6 @@ func TestParseLogLevel(t *testing.T) { t.Fatal(err) } - logger = logger - buf.Reset() logger.With("module", "wire").Debug("Kingpin") diff --git a/clist/clist_test.go b/clist/clist_test.go index ab5cf4b26..2063cf465 100644 --- a/clist/clist_test.go +++ b/clist/clist_test.go @@ -55,6 +55,7 @@ func TestSmall(t *testing.T) { This test is quite hacky because it relies on SetFinalizer which isn't guaranteed to run at all. */ +// nolint: megacheck func _TestGCFifo(t *testing.T) { const numElements = 1000000 @@ -102,6 +103,7 @@ func _TestGCFifo(t *testing.T) { This test is quite hacky because it relies on SetFinalizer which isn't guaranteed to run at all. */ +// nolint: megacheck func _TestGCRandom(t *testing.T) { const numElements = 1000000 @@ -132,7 +134,7 @@ func _TestGCRandom(t *testing.T) { for _, i := range rand.Perm(numElements) { el := els[i] l.Remove(el) - el = el.Next() + _ = el.Next() } runtime.GC() @@ -153,7 +155,7 @@ func TestScanRightDeleteRandom(t *testing.T) { l := New() stop := make(chan struct{}) - els := make([]*CElement, numElements, numElements) + els := make([]*CElement, numElements) for i := 0; i < numElements; i++ { el := l.PushBack(i) els[i] = el diff --git a/common/cmap.go b/common/cmap.go index 5de6fa2fa..e2a140dd0 100644 --- a/common/cmap.go +++ b/common/cmap.go @@ -10,7 +10,7 @@ type CMap struct { func NewCMap() *CMap { return &CMap{ - m: make(map[string]interface{}, 0), + m: make(map[string]interface{}), } } @@ -48,7 +48,7 @@ func (cm *CMap) Size() int { func (cm *CMap) Clear() { cm.l.Lock() defer cm.l.Unlock() - cm.m = make(map[string]interface{}, 0) + cm.m = make(map[string]interface{}) } func (cm *CMap) Values() []interface{} { diff --git a/common/errors.go b/common/errors.go index 3a1b09542..039342a67 100644 --- a/common/errors.go +++ b/common/errors.go @@ -21,7 +21,7 @@ func (se StackError) Error() string { // panic wrappers // A panic resulting from a sanity check means there is a programmer error -// and some gaurantee is not satisfied. +// and some guarantee is not satisfied. func PanicSanity(v interface{}) { panic(Fmt("Panicked on a Sanity Check: %v", v)) } diff --git a/common/http_test.go b/common/http_test.go index 73761fb1e..4272f6062 100644 --- a/common/http_test.go +++ b/common/http_test.go @@ -95,7 +95,7 @@ func TestWriteCode(t *testing.T) { common.WriteCode(w, &marshalFailer{}, code) wantCode := http.StatusBadRequest assert.Equal(t, w.Code, wantCode, "#%d", i) - assert.True(t, strings.Contains(string(w.Body.Bytes()), errFooFailed.Error()), + assert.True(t, strings.Contains(w.Body.String(), errFooFailed.Error()), "#%d: expected %q in the error message", i, errFooFailed) } } diff --git a/common/os.go b/common/os.go index 9c2bda508..8b7143f5a 100644 --- a/common/os.go +++ b/common/os.go @@ -8,6 +8,7 @@ import ( "os" "os/signal" "strings" + "syscall" ) var ( @@ -17,7 +18,7 @@ var ( func TrapSignal(cb func()) { c := make(chan os.Signal, 1) signal.Notify(c, os.Interrupt) - signal.Notify(c, os.Kill) + signal.Notify(c, syscall.SIGTERM) go func() { for sig := range c { fmt.Printf("captured %v, exiting...\n", sig) @@ -83,12 +84,7 @@ func MustReadFile(filePath string) []byte { } func WriteFile(filePath string, contents []byte, mode os.FileMode) error { - err := ioutil.WriteFile(filePath, contents, mode) - if err != nil { - return err - } - // fmt.Printf("File written to %v.\n", filePath) - return nil + return ioutil.WriteFile(filePath, contents, mode) } func MustWriteFile(filePath string, contents []byte, mode os.FileMode) { diff --git a/common/string.go b/common/string.go index 2818f5ed5..1ab91f15a 100644 --- a/common/string.go +++ b/common/string.go @@ -31,10 +31,7 @@ func LeftPadString(s string, totalLength int) string { func IsHex(s string) bool { if len(s) > 2 && s[:2] == "0x" { _, err := hex.DecodeString(s[2:]) - if err != nil { - return false - } - return true + return err == nil } return false } diff --git a/events/events_test.go b/events/events_test.go index c1b48b16f..dee50e5bd 100644 --- a/events/events_test.go +++ b/events/events_test.go @@ -14,7 +14,7 @@ import ( func TestAddListenerForEventFireOnce(t *testing.T) { evsw := NewEventSwitch() started, err := evsw.Start() - if started == false || err != nil { + if !started || err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } messages := make(chan EventData) @@ -34,7 +34,7 @@ func TestAddListenerForEventFireOnce(t *testing.T) { func TestAddListenerForEventFireMany(t *testing.T) { evsw := NewEventSwitch() started, err := evsw.Start() - if started == false || err != nil { + if !started || err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum := make(chan uint64) @@ -63,7 +63,7 @@ func TestAddListenerForEventFireMany(t *testing.T) { func TestAddListenerForDifferentEvents(t *testing.T) { evsw := NewEventSwitch() started, err := evsw.Start() - if started == false || err != nil { + if !started || err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum := make(chan uint64) @@ -108,7 +108,7 @@ func TestAddListenerForDifferentEvents(t *testing.T) { func TestAddDifferentListenerForDifferentEvents(t *testing.T) { evsw := NewEventSwitch() started, err := evsw.Start() - if started == false || err != nil { + if !started || err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum1 := make(chan uint64) @@ -168,7 +168,7 @@ func TestAddDifferentListenerForDifferentEvents(t *testing.T) { func TestAddAndRemoveListener(t *testing.T) { evsw := NewEventSwitch() started, err := evsw.Start() - if started == false || err != nil { + if !started || err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum1 := make(chan uint64) @@ -213,7 +213,7 @@ func TestAddAndRemoveListener(t *testing.T) { func TestRemoveListener(t *testing.T) { evsw := NewEventSwitch() started, err := evsw.Start() - if started == false || err != nil { + if !started || err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } count := 10 @@ -266,7 +266,7 @@ func TestRemoveListener(t *testing.T) { func TestRemoveListenersAsync(t *testing.T) { evsw := NewEventSwitch() started, err := evsw.Start() - if started == false || err != nil { + if !started || err != nil { t.Errorf("Failed to start EventSwitch, error: %v", err) } doneSum1 := make(chan uint64) @@ -377,5 +377,4 @@ func fireEvents(evsw EventSwitch, event string, doneChan chan uint64, } doneChan <- sentSum close(doneChan) - return } diff --git a/flowrate/io_test.go b/flowrate/io_test.go index 6d4934a8a..db40337c9 100644 --- a/flowrate/io_test.go +++ b/flowrate/io_test.go @@ -171,10 +171,7 @@ func statusesAreEqual(s1 *Status, s2 *Status) bool { } func durationsAreEqual(d1 time.Duration, d2 time.Duration, maxDeviation time.Duration) bool { - if d2-d1 <= maxDeviation { - return true - } - return false + return d2-d1 <= maxDeviation } func ratesAreEqual(r1 int64, r2 int64, maxDeviation int64) bool { diff --git a/log/filter_test.go b/log/filter_test.go index fafafacb0..8d8b3b27c 100644 --- a/log/filter_test.go +++ b/log/filter_test.go @@ -73,8 +73,7 @@ func TestVariousLevels(t *testing.T) { func TestLevelContext(t *testing.T) { var buf bytes.Buffer - var logger log.Logger - logger = log.NewTMJSONLogger(&buf) + logger := log.NewTMJSONLogger(&buf) logger = log.NewFilter(logger, log.AllowError()) logger = logger.With("context", "value") @@ -93,8 +92,7 @@ func TestLevelContext(t *testing.T) { func TestVariousAllowWith(t *testing.T) { var buf bytes.Buffer - var logger log.Logger - logger = log.NewTMJSONLogger(&buf) + logger := log.NewTMJSONLogger(&buf) logger1 := log.NewFilter(logger, log.AllowError(), log.AllowInfoWith("context", "value")) logger1.With("context", "value").Info("foo", "bar", "baz") diff --git a/log/tmfmt_logger.go b/log/tmfmt_logger.go index 14028d756..2b464a6b0 100644 --- a/log/tmfmt_logger.go +++ b/log/tmfmt_logger.go @@ -49,9 +49,10 @@ func (l tmfmtLogger) Log(keyvals ...interface{}) error { enc.Reset() defer tmfmtEncoderPool.Put(enc) + const unknown = "unknown" lvl := "none" - msg := "unknown" - module := "unknown" + msg := unknown + module := unknown // indexes of keys to skip while encoding later excludeIndexes := make([]int, 0) @@ -90,7 +91,7 @@ func (l tmfmtLogger) Log(keyvals ...interface{}) error { // Stopping ... - message enc.buf.WriteString(fmt.Sprintf("%c[%s] %-44s ", lvl[0]-32, time.Now().UTC().Format("01-02|15:04:05.000"), msg)) - if module != "unknown" { + if module != unknown { enc.buf.WriteString("module=" + module + " ") } diff --git a/log/tracing_logger_test.go b/log/tracing_logger_test.go index 584b34bef..6b0838ca8 100644 --- a/log/tracing_logger_test.go +++ b/log/tracing_logger_test.go @@ -14,8 +14,7 @@ import ( func TestTracingLogger(t *testing.T) { var buf bytes.Buffer - var logger log.Logger - logger = log.NewTMJSONLogger(&buf) + logger := log.NewTMJSONLogger(&buf) logger1 := log.NewTracingLogger(logger) err1 := errors.New("Courage is grace under pressure.") diff --git a/merkle/simple_tree.go b/merkle/simple_tree.go index b5520f723..b373743fc 100644 --- a/merkle/simple_tree.go +++ b/merkle/simple_tree.go @@ -22,6 +22,7 @@ For larger datasets, use IAVLTree. */ +// nolint: goimports package merkle import ( @@ -31,8 +32,8 @@ import ( "golang.org/x/crypto/ripemd160" - . "github.com/tendermint/tmlibs/common" "github.com/tendermint/go-wire" + . "github.com/tendermint/tmlibs/common" ) func SimpleHashFromTwoHashes(left []byte, right []byte) []byte { diff --git a/process/util.go b/process/util.go index b3e0aef11..24cf35280 100644 --- a/process/util.go +++ b/process/util.go @@ -15,8 +15,8 @@ func Run(dir string, command string, args []string) (string, bool, error) { <-proc.WaitCh if proc.ExitState.Success() { - return string(outFile.Bytes()), true, nil + return outFile.String(), true, nil } else { - return string(outFile.Bytes()), false, nil + return outFile.String(), false, nil } } diff --git a/pubsub/query/query.peg.go b/pubsub/query/query.peg.go index 37ce75cd9..8c3e83efc 100644 --- a/pubsub/query/query.peg.go +++ b/pubsub/query/query.peg.go @@ -1,3 +1,6 @@ +// nolint: megacheck +// nolint: varcheck +// nolint: deadcode package query import ( From 3c57c24921f8197343cf6e592de31b5fd31509cc Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Fri, 22 Sep 2017 12:14:27 -0400 Subject: [PATCH 02/26] linting: next round of fixes --- Makefile | 1 - autofile/group.go | 4 ++-- autofile/group_test.go | 8 +++----- common/service.go | 2 +- db/c_level_db_test.go | 2 +- db/go_level_db_test.go | 2 +- pubsub/query/query.peg.go | 4 +--- 7 files changed, 9 insertions(+), 14 deletions(-) diff --git a/Makefile b/Makefile index ba164ec1d..6b2c7463a 100644 --- a/Makefile +++ b/Makefile @@ -45,7 +45,6 @@ metalinter_test: ensure_tools --enable=varcheck \ --enable=vetshadow \ --enable=interfacer \ - --enable=unparam \ --enable=vet \ ./... diff --git a/autofile/group.go b/autofile/group.go index 689b1cb92..eedb67b50 100644 --- a/autofile/group.go +++ b/autofile/group.go @@ -567,8 +567,8 @@ func (gr *GroupReader) ReadLine() (string, error) { bytesRead, err := gr.curReader.ReadBytes('\n') if err == io.EOF { // Open the next file - if err := gr.openFile(gr.curIndex + 1); err != nil { - return "", err + if err1 := gr.openFile(gr.curIndex + 1); err1 != nil { + return "", err1 } if len(bytesRead) > 0 && bytesRead[len(bytesRead)-1] == byte('\n') { return linePrefix + string(bytesRead[:len(bytesRead)-1]), nil diff --git a/autofile/group_test.go b/autofile/group_test.go index 91c6a0bbd..0cfcef72f 100644 --- a/autofile/group_test.go +++ b/autofile/group_test.go @@ -77,8 +77,7 @@ func TestCheckHeadSizeLimit(t *testing.T) { assertGroupInfo(t, g.ReadGroupInfo(), 0, 0, 999000, 999000) // Write 1000 more bytes. - err := g.WriteLine(RandStr(999)) - if err != nil { + if err := g.WriteLine(RandStr(999)); err != nil { t.Fatal("Error appending to head", err) } g.Flush() @@ -88,8 +87,7 @@ func TestCheckHeadSizeLimit(t *testing.T) { assertGroupInfo(t, g.ReadGroupInfo(), 0, 1, 1000000, 0) // Write 1000 more bytes. - err = g.WriteLine(RandStr(999)) - if err != nil { + if err := g.WriteLine(RandStr(999)); err != nil { t.Fatal("Error appending to head", err) } g.Flush() @@ -112,7 +110,7 @@ func TestCheckHeadSizeLimit(t *testing.T) { assertGroupInfo(t, g.ReadGroupInfo(), 0, 2, 2000000, 0) // Write 1000 more bytes. - _, err = g.Head.Write([]byte(RandStr(999) + "\n")) + _, err := g.Head.Write([]byte(RandStr(999) + "\n")) if err != nil { t.Fatal("Error appending to head", err) } diff --git a/common/service.go b/common/service.go index 71fc03cb9..5ac386316 100644 --- a/common/service.go +++ b/common/service.go @@ -151,7 +151,7 @@ func (bs *BaseService) Reset() (bool, error) { return false, nil } // never happens - return false, nil + return false, nil // nolint: vet } // Implements Service diff --git a/db/c_level_db_test.go b/db/c_level_db_test.go index 0ee6d6414..e7336cc5f 100644 --- a/db/c_level_db_test.go +++ b/db/c_level_db_test.go @@ -50,7 +50,7 @@ func BenchmarkRandomReadsWrites2(b *testing.B) { //fmt.Printf("Get %X -> %X\n", idxBytes, valBytes) if val == 0 { if !bytes.Equal(valBytes, nil) { - b.Errorf("Expected %X for %v, got %X", + b.Errorf("Expected %v for %v, got %X", nil, idx, valBytes) break } diff --git a/db/go_level_db_test.go b/db/go_level_db_test.go index 0603b2d4f..2cd3192c3 100644 --- a/db/go_level_db_test.go +++ b/db/go_level_db_test.go @@ -49,7 +49,7 @@ func BenchmarkRandomReadsWrites(b *testing.B) { //fmt.Printf("Get %X -> %X\n", idxBytes, valBytes) if val == 0 { if !bytes.Equal(valBytes, nil) { - b.Errorf("Expected %X for %v, got %X", + b.Errorf("Expected %v for %v, got %X", nil, idx, valBytes) break } diff --git a/pubsub/query/query.peg.go b/pubsub/query/query.peg.go index 8c3e83efc..c86e4a47f 100644 --- a/pubsub/query/query.peg.go +++ b/pubsub/query/query.peg.go @@ -1,6 +1,4 @@ -// nolint: megacheck -// nolint: varcheck -// nolint: deadcode +// nolint package query import ( From 2681f32bddcc061e8ae062734404e73cbe6ea5d0 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Fri, 22 Sep 2017 12:35:52 -0400 Subject: [PATCH 03/26] circle: add metalinter to test --- Makefile | 4 +--- circle.yml | 2 +- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/Makefile b/Makefile index 6b2c7463a..902197e77 100644 --- a/Makefile +++ b/Makefile @@ -35,6 +35,7 @@ metalinter_test: ensure_tools --enable=gosimple \ --enable=gotype \ --enable=ineffassign \ + --enable=interfacer \ --enable=megacheck \ --enable=misspell \ --enable=staticcheck \ @@ -44,7 +45,6 @@ metalinter_test: ensure_tools --enable=unused \ --enable=varcheck \ --enable=vetshadow \ - --enable=interfacer \ --enable=vet \ ./... @@ -53,6 +53,4 @@ metalinter_test: ensure_tools #--enable=errcheck \ #--enable=gocyclo \ #--enable=golint \ <== comments on anything exported - #--enable=interfacer \ #--enable=unparam \ - #--enable=vet \ diff --git a/circle.yml b/circle.yml index 23ac4bd9f..8e3ad168b 100644 --- a/circle.yml +++ b/circle.yml @@ -18,4 +18,4 @@ dependencies: test: override: - "go version" - - "cd $PROJECT_PATH && make get_vendor_deps && make test" + - "cd $PROJECT_PATH && make get_vendor_deps && make metalinter_test && make test" From 296d2235455dc62627d9c3feaf31306497fd4aae Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Thu, 28 Sep 2017 17:26:24 +0200 Subject: [PATCH 04/26] Add Release & Error methods to Iterator We need this to properly support LevelDB backend, which needs to be released if we don't want to leak memory. --- db/db.go | 5 ++++- db/mem_db.go | 10 ++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/db/db.go b/db/db.go index aa8ff48a8..51283ac2a 100644 --- a/db/db.go +++ b/db/db.go @@ -10,10 +10,10 @@ type DB interface { DeleteSync([]byte) Close() NewBatch() Batch + Iterator() Iterator // For debugging Print() - Iterator() Iterator Stats() map[string]string } @@ -28,6 +28,9 @@ type Iterator interface { Key() []byte Value() []byte + + Release() + Error() error } //----------------------------------------------------------------------------- diff --git a/db/mem_db.go b/db/mem_db.go index db40227e8..04d48cac0 100644 --- a/db/mem_db.go +++ b/db/mem_db.go @@ -97,6 +97,16 @@ func (it *memDBIterator) Value() []byte { return it.db.Get(it.Key()) } +func (it *memDBIterator) Release() { + it.db = nil + it.keys = nil + return +} + +func (it *memDBIterator) Error() error { + return nil +} + func (db *MemDB) Iterator() Iterator { it := newMemDBIterator() it.db = db From 219d4e8427b2ddb0e9146ca7319bce2e1f22a6ce Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Thu, 28 Sep 2017 17:35:01 +0200 Subject: [PATCH 05/26] Add IteratorPrefix method to Iterator --- db/db.go | 1 + db/go_level_db.go | 5 +++++ db/mem_db.go | 9 ++++++++- 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/db/db.go b/db/db.go index 51283ac2a..8156c1e92 100644 --- a/db/db.go +++ b/db/db.go @@ -11,6 +11,7 @@ type DB interface { Close() NewBatch() Batch Iterator() Iterator + IteratorPrefix([]byte) Iterator // For debugging Print() diff --git a/db/go_level_db.go b/db/go_level_db.go index 54ae1149f..31c85ce86 100644 --- a/db/go_level_db.go +++ b/db/go_level_db.go @@ -7,6 +7,7 @@ import ( "github.com/syndtr/goleveldb/leveldb" "github.com/syndtr/goleveldb/leveldb/errors" "github.com/syndtr/goleveldb/leveldb/opt" + "github.com/syndtr/goleveldb/leveldb/util" . "github.com/tendermint/tmlibs/common" ) @@ -119,6 +120,10 @@ func (db *GoLevelDB) Iterator() Iterator { return db.db.NewIterator(nil, nil) } +func (db *GoLevelDB) IteratorPrefix(prefix []byte) Iterator { + return db.db.NewIterator(util.BytesPrefix(prefix), nil) +} + func (db *GoLevelDB) NewBatch() Batch { batch := new(leveldb.Batch) return &goLevelDBBatch{db, batch} diff --git a/db/mem_db.go b/db/mem_db.go index 04d48cac0..561633291 100644 --- a/db/mem_db.go +++ b/db/mem_db.go @@ -2,6 +2,7 @@ package db import ( "fmt" + "strings" "sync" ) @@ -108,6 +109,10 @@ func (it *memDBIterator) Error() error { } func (db *MemDB) Iterator() Iterator { + return db.IteratorPrefix([]byte{}) +} + +func (db *MemDB) IteratorPrefix(prefix []byte) Iterator { it := newMemDBIterator() it.db = db it.last = -1 @@ -117,7 +122,9 @@ func (db *MemDB) Iterator() Iterator { // unfortunately we need a copy of all of the keys for key, _ := range db.db { - it.keys = append(it.keys, key) + if strings.HasPrefix(key, string(prefix)) { + it.keys = append(it.keys, key) + } } return it } From e9e6ec3a2c22b3139c5787dd3e6c8a61960ec522 Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Thu, 28 Sep 2017 17:43:47 +0200 Subject: [PATCH 06/26] Implement Key/Value on Iterator for GoLevelDB This is needed because leveldb reuses the keys on each iteration. In our wrapper, we copy the key/value so that it is safe to store. --- db/go_level_db.go | 25 +++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/db/go_level_db.go b/db/go_level_db.go index 31c85ce86..2d2ba44e3 100644 --- a/db/go_level_db.go +++ b/db/go_level_db.go @@ -6,6 +6,7 @@ import ( "github.com/syndtr/goleveldb/leveldb" "github.com/syndtr/goleveldb/leveldb/errors" + "github.com/syndtr/goleveldb/leveldb/iterator" "github.com/syndtr/goleveldb/leveldb/opt" "github.com/syndtr/goleveldb/leveldb/util" @@ -116,12 +117,32 @@ func (db *GoLevelDB) Stats() map[string]string { return stats } +type goLevelDBIterator struct { + iterator.Iterator +} + +func (it *goLevelDBIterator) Key() []byte { + key := it.Key() + k := make([]byte, len(key)) + copy(k, key) + + return k +} + +func (it *goLevelDBIterator) Value() []byte { + val := it.Value() + v := make([]byte, len(val)) + copy(v, val) + + return v +} + func (db *GoLevelDB) Iterator() Iterator { - return db.db.NewIterator(nil, nil) + return &goLevelDBIterator{db.db.NewIterator(nil, nil)} } func (db *GoLevelDB) IteratorPrefix(prefix []byte) Iterator { - return db.db.NewIterator(util.BytesPrefix(prefix), nil) + return &goLevelDBIterator{db.db.NewIterator(util.BytesPrefix(prefix), nil)} } func (db *GoLevelDB) NewBatch() Batch { From cf49ba876fe8e7734acccd4c29289b77ea5829a5 Mon Sep 17 00:00:00 2001 From: Zach Ramsay Date: Tue, 3 Oct 2017 12:18:21 -0400 Subject: [PATCH 07/26] linter: couple fixes --- Makefile | 4 ++-- autofile/autofile_test.go | 21 ++++++++------------- common/os.go | 3 +-- common/service.go | 16 +++++++--------- merkle/simple_tree.go | 1 - 5 files changed, 18 insertions(+), 27 deletions(-) diff --git a/Makefile b/Makefile index 902197e77..25773ed36 100644 --- a/Makefile +++ b/Makefile @@ -31,9 +31,7 @@ metalinter_test: ensure_tools --enable=deadcode \ --enable=gas \ --enable=goconst \ - --enable=goimports \ --enable=gosimple \ - --enable=gotype \ --enable=ineffassign \ --enable=interfacer \ --enable=megacheck \ @@ -52,5 +50,7 @@ metalinter_test: ensure_tools #--enable=dupl \ #--enable=errcheck \ #--enable=gocyclo \ + #--enable=goimports \ #--enable=golint \ <== comments on anything exported + #--enable=gotype \ #--enable=unparam \ diff --git a/autofile/autofile_test.go b/autofile/autofile_test.go index c7aa93beb..05152219c 100644 --- a/autofile/autofile_test.go +++ b/autofile/autofile_test.go @@ -1,4 +1,3 @@ -// nolint: goimports package autofile import ( @@ -8,19 +7,18 @@ import ( "testing" "time" - . "github.com/tendermint/tmlibs/common" + cmn "github.com/tendermint/tmlibs/common" ) func TestSIGHUP(t *testing.T) { // First, create an AutoFile writing to a tempfile dir - file, name := Tempfile("sighup_test") - err := file.Close() - if err != nil { + file, name := cmn.Tempfile("sighup_test") + if err := file.Close(); err != nil { t.Fatalf("Error creating tempfile: %v", err) } // Here is the actual AutoFile - af, err := OpenAutoFile(name) + af, err := cmn.OpenAutoFile(name) if err != nil { t.Fatalf("Error creating autofile: %v", err) } @@ -36,8 +34,7 @@ func TestSIGHUP(t *testing.T) { } // Move the file over - err = os.Rename(name, name+"_old") - if err != nil { + if err := os.Rename(name, name+"_old"); err != nil { t.Fatalf("Error moving autofile: %v", err) } @@ -59,17 +56,15 @@ func TestSIGHUP(t *testing.T) { if err != nil { t.Fatalf("Error writing to autofile: %v", err) } - err = af.Close() - if err != nil { + if err := af.Close(); err != nil { t.Fatalf("Error closing autofile") } // Both files should exist - if body := MustReadFile(name + "_old"); string(body) != "Line 1\nLine 2\n" { + if body := cmn.MustReadFile(name + "_old"); string(body) != "Line 1\nLine 2\n" { t.Errorf("Unexpected body %s", body) } - if body := MustReadFile(name); string(body) != "Line 3\nLine 4\n" { + if body := cmn.MustReadFile(name); string(body) != "Line 3\nLine 4\n" { t.Errorf("Unexpected body %s", body) } - } diff --git a/common/os.go b/common/os.go index 8b7143f5a..19aa479f9 100644 --- a/common/os.go +++ b/common/os.go @@ -8,7 +8,6 @@ import ( "os" "os/signal" "strings" - "syscall" ) var ( @@ -18,7 +17,7 @@ var ( func TrapSignal(cb func()) { c := make(chan os.Signal, 1) signal.Notify(c, os.Interrupt) - signal.Notify(c, syscall.SIGTERM) + signal.Notify(c, os.Kill) // nolint: megacheck go func() { for sig := range c { fmt.Printf("captured %v, exiting...\n", sig) diff --git a/common/service.go b/common/service.go index 5ac386316..2d86baafe 100644 --- a/common/service.go +++ b/common/service.go @@ -140,18 +140,16 @@ func (bs *BaseService) OnStop() {} // Implements Service func (bs *BaseService) Reset() (bool, error) { - if atomic.CompareAndSwapUint32(&bs.stopped, 1, 0) { - // whether or not we've started, we can reset - atomic.CompareAndSwapUint32(&bs.started, 1, 0) - - bs.Quit = make(chan struct{}) - return true, bs.impl.OnReset() - } else { + if stopped := atomic.CompareAndSwapUint32(&bs.stopped, 1, 0); !stopped { bs.Logger.Debug(Fmt("Can't reset %v. Not stopped", bs.name), "impl", bs.impl) return false, nil } - // never happens - return false, nil // nolint: vet + + // whether or not we've started, we can reset + atomic.CompareAndSwapUint32(&bs.started, 1, 0) + + bs.Quit = make(chan struct{}) + return true, bs.impl.OnReset() } // Implements Service diff --git a/merkle/simple_tree.go b/merkle/simple_tree.go index b373743fc..8106246d6 100644 --- a/merkle/simple_tree.go +++ b/merkle/simple_tree.go @@ -22,7 +22,6 @@ For larger datasets, use IAVLTree. */ -// nolint: goimports package merkle import ( From c8805fd7deb52c9565f1f0c5e465b480dcf13f2c Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Wed, 4 Oct 2017 00:13:58 -0400 Subject: [PATCH 08/26] metalinter fixes from review --- autofile/autofile_test.go | 5 +++-- common/os.go | 4 ++-- common/service.go | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/autofile/autofile_test.go b/autofile/autofile_test.go index 05152219c..8f453dd07 100644 --- a/autofile/autofile_test.go +++ b/autofile/autofile_test.go @@ -18,7 +18,7 @@ func TestSIGHUP(t *testing.T) { t.Fatalf("Error creating tempfile: %v", err) } // Here is the actual AutoFile - af, err := cmn.OpenAutoFile(name) + af, err := OpenAutoFile(name) if err != nil { t.Fatalf("Error creating autofile: %v", err) } @@ -34,7 +34,8 @@ func TestSIGHUP(t *testing.T) { } // Move the file over - if err := os.Rename(name, name+"_old"); err != nil { + err = os.Rename(name, name+"_old") + if err != nil { t.Fatalf("Error moving autofile: %v", err) } diff --git a/common/os.go b/common/os.go index 19aa479f9..625d6ae16 100644 --- a/common/os.go +++ b/common/os.go @@ -8,6 +8,7 @@ import ( "os" "os/signal" "strings" + "syscall" ) var ( @@ -16,8 +17,7 @@ var ( func TrapSignal(cb func()) { c := make(chan os.Signal, 1) - signal.Notify(c, os.Interrupt) - signal.Notify(c, os.Kill) // nolint: megacheck + signal.Notify(c, os.Interrupt, syscall.SIGTERM) go func() { for sig := range c { fmt.Printf("captured %v, exiting...\n", sig) diff --git a/common/service.go b/common/service.go index 2d86baafe..8d4de30a8 100644 --- a/common/service.go +++ b/common/service.go @@ -140,7 +140,7 @@ func (bs *BaseService) OnStop() {} // Implements Service func (bs *BaseService) Reset() (bool, error) { - if stopped := atomic.CompareAndSwapUint32(&bs.stopped, 1, 0); !stopped { + if !atomic.CompareAndSwapUint32(&bs.stopped, 1, 0) { bs.Logger.Debug(Fmt("Can't reset %v. Not stopped", bs.name), "impl", bs.impl) return false, nil } From 35e38e8932e69dcf3038729c611358f9c2ac960d Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 11 Oct 2017 12:42:54 +0400 Subject: [PATCH 09/26] call go env GOPATH if env var is not found (Refs #60) --- common/os.go | 16 +++++++++++++++- common/os_test.go | 23 +++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/common/os.go b/common/os.go index e0a009264..8af6cd22e 100644 --- a/common/os.go +++ b/common/os.go @@ -6,6 +6,7 @@ import ( "io" "io/ioutil" "os" + "os/exec" "os/signal" "path/filepath" "strings" @@ -13,9 +14,22 @@ import ( ) var ( - GoPath = os.Getenv("GOPATH") + GoPath = gopath() ) +func gopath() string { + path := os.Getenv("GOPATH") + if len(path) == 0 { + goCmd := exec.Command("go", "env", "GOPATH") + out, err := goCmd.Output() + if err != nil { + panic(fmt.Sprintf("failed to determine gopath: %v", err)) + } + path = string(out) + } + return path +} + func TrapSignal(cb func()) { c := make(chan os.Signal, 1) signal.Notify(c, os.Interrupt, syscall.SIGTERM) diff --git a/common/os_test.go b/common/os_test.go index 05359e36e..168eb438c 100644 --- a/common/os_test.go +++ b/common/os_test.go @@ -27,3 +27,26 @@ func TestWriteFileAtomic(t *testing.T) { t.Fatal(err) } } + +func TestGoPath(t *testing.T) { + // restore original gopath upon exit + path := os.Getenv("GOPATH") + defer func() { + _ = os.Setenv("GOPATH", path) + }() + + err := os.Setenv("GOPATH", "~/testgopath") + if err != nil { + t.Fatal(err) + } + path = gopath() + if path != "~/testgopath" { + t.Fatalf("gopath should return GOPATH env var if set, got %v", path) + } + os.Unsetenv("GOPATH") + + path = gopath() + if path == "~/testgopath" || path == "" { + t.Fatalf("gopath should return go env GOPATH result if env var does not exist, got %v", path) + } +} From 7166252a521951eb8b6bd26db28b2b90586941a9 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 11 Oct 2017 12:48:05 +0400 Subject: [PATCH 10/26] add codeowners file [ci skip] --- CODEOWNERS | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 CODEOWNERS diff --git a/CODEOWNERS b/CODEOWNERS new file mode 100644 index 000000000..d2dddf85a --- /dev/null +++ b/CODEOWNERS @@ -0,0 +1,3 @@ +* @melekes @ebuchman +*.md @zramsay +*.rst @zramsay From 6b1e3bcee3cf69fbac004cef8f8f3788f12cd58e Mon Sep 17 00:00:00 2001 From: Alexis Sellier Date: Fri, 13 Oct 2017 13:03:43 +0200 Subject: [PATCH 11/26] Add comments about copying --- db/go_level_db.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/db/go_level_db.go b/db/go_level_db.go index 2d2ba44e3..f3ed79e30 100644 --- a/db/go_level_db.go +++ b/db/go_level_db.go @@ -121,6 +121,7 @@ type goLevelDBIterator struct { iterator.Iterator } +// Key returns a copy of the current key. func (it *goLevelDBIterator) Key() []byte { key := it.Key() k := make([]byte, len(key)) @@ -129,6 +130,7 @@ func (it *goLevelDBIterator) Key() []byte { return k } +// Value returns a copy of the current value. func (it *goLevelDBIterator) Value() []byte { val := it.Value() v := make([]byte, len(val)) From 0b22b27bbb08947e3875dcfe22f95b983635eb0a Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Mon, 16 Oct 2017 22:38:37 +0400 Subject: [PATCH 12/26] avoid infinite recursion by proxying to iterator ``` WARNING: staticcheck, gosimple and unused are all set, using megacheck instead db/go_level_db.go:126:15:warning: infinite recursive call (SA5007) (megacheck) db/go_level_db.go:135:17:warning: infinite recursive call (SA5007) (megacheck) db/mem_db.go:106:2:warning: redundant return statement (S1023) (megacheck) ``` https://circleci.com/gh/tendermint/tmlibs/220 Also remove unnecessary return statement --- db/go_level_db.go | 18 +++++++++++++++--- db/mem_db.go | 1 - 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/db/go_level_db.go b/db/go_level_db.go index f3ed79e30..4abd76112 100644 --- a/db/go_level_db.go +++ b/db/go_level_db.go @@ -118,12 +118,12 @@ func (db *GoLevelDB) Stats() map[string]string { } type goLevelDBIterator struct { - iterator.Iterator + source iterator.Iterator } // Key returns a copy of the current key. func (it *goLevelDBIterator) Key() []byte { - key := it.Key() + key := it.source.Key() k := make([]byte, len(key)) copy(k, key) @@ -132,13 +132,25 @@ func (it *goLevelDBIterator) Key() []byte { // Value returns a copy of the current value. func (it *goLevelDBIterator) Value() []byte { - val := it.Value() + val := it.source.Value() v := make([]byte, len(val)) copy(v, val) return v } +func (it *goLevelDBIterator) Error() error { + return it.source.Error() +} + +func (it *goLevelDBIterator) Next() bool { + return it.source.Next() +} + +func (it *goLevelDBIterator) Release() { + it.source.Release() +} + func (db *GoLevelDB) Iterator() Iterator { return &goLevelDBIterator{db.db.NewIterator(nil, nil)} } diff --git a/db/mem_db.go b/db/mem_db.go index 55d594fa6..077427509 100644 --- a/db/mem_db.go +++ b/db/mem_db.go @@ -103,7 +103,6 @@ func (it *memDBIterator) Value() []byte { func (it *memDBIterator) Release() { it.db = nil it.keys = nil - return } func (it *memDBIterator) Error() error { From 498fb1134a550761f2cc0acef5cf9fbc76fb2562 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 17 Oct 2017 16:26:52 +0400 Subject: [PATCH 13/26] write docs for autofile/group --- autofile/group.go | 73 +++++++++++++++++++++++++++++------------------ 1 file changed, 46 insertions(+), 27 deletions(-) diff --git a/autofile/group.go b/autofile/group.go index eedb67b50..b66b5c692 100644 --- a/autofile/group.go +++ b/autofile/group.go @@ -18,6 +18,13 @@ import ( . "github.com/tendermint/tmlibs/common" ) +const ( + groupCheckDuration = 5000 * time.Millisecond + defaultHeadSizeLimit = 10 * 1024 * 1024 // 10MB + defaultTotalSizeLimit = 1 * 1024 * 1024 * 1024 // 1GB + maxFilesToRemove = 4 // needs to be greater than 1 +) + /* You can open a Group to keep restrictions on an AutoFile, like the maximum size of each chunk, and/or the total amount of bytes @@ -25,33 +32,27 @@ stored in the group. The first file to be written in the Group.Dir is the head file. - Dir/ - - + Dir/ + - Once the Head file reaches the size limit, it will be rotated. - Dir/ - - .000 // First rolled file - - // New head path, starts empty. - // The implicit index is 001. + Dir/ + - .000 // First rolled file + - // New head path, starts empty. + // The implicit index is 001. As more files are written, the index numbers grow... - Dir/ - - .000 // First rolled file - - .001 // Second rolled file - - ... - - // New head path + Dir/ + - .000 // First rolled file + - .001 // Second rolled file + - ... + - // New head path The Group can also be used to binary-search for some line, assuming that marker lines are written occasionally. */ - -const groupCheckDuration = 5000 * time.Millisecond -const defaultHeadSizeLimit = 10 * 1024 * 1024 // 10MB -const defaultTotalSizeLimit = 1 * 1024 * 1024 * 1024 // 1GB -const maxFilesToRemove = 4 // needs to be greater than 1 - type Group struct { BaseService @@ -109,37 +110,43 @@ func (g *Group) OnStop() { g.ticker.Stop() } +// SetHeadSizeLimit allows you to overwrite default head size limit - 10MB. func (g *Group) SetHeadSizeLimit(limit int64) { g.mtx.Lock() g.headSizeLimit = limit g.mtx.Unlock() } +// HeadSizeLimit returns the current head size limit. func (g *Group) HeadSizeLimit() int64 { g.mtx.Lock() defer g.mtx.Unlock() return g.headSizeLimit } +// SetTotalSizeLimit allows you to overwrite default total size limit of the +// group - 1GB. func (g *Group) SetTotalSizeLimit(limit int64) { g.mtx.Lock() g.totalSizeLimit = limit g.mtx.Unlock() } +// TotalSizeLimit returns total size limit of the group. func (g *Group) TotalSizeLimit() int64 { g.mtx.Lock() defer g.mtx.Unlock() return g.totalSizeLimit } +// MaxIndex returns index of the last file in the group. func (g *Group) MaxIndex() int { g.mtx.Lock() defer g.mtx.Unlock() return g.maxIndex } -// Auto appends "\n" +// WriteLine writes line into the current head of the group. It also appends "\n". // NOTE: Writes are buffered so they don't write synchronously // TODO: Make it halt if space is unavailable func (g *Group) WriteLine(line string) error { @@ -149,6 +156,8 @@ func (g *Group) WriteLine(line string) error { return err } +// Flush writes any buffered data to the underlying file and commits the +// current content of the file to stable storage. func (g *Group) Flush() error { g.mtx.Lock() defer g.mtx.Unlock() @@ -223,6 +232,8 @@ func (g *Group) checkTotalSizeLimit() { } } +// RotateFile causes group to close the current head and assign it some index. +// Note it does not create a new head. func (g *Group) RotateFile() { g.mtx.Lock() defer g.mtx.Unlock() @@ -241,8 +252,8 @@ func (g *Group) RotateFile() { g.maxIndex += 1 } -// NOTE: if error, returns no GroupReader. -// CONTRACT: Caller must close the returned GroupReader +// NewReader returns a new group reader. +// CONTRACT: Caller must close the returned GroupReader. func (g *Group) NewReader(index int) (*GroupReader, error) { r := newGroupReader(g) err := r.SetIndex(index) @@ -423,14 +434,15 @@ GROUP_LOOP: return } +// GroupInfo holds information about the group. type GroupInfo struct { - MinIndex int - MaxIndex int - TotalSize int64 - HeadSize int64 + MinIndex int // index of the first file in the group, including head + MaxIndex int // index of the last file in the group, including head + TotalSize int64 // total size of the group + HeadSize int64 // size of the head } -// Returns info after scanning all files in g.Head's dir +// Returns info after scanning all files in g.Head's dir. func (g *Group) ReadGroupInfo() GroupInfo { g.mtx.Lock() defer g.mtx.Unlock() @@ -505,6 +517,7 @@ func filePathForIndex(headPath string, index int, maxIndex int) string { //-------------------------------------------------------------------------------- +// GroupReader provides an interface for reading from a Group. type GroupReader struct { *Group mtx sync.Mutex @@ -524,6 +537,7 @@ func newGroupReader(g *Group) *GroupReader { } } +// Close closes the GroupReader by closing the cursor file. func (gr *GroupReader) Close() error { gr.mtx.Lock() defer gr.mtx.Unlock() @@ -540,7 +554,7 @@ func (gr *GroupReader) Close() error { } } -// Reads a line (without delimiter) +// ReadLine reads a line (without delimiter). // just return io.EOF if no new lines found. func (gr *GroupReader) ReadLine() (string, error) { gr.mtx.Lock() @@ -613,6 +627,9 @@ func (gr *GroupReader) openFile(index int) error { return nil } +// PushLine makes the given line the current one, so the next time somebody +// calls ReadLine, this line will be returned. +// panics if called twice without calling ReadLine. func (gr *GroupReader) PushLine(line string) { gr.mtx.Lock() defer gr.mtx.Unlock() @@ -624,13 +641,15 @@ func (gr *GroupReader) PushLine(line string) { } } -// Cursor's file index. +// CurIndex returns cursor's file index. func (gr *GroupReader) CurIndex() int { gr.mtx.Lock() defer gr.mtx.Unlock() return gr.curIndex } +// SetIndex sets the cursor's file index to index by opening a file at this +// position. func (gr *GroupReader) SetIndex(index int) error { gr.mtx.Lock() defer gr.mtx.Unlock() From 45095e83e790624980240e83628d6971cafa5495 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 17 Oct 2017 16:48:44 +0400 Subject: [PATCH 14/26] add Write method to autofile/Group --- autofile/group.go | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/autofile/group.go b/autofile/group.go index b66b5c692..947c295f8 100644 --- a/autofile/group.go +++ b/autofile/group.go @@ -146,6 +146,17 @@ func (g *Group) MaxIndex() int { return g.maxIndex } +// Write writes the contents of p into the current head of the group. It +// returns the number of bytes written. If nn < len(p), it also returns an +// error explaining why the write is short. +// NOTE: Writes are buffered so they don't write synchronously +// TODO: Make it halt if space is unavailable +func (g *Group) Write(p []byte) (nn int, err error) { + g.mtx.Lock() + defer g.mtx.Unlock() + return g.headBuf.Write(p) +} + // WriteLine writes line into the current head of the group. It also appends "\n". // NOTE: Writes are buffered so they don't write synchronously // TODO: Make it halt if space is unavailable From aace56018a5f70c09a2ab26b280c943a85aba5d7 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Fri, 20 Oct 2017 12:38:45 +0400 Subject: [PATCH 15/26] add Read method to GroupReader --- autofile/group.go | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/autofile/group.go b/autofile/group.go index 947c295f8..2c6aa6109 100644 --- a/autofile/group.go +++ b/autofile/group.go @@ -565,6 +565,42 @@ func (gr *GroupReader) Close() error { } } +// Read implements io.Reader, reading bytes from the current Reader +// incrementing index until enough bytes are read. +func (gr *GroupReader) Read(p []byte) (n int, err error) { + gr.mtx.Lock() + defer gr.mtx.Unlock() + + // Open file if not open yet + if gr.curReader == nil { + if err = gr.openFile(gr.curIndex); err != nil { + return 0, err + } + } + + // Iterate over files until enough bytes are read + lenP := len(p) + for { + nn, err := gr.curReader.Read(p[n:]) + n += nn + if err == io.EOF { + // Open the next file + if err1 := gr.openFile(gr.curIndex + 1); err1 != nil { + return n, err1 + } + if n >= lenP { + return n, nil + } else { + continue + } + } else if err != nil { + return n, err + } + } + + return n, err +} + // ReadLine reads a line (without delimiter). // just return io.EOF if no new lines found. func (gr *GroupReader) ReadLine() (string, error) { From 35e81018e9bd183be7121b6b900dff3d49e234d5 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Fri, 20 Oct 2017 13:09:11 +0400 Subject: [PATCH 16/26] add MinIndex method to Group --- autofile/group.go | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/autofile/group.go b/autofile/group.go index 2c6aa6109..d5797d087 100644 --- a/autofile/group.go +++ b/autofile/group.go @@ -146,6 +146,13 @@ func (g *Group) MaxIndex() int { return g.maxIndex } +// MinIndex returns index of the first file in the group. +func (g *Group) MinIndex() int { + g.mtx.Lock() + defer g.mtx.Unlock() + return g.minIndex +} + // Write writes the contents of p into the current head of the group. It // returns the number of bytes written. If nn < len(p), it also returns an // error explaining why the write is short. From c75ddd0fa3f669c1b391291a10361ddf8c5170bf Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Mon, 23 Oct 2017 13:02:02 +0400 Subject: [PATCH 17/26] return err if empty slice given --- autofile/group.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/autofile/group.go b/autofile/group.go index d5797d087..4b3cd6565 100644 --- a/autofile/group.go +++ b/autofile/group.go @@ -575,6 +575,11 @@ func (gr *GroupReader) Close() error { // Read implements io.Reader, reading bytes from the current Reader // incrementing index until enough bytes are read. func (gr *GroupReader) Read(p []byte) (n int, err error) { + lenP := len(p) + if lenP == 0 { + return 0, errors.New("given empty slice") + } + gr.mtx.Lock() defer gr.mtx.Unlock() @@ -586,7 +591,6 @@ func (gr *GroupReader) Read(p []byte) (n int, err error) { } // Iterate over files until enough bytes are read - lenP := len(p) for { nn, err := gr.curReader.Read(p[n:]) n += nn From 21b2c26fb1b26edf5846792890e01eaa8a472508 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Mon, 23 Oct 2017 13:02:14 +0400 Subject: [PATCH 18/26] GroupReader#Read: return io.EOF if file is empty --- autofile/group.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/autofile/group.go b/autofile/group.go index 4b3cd6565..6d70a3dbb 100644 --- a/autofile/group.go +++ b/autofile/group.go @@ -606,6 +606,8 @@ func (gr *GroupReader) Read(p []byte) (n int, err error) { } } else if err != nil { return n, err + } else if nn == 0 { // empty file + return n, err } } From 0eff425bc7e3d6137ea3c59ad7436eafe5ef55d2 Mon Sep 17 00:00:00 2001 From: Silas Davis Date: Mon, 23 Oct 2017 18:51:49 +0100 Subject: [PATCH 19/26] fix zeroed buffer getting flushed to the empty event --- events/event_cache.go | 12 ++++-------- events/event_cache_test.go | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 8 deletions(-) create mode 100644 events/event_cache_test.go diff --git a/events/event_cache.go b/events/event_cache.go index 905f1096a..f508e873d 100644 --- a/events/event_cache.go +++ b/events/event_cache.go @@ -1,9 +1,5 @@ package events -const ( - eventsBufferSize = 1000 -) - // An EventCache buffers events for a Fireable // All events are cached. Filtering happens on Flush type EventCache struct { @@ -14,8 +10,7 @@ type EventCache struct { // Create a new EventCache with an EventSwitch as backend func NewEventCache(evsw Fireable) *EventCache { return &EventCache{ - evsw: evsw, - events: make([]eventInfo, eventsBufferSize), + evsw: evsw, } } @@ -27,7 +22,7 @@ type eventInfo struct { // Cache an event to be fired upon finality. func (evc *EventCache) FireEvent(event string, data EventData) { - // append to list + // append to list (go will grow our backing array exponentially) evc.events = append(evc.events, eventInfo{event, data}) } @@ -37,5 +32,6 @@ func (evc *EventCache) Flush() { for _, ei := range evc.events { evc.evsw.FireEvent(ei.event, ei.data) } - evc.events = make([]eventInfo, eventsBufferSize) + // Clear the buffer, since we only add to it with append it's safe to just set it to nil and maybe safe an allocation + evc.events = nil } diff --git a/events/event_cache_test.go b/events/event_cache_test.go new file mode 100644 index 000000000..ab321da3a --- /dev/null +++ b/events/event_cache_test.go @@ -0,0 +1,35 @@ +package events + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestEventCache_Flush(t *testing.T) { + evsw := NewEventSwitch() + evsw.Start() + evsw.AddListenerForEvent("nothingness", "", func(data EventData) { + // Check we are not initialising an empty buffer full of zeroed eventInfos in the EventCache + require.FailNow(t, "We should never receive a message on this switch since none are fired") + }) + evc := NewEventCache(evsw) + evc.Flush() + // Check after reset + evc.Flush() + fail := true + pass := false + evsw.AddListenerForEvent("somethingness", "something", func(data EventData) { + if fail { + require.FailNow(t, "Shouldn't see a message until flushed") + } + pass = true + }) + evc.FireEvent("something", struct{ int }{1}) + evc.FireEvent("something", struct{ int }{2}) + evc.FireEvent("something", struct{ int }{3}) + fail = false + evc.Flush() + assert.True(t, pass) +} From 81591e288e87eba7735df53207f74a09ba5f289a Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 24 Oct 2017 23:19:53 +0400 Subject: [PATCH 20/26] fix metalinter warnings --- autofile/group.go | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/autofile/group.go b/autofile/group.go index 6d70a3dbb..bbf77d27e 100644 --- a/autofile/group.go +++ b/autofile/group.go @@ -591,8 +591,9 @@ func (gr *GroupReader) Read(p []byte) (n int, err error) { } // Iterate over files until enough bytes are read + var nn int for { - nn, err := gr.curReader.Read(p[n:]) + nn, err = gr.curReader.Read(p[n:]) n += nn if err == io.EOF { // Open the next file @@ -610,8 +611,6 @@ func (gr *GroupReader) Read(p []byte) (n int, err error) { return n, err } } - - return n, err } // ReadLine reads a line (without delimiter). From 103fee61921ee8bebd055bedd0815ddc71e03d90 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 24 Oct 2017 23:20:17 +0400 Subject: [PATCH 21/26] add tests for autofile group Write, reader#Read --- autofile/group_test.go | 91 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 91 insertions(+) diff --git a/autofile/group_test.go b/autofile/group_test.go index 0cfcef72f..398ea3ae9 100644 --- a/autofile/group_test.go +++ b/autofile/group_test.go @@ -1,6 +1,7 @@ package autofile import ( + "bytes" "errors" "io" "io/ioutil" @@ -400,3 +401,93 @@ func TestFindLast4(t *testing.T) { // Cleanup destroyTestGroup(t, g) } + +func TestWrite(t *testing.T) { + g := createTestGroup(t, 0) + + written := []byte("Medusa") + g.Write(written) + g.Flush() + + read := make([]byte, len(written)) + gr, err := g.NewReader(0) + if err != nil { + t.Fatalf("Failed to create reader: %v", err) + } + _, err = gr.Read(read) + if err != nil { + t.Fatalf("Failed to read data: %v", err) + } + + if !bytes.Equal(written, read) { + t.Errorf("%s, %s should be equal", string(written), string(read)) + } + + // Cleanup + destroyTestGroup(t, g) +} + +func TestGroupReaderRead(t *testing.T) { + g := createTestGroup(t, 0) + + professor := []byte("Professor Monster") + g.Write(professor) + g.Flush() + g.RotateFile() + frankenstein := []byte("Frankenstein's Monster") + g.Write(frankenstein) + g.Flush() + + totalWrittenLength := len(professor) + len(frankenstein) + read := make([]byte, totalWrittenLength) + gr, err := g.NewReader(0) + if err != nil { + t.Fatalf("Failed to create reader: %v", err) + } + n, err := gr.Read(read) + if err != nil { + t.Fatalf("Failed to read data: %v", err) + } + if n != totalWrittenLength { + t.Errorf("Failed to read enough bytes: wanted %d, but read %d", totalWrittenLength, n) + } + + professorPlusFrankenstein := professor + professorPlusFrankenstein = append(professorPlusFrankenstein, frankenstein...) + if !bytes.Equal(read, professorPlusFrankenstein) { + t.Errorf("%s, %s should be equal", string(professorPlusFrankenstein), string(read)) + } + + // Cleanup + destroyTestGroup(t, g) +} + +func TestMinIndex(t *testing.T) { + g := createTestGroup(t, 0) + + if g.MinIndex() != 0 { + t.Error("MinIndex should be zero at the beginning") + } + + // Cleanup + destroyTestGroup(t, g) +} + +func TestMaxIndex(t *testing.T) { + g := createTestGroup(t, 0) + + if g.MaxIndex() != 0 { + t.Error("MaxIndex should be zero at the beginning") + } + + g.WriteLine("Line 1") + g.Flush() + g.RotateFile() + + if g.MaxIndex() != 1 { + t.Error("MaxIndex should point to the last file") + } + + // Cleanup + destroyTestGroup(t, g) +} From f99c73502ca32782c0e68258848cc5c365e133b6 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Tue, 24 Oct 2017 23:30:05 +0400 Subject: [PATCH 22/26] add codecov --- circle.yml | 10 +++++----- test.sh | 12 ++++++++++++ 2 files changed, 17 insertions(+), 5 deletions(-) create mode 100755 test.sh diff --git a/circle.yml b/circle.yml index 8e3ad168b..3dba976be 100644 --- a/circle.yml +++ b/circle.yml @@ -1,11 +1,9 @@ machine: environment: - GOPATH: /home/ubuntu/.go_workspace + GOPATH: "${HOME}/.go_workspace" PROJECT_PARENT_PATH: "$GOPATH/src/github.com/$CIRCLE_PROJECT_USERNAME" PROJECT_PATH: $GOPATH/src/github.com/$CIRCLE_PROJECT_USERNAME/$CIRCLE_PROJECT_REPONAME - GO15VENDOREXPERIMENT: 1 hosts: - circlehost: 127.0.0.1 localhost: 127.0.0.1 dependencies: @@ -17,5 +15,7 @@ dependencies: test: override: - - "go version" - - "cd $PROJECT_PATH && make get_vendor_deps && make metalinter_test && make test" + - cd $PROJECT_PATH && make get_vendor_deps && make metalinter_test && bash ./test.sh + post: + - cd "$PROJECT_PATH" && bash <(curl -s https://codecov.io/bash) -f coverage.txt + - cd "$PROJECT_PATH" && mv coverage.txt "${CIRCLE_ARTIFACTS}" diff --git a/test.sh b/test.sh new file mode 100755 index 000000000..012162b07 --- /dev/null +++ b/test.sh @@ -0,0 +1,12 @@ +#!/usr/bin/env bash + +set -e +echo "" > coverage.txt + +for d in $(go list ./... | grep -v vendor); do + go test -race -coverprofile=profile.out -covermode=atomic "$d" + if [ -f profile.out ]; then + cat profile.out >> coverage.txt + rm profile.out + fi +done From bcf15e527d89444bf260fd83699ef70fde2fb6e1 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Wed, 25 Oct 2017 11:01:52 +0400 Subject: [PATCH 23/26] make GoPath a function otherwise it could try to execute go binary and panic if no go binary found. See https://github.com/tendermint/tendermint/issues/782 --- common/os.go | 13 +++++++++---- common/os_test.go | 27 ++++++++++++++++++++++----- 2 files changed, 31 insertions(+), 9 deletions(-) diff --git a/common/os.go b/common/os.go index 8af6cd22e..6b591ce59 100644 --- a/common/os.go +++ b/common/os.go @@ -13,11 +13,15 @@ import ( "syscall" ) -var ( - GoPath = gopath() -) +var gopath string + +// GoPath returns GOPATH env variable value. If it is not set, this function +// will try to call `go env GOPATH` subcommand. +func GoPath() string { + if gopath != "" { + return gopath + } -func gopath() string { path := os.Getenv("GOPATH") if len(path) == 0 { goCmd := exec.Command("go", "env", "GOPATH") @@ -27,6 +31,7 @@ func gopath() string { } path = string(out) } + gopath = path return path } diff --git a/common/os_test.go b/common/os_test.go index 168eb438c..126723aa6 100644 --- a/common/os_test.go +++ b/common/os_test.go @@ -39,14 +39,31 @@ func TestGoPath(t *testing.T) { if err != nil { t.Fatal(err) } - path = gopath() + path = GoPath() if path != "~/testgopath" { - t.Fatalf("gopath should return GOPATH env var if set, got %v", path) + t.Fatalf("should get GOPATH env var value, got %v", path) } os.Unsetenv("GOPATH") - path = gopath() - if path == "~/testgopath" || path == "" { - t.Fatalf("gopath should return go env GOPATH result if env var does not exist, got %v", path) + path = GoPath() + if path != "~/testgopath" { + t.Fatalf("subsequent calls should return the same value, got %v", path) + } +} + +func TestGoPathWithoutEnvVar(t *testing.T) { + // restore original gopath upon exit + path := os.Getenv("GOPATH") + defer func() { + _ = os.Setenv("GOPATH", path) + }() + + os.Unsetenv("GOPATH") + // reset cache + gopath = "" + + path = GoPath() + if path == "" || path == "~/testgopath" { + t.Fatalf("should get nonempty result of calling go env GOPATH, got %v", path) } } From 42145a82bd953e4a666bcc01a37e2d140b9c1e8e Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Thu, 26 Oct 2017 20:57:33 -0400 Subject: [PATCH 24/26] version and changelog --- CHANGELOG.md | 15 +++++++++++++++ version/version.go | 2 +- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e36a02d9a..3a21dd5dd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,20 @@ # Changelog +## 0.4.0 (October 26, 2017) + +BREAKING: + - [common] GoPath is now a function + - [db] `DB` and `Iterator` interfaces have new methods to better support iteration + +FEATURES: + - [autofile] `Read([]byte)` and `Write([]byte)` methods on `Group` to support binary WAL + +IMPROVEMENTS: + - comments and linting + +BUG FIXES: + - [events] fix allocation error prefixing cache with 1000 empty events + ## 0.3.2 (October 2, 2017) BUG FIXES: diff --git a/version/version.go b/version/version.go index 77580b5ad..c1635d202 100644 --- a/version/version.go +++ b/version/version.go @@ -1,3 +1,3 @@ package version -const Version = "0.3.2" +const Version = "0.4.0" From 092eb701c7276907cdbed258750e22ce895b6735 Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Fri, 27 Oct 2017 11:01:40 -0400 Subject: [PATCH 25/26] cmn: Kill --- common/os.go | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/common/os.go b/common/os.go index 6b591ce59..81f703c7d 100644 --- a/common/os.go +++ b/common/os.go @@ -50,6 +50,12 @@ func TrapSignal(cb func()) { select {} } +// Kill the running process by sending itself SIGTERM +func Kill() error { + pid := os.Getpid() + return syscall.Kill(pid, syscall.SIGTERM) +} + func Exit(s string) { fmt.Printf(s + "\n") os.Exit(1) From b14c99669810fdb9bdd305fe0e09f72c86df815b Mon Sep 17 00:00:00 2001 From: Ethan Buchman Date: Fri, 27 Oct 2017 11:52:10 -0400 Subject: [PATCH 26/26] update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a21dd5dd..c380fdcd0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ BREAKING: FEATURES: - [autofile] `Read([]byte)` and `Write([]byte)` methods on `Group` to support binary WAL + - [common] `Kill()` sends SIGTERM to the current process IMPROVEMENTS: - comments and linting