From b31daa3d66b714f429a968009de5e54778ce9b37 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Fri, 20 Sep 2024 09:40:36 +0400 Subject: [PATCH] feat!: add `goleveldb` and remove `pebbledb` build flag (#202) * feat: add `goleveldb` build flag because goleveldb won't be the default CometBFT DB in the future. * add changelog * copy golangci config from cometbft * remove pebbledb tag * update *DBBackend descriptions --- .../breaking/578-add-goleveldb-build-flag.md | 2 + .golangci.yml | 137 ++++++++++-------- backend_test.go | 12 +- common_test.go | 9 +- db.go | 14 +- goleveldb.go | 3 + goleveldb_batch.go | 3 + goleveldb_iterator.go | 3 + goleveldb_test.go | 14 ++ memdb.go | 9 +- memdb_batch.go | 4 +- pebble.go | 18 +-- pebble_test.go | 3 - prefixdb.go | 5 +- test_helpers.go | 1 - types.go | 10 +- util.go | 2 +- 17 files changed, 134 insertions(+), 115 deletions(-) create mode 100644 .changelog/unreleased/breaking/578-add-goleveldb-build-flag.md diff --git a/.changelog/unreleased/breaking/578-add-goleveldb-build-flag.md b/.changelog/unreleased/breaking/578-add-goleveldb-build-flag.md new file mode 100644 index 0000000..e893251 --- /dev/null +++ b/.changelog/unreleased/breaking/578-add-goleveldb-build-flag.md @@ -0,0 +1,2 @@ +- Add `goleveldb` build flag. + ([\#202](https://github.com/cometbft/cometbft-db/pull/202)) diff --git a/.golangci.yml b/.golangci.yml index cad39dc..3431737 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -3,51 +3,66 @@ run: timeout: 10m linters: - disable-all: true - enable: - - copyloopvar - - errcheck - - gci - - goconst - - gocritic - - gofumpt - - gosec - - gosimple - - govet - - ineffassign - - misspell - - nakedret - - staticcheck - - thelper - - typecheck - - stylecheck - - revive - - typecheck - - tenv - - unconvert - # Prefer unparam over revive's unused param. It is more thorough in its checking. - - unparam - - unused - - misspell + enable-all: true + disable: + - containedctx + - contextcheck + - cyclop + - dupword + - errorlint + - errname + - err113 + - exhaustive + - exhaustruct + - execinquery + - forbidigo + - forcetypeassert + - funlen + - gochecknoglobals + - gochecknoinits + - gocognit + - gocyclo + - godox + - gomnd + - interfacebloat + - intrange + - ireturn + - lll + - maintidx + - mnd + - nestif + - nilnil + - nlreturn + - nonamedreturns + - predeclared + - tagliatelle + - testifylint + - varnamelen + - wrapcheck + - wsl issues: exclude-rules: - - text: 'differs only by capitalization to method' - linters: - - revive - - text: 'Use of weak random number generator' + - path: _test\.go linters: + - gocritic + - gofmt + - goimport - gosec - - linters: - - staticcheck - text: "SA1019:" # silence errors on usage of deprecated funcs - + - noctx + - paralleltest + - testpackage + - tparallel max-issues-per-linter: 10000 max-same-issues: 10000 linters-settings: - errcheck: - check-blank: true + dogsled: + max-blank-identifiers: 3 + goconst: + ignore-tests: true + misspell: + locale: US gci: sections: - standard # Standard section: captures all standard packages. @@ -56,14 +71,27 @@ linters-settings: - dot # dot imports - prefix(github.com/cometbft/cometbft-db) custom-order: true + depguard: + rules: + main: + files: + - $all + - "!$test" + allow: + - $gostd + - github.com/cockroachdb/pebble + - github.com/google/btree + test: + files: + - "$test" + allow: + - $gostd + - github.com/stretchr/testify + revive: enable-all-rules: true - # Do NOT whine about the following, full explanation found in: - # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#description-of-available-rules rules: - - name: use-any - disabled: true - - name: if-return + - name: comment-spacings # temporarily disabled disabled: true - name: max-public-structs disabled: true @@ -73,6 +101,8 @@ linters-settings: disabled: true - name: cyclomatic disabled: true + - name: deep-exit + disabled: true - name: file-header disabled: true - name: function-length @@ -87,29 +117,22 @@ linters-settings: disabled: true - name: empty-lines disabled: true - - name: banned-characters - disabled: true - - name: deep-exit - disabled: true - - name: confusing-results - disabled: true - - name: unused-parameter + - name: import-shadowing disabled: true - name: modifies-value-receiver disabled: true - - name: early-return - disabled: true - name: confusing-naming disabled: true - name: defer disabled: true - # Disabled in favour of unparam. - - name: unused-parameter + - name: unchecked-type-assertion disabled: true - name: unhandled-error - disabled: false + disabled: true arguments: - - 'fmt.Printf' - - 'fmt.Print' - - 'fmt.Println' - - 'myFunction' + - "fmt.Printf" + - "fmt.Print" + - "fmt.Println" + gosec: + excludes: + - G115 diff --git a/backend_test.go b/backend_test.go index b30600f..377bcec 100644 --- a/backend_test.go +++ b/backend_test.go @@ -12,7 +12,7 @@ import ( "github.com/stretchr/testify/require" ) -// Register a test backend for PrefixDB as well, with some unrelated junk data +// Register a test backend for PrefixDB as well, with some unrelated junk data. func init() { //nolint: errcheck, revive // probably should check errors? registerDBCreator("prefixdb", func(name, dir string) (DB, error) { @@ -166,16 +166,6 @@ func TestBackendsGetSetDelete(t *testing.T) { } } -func TestGoLevelDBBackend(t *testing.T) { - name := fmt.Sprintf("test_%x", randStr(12)) - db, err := NewDB(name, GoLevelDBBackend, "") - require.NoError(t, err) - defer cleanupDBDir("", name) - - _, ok := db.(*GoLevelDB) - assert.True(t, ok) -} - func TestDBIterator(t *testing.T) { for dbType := range backends { t.Run(string(dbType), func(t *testing.T) { diff --git a/common_test.go b/common_test.go index f6e6f54..7bbb37c 100644 --- a/common_test.go +++ b/common_test.go @@ -106,7 +106,7 @@ func benchmarkRangeScans(b *testing.B, db DB, dbSize int64) { b.StartTimer() for i := 0; i < b.N; i++ { - start := rand.Int63n(dbSize - rangeSize) //nolint:gosec + start := rand.Int63n(dbSize - rangeSize) end := start + rangeSize iter, err := db.Iterator(int642Bytes(start), int642Bytes(end)) require.NoError(b, err) @@ -136,7 +136,7 @@ func benchmarkRandomReadsWrites(b *testing.B, db DB) { for i := 0; i < b.N; i++ { // Write something { - idx := rand.Int63n(numItems) //nolint:gosec + idx := rand.Int63n(numItems) internal[idx]++ val := internal[idx] idxBytes := int642Bytes(idx) @@ -150,7 +150,7 @@ func benchmarkRandomReadsWrites(b *testing.B, db DB) { // Read something { - idx := rand.Int63n(numItems) //nolint:gosec + idx := rand.Int63n(numItems) valExp := internal[idx] idxBytes := int642Bytes(idx) valBytes, err := db.Get(idxBytes) @@ -175,7 +175,6 @@ func benchmarkRandomReadsWrites(b *testing.B, db DB) { } } } - } } @@ -186,5 +185,5 @@ func int642Bytes(i int64) []byte { } func bytes2Int64(buf []byte) int64 { - return int64(binary.BigEndian.Uint64(buf)) //nolint:gosec + return int64(binary.BigEndian.Uint64(buf)) } diff --git a/db.go b/db.go index 517d2cd..de0ad30 100644 --- a/db.go +++ b/db.go @@ -11,11 +11,13 @@ type BackendType string const ( // GoLevelDBBackend represents goleveldb (github.com/syndtr/goleveldb - most // popular implementation) + // - UNMAINTANED // - pure go // - stable - // - unmaintaned + // - use goleveldb build tag (go build -tags goleveldb) GoLevelDBBackend BackendType = "goleveldb" // CLevelDBBackend represents cleveldb (uses levigo wrapper) + // - DEPRECATED // - fast // - requires gcc // - use cleveldb build tag (go build -tags cleveldb) @@ -25,22 +27,20 @@ const ( MemDBBackend BackendType = "memdb" // BoltDBBackend represents bolt (uses etcd's fork of bolt - // github.com/etcd-io/bbolt) - // - EXPERIMENTAL - // - may be faster is some use-cases (random reads - indexer) + // - DEPRECATED + // - pure go // - use boltdb build tag (go build -tags boltdb) BoltDBBackend BackendType = "boltdb" // RocksDBBackend represents rocksdb (uses github.com/tecbot/gorocksdb) - // - EXPERIMENTAL // - requires gcc // - use rocksdb build tag (go build -tags rocksdb) RocksDBBackend BackendType = "rocksdb" // BadgerDBBackend represents badger (uses github.com/dgraph-io/badger) - // - EXPERIMENTAL + // - pure go // - use badgerdb build tag (go build -tags badgerdb) BadgerDBBackend BackendType = "badgerdb" // PebbleDBDBBackend represents pebble (uses github.com/cockroachdb/pebble) - // - EXPERIMENTAL - // - use pebbledb build tag (go build -tags pebbledb) + // - pure go PebbleDBBackend BackendType = "pebbledb" ) diff --git a/goleveldb.go b/goleveldb.go index e54f91b..0a08654 100644 --- a/goleveldb.go +++ b/goleveldb.go @@ -1,3 +1,6 @@ +//go:build goleveldb +// +build goleveldb + package db import ( diff --git a/goleveldb_batch.go b/goleveldb_batch.go index 4db2bd0..351e90c 100644 --- a/goleveldb_batch.go +++ b/goleveldb_batch.go @@ -1,3 +1,6 @@ +//go:build goleveldb +// +build goleveldb + package db import ( diff --git a/goleveldb_iterator.go b/goleveldb_iterator.go index 6a3c445..c181c2b 100644 --- a/goleveldb_iterator.go +++ b/goleveldb_iterator.go @@ -1,3 +1,6 @@ +//go:build goleveldb +// +build goleveldb + package db import ( diff --git a/goleveldb_test.go b/goleveldb_test.go index 54b1d22..4c16709 100644 --- a/goleveldb_test.go +++ b/goleveldb_test.go @@ -1,9 +1,13 @@ +//go:build goleveldb +// +build goleveldb + package db import ( "fmt" "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/syndtr/goleveldb/leveldb/opt" ) @@ -43,3 +47,13 @@ func BenchmarkGoLevelDBRandomReadsWrites(b *testing.B) { benchmarkRandomReadsWrites(b, db) } + +func TestGoLevelDBBackend(t *testing.T) { + name := fmt.Sprintf("test_%x", randStr(12)) + db, err := NewDB(name, GoLevelDBBackend, "") + require.NoError(t, err) + defer cleanupDBDir("", name) + + _, ok := db.(*GoLevelDB) + assert.True(t, ok) +} diff --git a/memdb.go b/memdb.go index b915504..1e977de 100644 --- a/memdb.go +++ b/memdb.go @@ -3,6 +3,7 @@ package db import ( "bytes" "fmt" + "strconv" "sync" "github.com/google/btree" @@ -14,12 +15,12 @@ const ( ) func init() { - registerDBCreator(MemDBBackend, func(name, dir string) (DB, error) { + registerDBCreator(MemDBBackend, func(_, _ string) (DB, error) { return NewMemDB(), nil }) } -// item is a btree.Item with byte slices as keys and values +// item is a btree.Item with byte slices as keys and values. type item struct { key []byte value []byte @@ -166,7 +167,7 @@ func (db *MemDB) Stats() map[string]string { stats := make(map[string]string) stats["database.type"] = "memDB" - stats["database.size"] = fmt.Sprintf("%d", db.btree.Len()) + stats["database.size"] = strconv.Itoa(db.btree.Len()) return stats } @@ -209,7 +210,7 @@ func (db *MemDB) ReverseIteratorNoMtx(start, end []byte) (Iterator, error) { return newMemDBIteratorMtxChoice(db, start, end, true, false), nil } -func (*MemDB) Compact(start, end []byte) error { +func (*MemDB) Compact(_, _ []byte) error { // No Compaction is supported for memDB and there is no point in supporting compaction for a memory DB return nil } diff --git a/memdb_batch.go b/memdb_batch.go index 6ff30fe..c1e4d6f 100644 --- a/memdb_batch.go +++ b/memdb_batch.go @@ -2,7 +2,7 @@ package db import "fmt" -// memDBBatch operations +// memDBBatch operations. type opType int const ( @@ -24,7 +24,7 @@ type memDBBatch struct { var _ Batch = (*memDBBatch)(nil) -// newMemDBBatch creates a new memDBBatch +// newMemDBBatch creates a new memDBBatch. func newMemDBBatch(db *MemDB) *memDBBatch { return &memDBBatch{ db: db, diff --git a/pebble.go b/pebble.go index ed8f21a..3eb2672 100644 --- a/pebble.go +++ b/pebble.go @@ -1,6 +1,3 @@ -//go:build pebbledb -// +build pebbledb - package db import ( @@ -113,11 +110,7 @@ func (db *PebbleDB) Delete(key []byte) error { } wopts := pebble.NoSync - err := db.db.Delete(key, wopts) - if err != nil { - return err - } - return nil + return db.db.Delete(key, wopts) } // DeleteSync implements DB. @@ -125,11 +118,7 @@ func (db PebbleDB) DeleteSync(key []byte) error { if len(key) == 0 { return errKeyEmpty } - err := db.db.Delete(key, pebble.Sync) - if err != nil { - return nil - } - return nil + return db.db.Delete(key, pebble.Sync) } func (db *PebbleDB) DB() *pebble.DB { @@ -160,8 +149,7 @@ func (db *PebbleDB) Compact(start, end []byte) (err error) { if end == nil && iter.Last() { end = append(end, iter.Key()...) } - err = db.db.Compact(start, end, true) - return + return db.db.Compact(start, end, true) } // Close implements DB. diff --git a/pebble_test.go b/pebble_test.go index 17c4e8b..3898816 100644 --- a/pebble_test.go +++ b/pebble_test.go @@ -1,6 +1,3 @@ -//go:build pebbledb -// +build pebbledb - package db import ( diff --git a/prefixdb.go b/prefixdb.go index 30e29a5..bb10113 100644 --- a/prefixdb.go +++ b/prefixdb.go @@ -66,10 +66,7 @@ func (pdb *PrefixDB) Set(key []byte, value []byte) error { defer pdb.mtx.Unlock() pkey := pdb.prefixed(key) - if err := pdb.db.Set(pkey, value); err != nil { - return err - } - return nil + return pdb.db.Set(pkey, value) } // SetSync implements DB. diff --git a/test_helpers.go b/test_helpers.go index 2d03345..a6ff90c 100644 --- a/test_helpers.go +++ b/test_helpers.go @@ -28,7 +28,6 @@ MAIN_LOOP: break MAIN_LOOP } val >>= 6 - } } diff --git a/types.go b/types.go index 11b0c07..94b3736 100644 --- a/types.go +++ b/types.go @@ -21,7 +21,7 @@ var ( type DB interface { // Get fetches the value of the given key, or nil if it does not exist. // CONTRACT: key, value readonly []byte - Get([]byte) ([]byte, error) + Get(key []byte) ([]byte, error) // Has checks if a key exists. // CONTRACT: key, value readonly []byte @@ -29,17 +29,17 @@ type DB interface { // Set sets the value for the given key, replacing it if it already exists. // CONTRACT: key, value readonly []byte - Set([]byte, []byte) error + Set(key []byte, value []byte) error // SetSync sets the value for the given key, and flushes it to storage before returning. - SetSync([]byte, []byte) error + SetSync(key []byte, value []byte) error // Delete deletes the key, or does nothing if the key does not exist. // CONTRACT: key readonly []byte - Delete([]byte) error + Delete(key []byte) error // DeleteSync deletes the key, and flushes the delete to storage before returning. - DeleteSync([]byte) error + DeleteSync(key []byte) error // Iterator returns an iterator over a domain of keys, in ascending order. The caller must call // Close when done. End is exclusive, and start must be less than end. A nil start iterates diff --git a/util.go b/util.go index 7ba3a74..c7e57dd 100644 --- a/util.go +++ b/util.go @@ -14,7 +14,7 @@ func cp(bz []byte) (ret []byte) { // Returns a slice of the same length (big endian) // except incremented by one. // Returns nil on overflow (e.g. if bz bytes are all 0xFF) -// CONTRACT: len(bz) > 0 +// CONTRACT: len(bz) > 0. func cpIncr(bz []byte) (ret []byte) { if len(bz) == 0 { panic("cpIncr expects non-zero bz length")