Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

storage: add support of BadgerDB #839

Merged
merged 1 commit into from
Apr 9, 2020
Merged

Conversation

AnnaShaleva
Copy link
Member

closes #820

@AnnaShaleva AnnaShaleva self-assigned this Apr 8, 2020
@AnnaShaleva AnnaShaleva force-pushed the feature/badgerdb_support branch 2 times, most recently from bac693d to db147ba Compare April 8, 2020 14:24

// BadgerDBOptions configuration for BadgerDB.
type BadgerDBOptions struct {
Dir string `yaml:"BadgerDir"`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should use the same DataDirectoryPath as LevelDB here?

// Batch implements the Batch interface and returns a badgerdb
// compatible Batch.
func (b *BadgerDBStore) Batch() Batch {
return newMemoryBatch()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I expected to see db.NewWriteBatch() here or some wrapper around that.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or are you not using to avoid Set and Delete error processing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just didn't realized that it's possible. Fixed with wrapper around db.NewWriteBatch().

@@ -50,6 +50,8 @@ ApplicationConfiguration:
# DB: 0
# BoltDBOptions:
# FilePath: "./chains/mainnet.bolt"
# BadgerDbOptions:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Everywhere in code and in other options it is DB, not Db.

@@ -50,6 +50,8 @@ ApplicationConfiguration:
# DB: 0
# BoltDBOptions:
# FilePath: "./chains/mainnet.bolt"
# BadgerDbOptions:
# BadgerDir: "./chains/mainnet"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May we change this to ./chains/mainnet.badger like how it was done with bolt? This allows to test multiple backends simultaneously and diminish chances of unintentionally corrupting something.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bolt is a bit different in that it's a file, although I'm not sure how LevelDB would react on Badger data and vice versa, they should probably just fail to start correctly. But then again your proposal is probably a safer choice.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checked it: Badger needs a folder, not just a file. In case of ./chains/privnet.badger it creates folder privnet.badger and puts all data into that folder. So, which path should we use?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's use ${NET}.badger.

// Seek implements the Store interface.
func (b *BadgerDBStore) Seek(key []byte, f func(k, v []byte)) {
err := b.db.View(func(txn *badger.Txn) error {
it := txn.NewIterator(badger.DefaultIteratorOptions)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IteratorOptions has Prefix field. Doesn't it do what we need?
https://pkg.go.dev/github.com/dgraph-io/badger?tab=doc#IteratorOptions

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not quite, so we can't get reed of it.Seek(key) here. See https://github.com/dgraph-io/badger#prefix-scans

@AnnaShaleva AnnaShaleva force-pushed the feature/badgerdb_support branch 2 times, most recently from 7f5fde3 to 1d4af7c Compare April 9, 2020 10:08
@codecov
Copy link

codecov bot commented Apr 9, 2020

Codecov Report

Merging #839 into master will increase coverage by 0.06%.
The diff coverage is 80.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #839      +/-   ##
==========================================
+ Coverage   67.13%   67.20%   +0.06%     
==========================================
  Files         141      142       +1     
  Lines       13351    13422      +71     
==========================================
+ Hits         8963     9020      +57     
- Misses       3982     3990       +8     
- Partials      406      412       +6     
Impacted Files Coverage Δ
pkg/core/storage/store.go 34.61% <0.00%> (-2.89%) ⬇️
pkg/core/storage/badgerdb_store.go 82.60% <82.60%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 60b795f...54cdfe4. Read the comment docs.

@AnnaShaleva AnnaShaleva force-pushed the feature/badgerdb_support branch 2 times, most recently from 7cbd855 to a60c67d Compare April 9, 2020 10:49
@roman-khimov roman-khimov merged commit bfaa025 into master Apr 9, 2020
@roman-khimov roman-khimov deleted the feature/badgerdb_support branch April 9, 2020 12:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BadgerDB support
4 participants