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

Use RocksDB? #966

Closed
lock9 opened this issue Jul 30, 2019 · 35 comments · Fixed by neo-project/neo-modules#144
Closed

Use RocksDB? #966

lock9 opened this issue Jul 30, 2019 · 35 comments · Fixed by neo-project/neo-modules#144
Assignees
Labels
enhancement Type - Changes that may affect performance, usability or add new features to existing modules. ledger Module - The ledger is our 'database', this is used to tag changes about how we store information ready-to-implement Issue state: Ready to be implemented or implementation in progress

Comments

@lock9
Copy link
Contributor

lock9 commented Jul 30, 2019

Should we use RocksDB instead of LevelDB?

@lock9 lock9 added the discussion Initial issue state - proposed but not yet accepted label Jul 30, 2019
@shargon
Copy link
Member

shargon commented Jul 30, 2019

@lock9
Copy link
Contributor Author

lock9 commented Jul 30, 2019

@shargon is there any security or consistency problem we could have by using that?

@erikzhang
Copy link
Member

Is there a .NET Standard library for RocksDB?

@shargon
Copy link
Member

shargon commented Jul 31, 2019

This nuget is for dotnet https://github.com/warrenfalk/rocksdb-sharp

@shargon
Copy link
Member

shargon commented Jul 31, 2019

@shargon is there any security or consistency problem we could have by using that?

No, and maybe is better because now we are using rocksdb in mac, this unify all the storage layer

@Tommo-L
Copy link
Contributor

Tommo-L commented Aug 1, 2019

There is a inconsistency bug before leveldb 1.22, google/leveldb#320.
But it has been fixed in rocksdb long long ago.

@shargon
Copy link
Member

shargon commented Aug 3, 2019

it is already decided? @neo-project/core

@erikzhang
Copy link
Member

Not yet.

@igormcoelho
Copy link
Contributor

igormcoelho commented Aug 3, 2019

For me, if it's safe enough to use natively on nugets, it's better than current one (easier to use, test and maintain). So I agree.

Too many code changes are expected on this DB "migration", or just few? During these changes, it would be nice to leave things more flexible, so that on the future, it becomes even easier to change that (mainly to support memory-only databases).

@lightszero
Copy link
Member

Rocksdb is developed base on leveldb,so it is very easy to use it for NEO.

@shargon
Copy link
Member

shargon commented Aug 4, 2019

Take into account that rocksDb only works on x64 computers

@vncoelho
Copy link
Member

vncoelho commented Aug 4, 2019

@jsolman, what is your opinion?

@jsolman
Copy link
Contributor

jsolman commented Aug 4, 2019

There is nothing wrong with using LevelDB as long as using the release that fixes the compaction bug mentioned earlier.

@lock9
Copy link
Contributor Author

lock9 commented Aug 5, 2019

It looks like the majority of people are either in favor of this or at least "not against".
I like this because it will allow me to run neo-cli in MacOS

@lock9 lock9 added this to Preview2 - 27/09/2019 in NEO 3 Releases Aug 7, 2019
@Qiao-Jin
Copy link
Contributor

Qiao-Jin commented Aug 9, 2019

Some leveldb bugs might probably occur in NEO like google/leveldb#320, but don't exist in ricksdb.
Rocksdb also provides better write efficiency & functionalities.
Moreover, rocksdb's API is very similiar to leveldb, which makes it convinient to transfer from leveldb to rocksdb.
I have tried the following experiment in Neo system:

  1. Directly convert .ldb files to .sst ones by modifying file extension.
  2. Use rocksdb's library in Native.cs funtions instead of leveldb's.

Node can be started without any problem.

P.s.
Rocksdb's API: https://github.com/facebook/rocksdb/blob/master/include/rocksdb/c.h

Migrating from LevelDB to RocksDB: https://rocksdb.org/blog/2015/01/16/migrating-from-leveldb-to-rocksdb-2.html

Benchmarking LevelDB vs. RocksDB vs. HyperLevelDB vs. LMDB Performance for InfluxDB: https://www.influxdata.com/blog/benchmarking-leveldb-vs-rocksdb-vs-hyperleveldb-vs-lmdb-performance-for-influxdb/

@Qiao-Jin
Copy link
Contributor

Qiao-Jin commented Aug 9, 2019

Using rocksdb might also be a solution for this issue as I commented: #597 (comment)

@lock9
Copy link
Contributor Author

lock9 commented Aug 9, 2019

@neo-project/core we don't have a voting system in place yet, but it seems that we have a large majority of people that vote for RocksDB. Can I move this to 'ready to implement'?

@lock9 lock9 added enhancement Type - Changes that may affect performance, usability or add new features to existing modules. ledger Module - The ledger is our 'database', this is used to tag changes about how we store information labels Aug 9, 2019
@shargon
Copy link
Member

shargon commented Aug 9, 2019

