add boltdb for storage #3604

Lawliet-Chan · 2019-04-30T00:57:37Z

add boltdb implement for db interface.

The boltdb is safe and high performence embedded KV database. It is based on B+tree and Mmap.
Now it is maintained by etcd-io org. https://github.com/etcd-io/bbolt
It is much better than LSM tree (levelDB) when RANDOM and SCAN read.

golangcibot · 2019-04-30T00:58:39Z

libs/db/boltdb_test.go

+	require.Nil(t, err)
+}
+
+func BenchmarkBoltdbRandomReadsWrites(b *testing.B) {


34-95 lines are duplicate of libs/db/go_level_db_test.go:35-96 (from dupl)

I think that boltdb and leveldb they are both kv database. So it should be tested in the same case. Is it right?

We can probably refactor the tests to benchmark them via t.Run to remove code duplication. Can happen in a follow-up PR too

Thank you for your reply.
In my opinion, the benchmark cases should be the same so that we can see which database performs better .

libs/db/boltdb.go

melekes

Awesome! This is a great start @CrocdileChan 👍 Added a note about upgrading existing deps and naming suggestions. We should also write more tests.

Gopkg.lock

libs/db/boltdb.go

melekes · 2019-04-30T06:52:49Z

libs/db/boltdb.go

+var bucket = []byte("boltdb_bucket")
+
+func init() {
+	registerDBCreator(BoltDBBackend, func(name string, dir string) (DB, error) {


Suggested change

registerDBCreator(BoltDBBackend, func(name string, dir string) (DB, error) {

registerDBCreator(BoltDBBackend, func(name, dir string) (DB, error) {

libs/db/boltdb.go

melekes · 2019-04-30T06:59:01Z

libs/db/boltdb.go

+	bdb.Write()
+}
+
+func (bdb *BoltdbBatch) Close() {}


Do you think Close should clean bdb.buffer?

OH, I got your point.
The golang has GC and the bdb.buffer is only a *sync.map.
It won't return memory to the operating system even though do bdb.buffer=nil. Actually, it is the same op in go_level_db.go
So it is fine now.

liamsi · 2019-04-30T09:11:13Z

libs/db/boltdb.go

@@ -0,0 +1,253 @@
+package db


Not blocking this PR but shouldn't we create a package per implementation (bolt, golevel, mem, etc)?

Yes to ^. We should also use conditional compilation to only include deps & code when X db is requested (// +build boltdb).

libs/db/boltdb.go

Lawliet-Chan · 2019-05-01T03:07:53Z

I've requested a PR yesterday 490981f.
Please review it :-p
Can it be merged?

Thanks a lot :-p

melekes · 2019-05-01T04:50:29Z

Can it be merged?

Of course! But before we do that could you please address the above comments.

Lawliet-Chan

Can it be merged?

Of course! But before we do that could you please address the above comments.

#3604 (comment)

#3604 (comment)

#3604 (comment)

OK, but I don't know why and how to modify the boltdb_test.go. I think the benchmark cases should be the same.
If you want me to modify the benchmark case, please teach me how to do it.
Thank you very much indeed.

Lawliet-Chan · 2019-04-30T01:12:39Z

libs/db/boltdb_test.go

+	require.Nil(t, err)
+}
+
+func BenchmarkBoltdbRandomReadsWrites(b *testing.B) {


I think that boltdb and leveldb they are both kv database. So it should be tested in the same case. Is it right?

libs/db/boltdb.go

Lawliet-Chan · 2019-04-30T08:44:16Z

libs/db/boltdb.go

+	bdb.Write()
+}
+
+func (bdb *BoltdbBatch) Close() {}


OH, I got your point.
The golang has GC and the bdb.buffer is only a *sync.map.
It won't return memory to the operating system even though do bdb.buffer=nil. Actually, it is the same op in go_level_db.go
So it is fine now.

libs/db/boltdb.go

Lawliet-Chan · 2019-04-30T10:21:59Z

libs/db/boltdb_test.go

+	require.Nil(t, err)
+}
+
+func BenchmarkBoltdbRandomReadsWrites(b *testing.B) {


Thank you for your reply.
In my opinion, the benchmark cases should be the same so that we can see which database performs better .

melekes · 2019-05-01T08:37:49Z

If you want me to modify the benchmark case, please teach me how to do it.

No problem. I can work on this.

melekes · 2019-05-01T09:52:30Z

Closing in favor of #3610

Description: add boltdb implement for db interface. The boltdb is safe and high performence embedded KV database. It is based on B+tree and Mmap. Now it is maintained by etcd-io org. https://github.com/etcd-io/bbolt It is much better than LSM tree (levelDB) when RANDOM and SCAN read. Replaced #3604 Refs #1835 Commits: * add boltdb for storage * update boltdb in gopkg.toml * update bbolt in Gopkg.lock * dep update Gopkg.lock * add boltdb'bucket when create Boltdb * some modifies * address my own comments * fix most of the Iterator tests for boltdb * bolt does not support concurrent writes while iterating * add WARNING word * note that ReadOnly option is not supported * fixes after Ismail's review Co-Authored-By: melekes <anton.kalyaev@gmail.com> * panic if View returns error plus specify bucket in the top comment

Description: add boltdb implement for db interface. The boltdb is safe and high performence embedded KV database. It is based on B+tree and Mmap. Now it is maintained by etcd-io org. https://github.com/etcd-io/bbolt It is much better than LSM tree (levelDB) when RANDOM and SCAN read. Replaced tendermint#3604 Refs tendermint#1835 Commits: * add boltdb for storage * update boltdb in gopkg.toml * update bbolt in Gopkg.lock * dep update Gopkg.lock * add boltdb'bucket when create Boltdb * some modifies * address my own comments * fix most of the Iterator tests for boltdb * bolt does not support concurrent writes while iterating * add WARNING word * note that ReadOnly option is not supported * fixes after Ismail's review Co-Authored-By: melekes <anton.kalyaev@gmail.com> * panic if View returns error plus specify bucket in the top comment

add boltdb for storage

bccbcc5

Lawliet-Chan requested review from ebuchman, melekes and xla as code owners April 30, 2019 00:57

golangcibot reviewed Apr 30, 2019

View reviewed changes

CrocdileChan added 4 commits April 30, 2019 10:28

update boltdb in gopkg.toml

d39c111

update bbolt in Gopkg.lock

2414201

dep update Gopkg.lock

8ba4913

add boltdb'bucket when create Boltdb

b6e767a

melekes suggested changes Apr 30, 2019

View reviewed changes

melekes changed the base branch from master to develop April 30, 2019 07:38

liamsi reviewed Apr 30, 2019

View reviewed changes

libs/db/boltdb.go Show resolved Hide resolved

some modifies

490981f

Lawliet-Chan commented May 1, 2019

View reviewed changes

fix some comments

9b67a0a

melekes mentioned this pull request May 1, 2019

bbolt (etcd's fork of bolt) #3610

Merged

4 tasks

melekes closed this May 1, 2019

melekes mentioned this pull request May 1, 2019

libs/db: conditional compilation #3611

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add boltdb for storage #3604

add boltdb for storage #3604

Lawliet-Chan commented Apr 30, 2019

golangcibot Apr 30, 2019

Lawliet-Chan Apr 30, 2019

liamsi Apr 30, 2019

Lawliet-Chan Apr 30, 2019

melekes left a comment

melekes Apr 30, 2019

melekes Apr 30, 2019 •

edited

Lawliet-Chan Apr 30, 2019

liamsi Apr 30, 2019

melekes Apr 30, 2019

melekes May 1, 2019

Lawliet-Chan commented May 1, 2019

melekes commented May 1, 2019 •

edited

Lawliet-Chan left a comment

Lawliet-Chan Apr 30, 2019

Lawliet-Chan Apr 30, 2019

Lawliet-Chan Apr 30, 2019

melekes commented May 1, 2019

melekes commented May 1, 2019

	registerDBCreator(BoltDBBackend, func(name string, dir string) (DB, error) {
	registerDBCreator(BoltDBBackend, func(name, dir string) (DB, error) {

add boltdb for storage #3604

add boltdb for storage #3604

Conversation

Lawliet-Chan commented Apr 30, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

melekes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

melekes Apr 30, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Lawliet-Chan commented May 1, 2019

melekes commented May 1, 2019 • edited

Lawliet-Chan left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

melekes commented May 1, 2019

melekes commented May 1, 2019

melekes Apr 30, 2019 •

edited

melekes commented May 1, 2019 •

edited