kvrocks
kvrocks copied to clipboard
apache
Updating C++ 17 and start implementation of RocksDB 7.х
Search before asking
- [X] I had searched in the issues and found no similar issues.
Motivation
RocksDB are expressive storage engine and a lot of engineers improve it. Currently, a head version are 7.5.х and 7.6 are coming in next week. Great work, many new features, a lot of performance patch and bugs are fixed. We want it!
Solution
We are using a stable but old version 6.29.х. So, lets start to updating!
In collaboration with @torwig we have a ambitious plan to implement support of major release of RocksDB, implement API changes and new features.
A first step are:
- Updating C++ up to 17 (now we use v11). In some cases this should be not so clean (Ubuntu 16.х maybe not supported).
- Updating RocksDB up to 7.2.2 - this version haven't many breaking changes, so we integrate it without rewrite a code base.
And this is a base platform for step-by-step updating next version of RocksDB.
If our plan is OK, we submit PR and will work about next integrations.
Are you willing to submit a PR?
- [X] I'm willing to submit a PR!
@git-hulk @PragmaTwice @tisonkun Do we have any blockers or considerations that might stop us from updating RocksDb to a newer major version?
I favor updating to newer c++ standard, but it may be a more complicated task and I think you could open a separate issue to discuss the standard updating.
Things that might need to be investigated to update the c++ standard include:
- compiler support, which version of the compiler standard we need to upgrade to
- breaking changes of the newer standard and semantic changes, and their impact on kvrocks
- behavior differences of dependency libraries across standards
The main concern from my side is that C++17 may be too new for many users, and RocksDB 7.x is too radical since it should NOT have enough users now. So I prefer to keep 6.29.X and C++11 for a while before we have the strong intention to introduce some cool features in RocksDB 7.x.
In other hands - if we keep old version of RocksDB, any other migrations will be a lot of pain. Now, in our private fork, has no critical changes, all test are passed, all dependency build clean.
We can slowly migration step by step to forward version of RocksDB, but from 7.2 this is much more trivial now, than when they are going to next release like 8.х.
Maybe we can separate task - and a first prepare PR for C++17, and then look into rocksdb?
In new version binary file has 20.3Mb vs 35Mb old (6.2/C++11) while use Release building mode
@aleksraiden I suggest you start a discussion about upgrading the C++ standard in the Discussion forum. It's not an actionable task (for now) but an open-ended discussion. Also, answering in-thread should be better than quote and reply in an issue.
For info - in our fork with updates, successfully merged with current upstream RocksDB without any code changes. And work into internal evn in a week. As I think, this can be a flag for x.py to choose a version of RocksDB
A little bit bad news - for performance boost some options needs to be compiled with C++20 (concurrently read data blocks from multiple files in a level in batched MultiGet).
For info - in our fork with updates, successfully merged with current upstream RocksDB without any code changes. And work into internal evn in a week. As I think, this can be a flag for x.py to choose a version of RocksDB
I think it is hard to maintain kvrocks with multiple version of rocksdb or multiple standard in same time, so I prefer update standard/dependency version directly.
A little bit bad news - for performance boost some options needs to be compiled with C++20 (concurrently read data blocks from multiple files in a level in batched MultiGet).
Could you give some more specific information? For example, which source file/code segment that requires c++20 is involved in rocksdb.
@git-hulk @PragmaTwice can this patch be closed now? Sinces kvrocks is already using C++17 and RocksDB 7.x
@git-hulk @PragmaTwice can this patch be closed now? Sinces kvrocks is already using C++17 and RocksDB 7.x
Sure.