Rocksdb works only on x64, maybe we should be able to work with both of them, and switch by configuration

@lock9
Copy link
Contributor Author

lock9 commented Aug 13, 2019

@shargon maybe this is a good option to keep compatibility.
@neo-project/core Can we move this forward? I think we have sufficient votes and a solution with backward compatibility

@lock9
Copy link
Contributor Author

lock9 commented Aug 20, 2019

@neo-project/core Can we move this to ready-to-implement? This solves a lot of problems. Thanks.

@shargon shargon added ready-to-implement Issue state: Ready to be implemented or implementation in progress and removed discussion Initial issue state - proposed but not yet accepted labels Aug 27, 2019
@shargon shargon self-assigned this Aug 27, 2019
@shargon shargon mentioned this issue Aug 27, 2019
3 tasks
@shargon
Copy link
Member

shargon commented Aug 27, 2019

I think that we should use column families intead of prefixes, what do you think @neo-project/core
https://github.com/facebook/rocksdb/wiki/Column-Families#targetText=Each%20key-value%20pair%20in,to%20logically%20partition%20the%20database.

@vncoelho
Copy link
Member

@shargon, I think that it was not yet decided to migrate to RocksDB.

@jsolman and @erikzhang did not show appreciation for this migration.
I also did not really investigated this enough. I search for some benchmarks and did not get the real feeling that this would be better.

@igormcoelho, can you please double check this with someone specialized in DBs?

@shargon
Copy link
Member

shargon commented Aug 27, 2019

The last comment is from 7 days ago

@neo-project/core Can we move this to ready-to-implement? This solves a lot of problems. Thanks.

And there are no more opinions until my pull request 😕

@vncoelho
Copy link
Member

I understand, @shargon, but I think that the police is not "silence is consent"...ahauehau

I do not know, I think that this needs a better discussion. Unless you are sure that RocksDB is going to have a better performance and other needed features.

@vncoelho
Copy link
Member

@shargon, I really think that we need to move in a direction in which we make a chance for it to become more flexible, not really swap LevelDB for RockDB, but just create an option for switching it.

@shargon
Copy link
Member

shargon commented Aug 27, 2019

Now it's switchable neo-project/neo-node#451

@vncoelho
Copy link
Member

vncoelho commented Aug 27, 2019

Even with Column Families? Nice job. It looks like it will be more readable.
Edited: I saw it is now isolated.

@shargon
Copy link
Member

shargon commented Aug 27, 2019

I think that column families could help to save space, 1 byte per entry, and also, will save a lot of concatenations

@erikzhang
Copy link
Member

erikzhang commented Aug 27, 2019

Can we use RocksDb on x64 and LevelDb on x86? Or should we just abandon x86 since there is no 32 bits computer on the world any more.

@shargon
Copy link
Member

shargon commented Aug 27, 2019

Yes, we can change by config, or made LevelDB mandatory on x86.

Configuration: default -> RocksDB

if the system is x86 -> use LevelDB mandatory

@shargon
Copy link
Member

shargon commented Aug 27, 2019

@vncoelho
Copy link
Member

vncoelho commented Aug 28, 2019

I saw this benchmark before, it is almost the only one I really found in favor of RocksDB, I read other discussions in favor of leveldb but without tests.

But this benchmark is not very well experimentally designed, in my point of view, I usually do not consider this as a final answer, that is why I am still not convinced that it is really more efficient for our case. However, I have faith it will.

It will be good that we can switch by any of them, then, anyone can just which one to use, @shargon.
In addition, that idea of just running on local memory will also make us design something each time more flexible.
I believe that your implementation will be good and open such doors.

@erikzhang
Copy link
Member

I have another option: FASTER. https://github.com/microsoft/FASTER

FASTER is a concurrent key-value store + cache that is designed for point lookups and heavy updates. FASTER supports data larger than memory, by leveraging fast external storage. It also supports consistent recovery using a new checkpointing technique that lets applications trade-off performance for commit latency.

Detailed analysis of C# FASTER performance: https://github.com/Microsoft/FASTER/wiki/Performance-of-FASTER-in-C%23

FASTER is implemented in two languages: C# and C++.

The performance of the C# and C++ versions of FASTER are very similar.

@neo-project/core Let's consider it.

@shargon
Copy link
Member

shargon commented Sep 2, 2019

I considered it in the past and i think that @PeterLinX discarded it after his research. But I will review it again, maybe now is better!

@lightszero
Copy link
Member

lightszero commented Sep 2, 2019

how about try to build a x86 version rocksdb?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Type - Changes that may affect performance, usability or add new features to existing modules. ledger Module - The ledger is our 'database', this is used to tag changes about how we store information ready-to-implement Issue state: Ready to be implemented or implementation in progress
Projects
NEO 3 Releases
Preview2 - 27/09/2019
Development

Successfully merging a pull request may close this issue.

9 participants