You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by GitBox <gi...@apache.org> on 2022/09/08 14:05:01 UTC

[GitHub] [incubator-kvrocks] aleksraiden opened a new issue, #836: Updating C++ 17 and start implementation of RocksDB 7.х

aleksraiden opened a new issue, #836:
URL: https://github.com/apache/incubator-kvrocks/issues/836

   ### Search before asking
   
   - [X] I had searched in the [issues](https://github.com/apache/incubator-kvrocks/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:
   
   1. Updating C++ up to 17 (now we use v11). In some cases this should be not so clean (Ubuntu 16.х maybe not supported).
   2. 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!


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] torwig commented on issue #836: Updating C++ 17 and start implementation of RocksDB 7.х

Posted by GitBox <gi...@apache.org>.
torwig commented on issue #836:
URL: https://github.com/apache/incubator-kvrocks/issues/836#issuecomment-1240774233

   @git-hulk @PragmaTwice @tisonkun Do we have any blockers or considerations that might stop us from updating RocksDb to a newer major version?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] aleksraiden commented on issue #836: Updating C++ 17 and start implementation of RocksDB 7.х

Posted by GitBox <gi...@apache.org>.
aleksraiden commented on issue #836:
URL: https://github.com/apache/incubator-kvrocks/issues/836#issuecomment-1253494862

   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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] mapleFU commented on issue #836: Updating C++ 17 and start implementation of RocksDB 7.х

Posted by GitBox <gi...@apache.org>.
mapleFU commented on issue #836:
URL: https://github.com/apache/incubator-kvrocks/issues/836#issuecomment-1303316099

   @git-hulk @PragmaTwice can this patch be closed now? Sinces kvrocks is already using C++17 and RocksDB 7.x


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice closed issue #836: Updating C++ 17 and start implementation of RocksDB 7.х

Posted by GitBox <gi...@apache.org>.
PragmaTwice closed issue #836: Updating C++ 17 and start implementation of RocksDB 7.х
URL: https://github.com/apache/incubator-kvrocks/issues/836


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on issue #836: Updating C++ 17 and start implementation of RocksDB 7.х

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on issue #836:
URL: https://github.com/apache/incubator-kvrocks/issues/836#issuecomment-1303383531

   > @git-hulk @PragmaTwice can this patch be closed now? Sinces kvrocks is already using C++17 and RocksDB 7.x
   
   Sure.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] aleksraiden commented on issue #836: Updating C++ 17 and start implementation of RocksDB 7.х

Posted by GitBox <gi...@apache.org>.
aleksraiden commented on issue #836:
URL: https://github.com/apache/incubator-kvrocks/issues/836#issuecomment-1240860552

   Good idea, thanks @tisonkun. lets go to Discussion, @torwig 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on issue #836: Updating C++ 17 and start implementation of RocksDB 7.х

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on issue #836:
URL: https://github.com/apache/incubator-kvrocks/issues/836#issuecomment-1255971713

   > 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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] aleksraiden commented on issue #836: Updating C++ 17 and start implementation of RocksDB 7.х

Posted by GitBox <gi...@apache.org>.
aleksraiden commented on issue #836:
URL: https://github.com/apache/incubator-kvrocks/issues/836#issuecomment-1240856692

   In new version binary file has 20.3Mb vs 35Mb old (6.2/C++11) while use Release building mode


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] aleksraiden commented on issue #836: Updating C++ 17 and start implementation of RocksDB 7.х

Posted by GitBox <gi...@apache.org>.
aleksraiden commented on issue #836:
URL: https://github.com/apache/incubator-kvrocks/issues/836#issuecomment-1240804114

   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?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] git-hulk commented on issue #836: Updating C++ 17 and start implementation of RocksDB 7.х

Posted by GitBox <gi...@apache.org>.
git-hulk commented on issue #836:
URL: https://github.com/apache/incubator-kvrocks/issues/836#issuecomment-1240789598

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] aleksraiden commented on issue #836: Updating C++ 17 and start implementation of RocksDB 7.х

Posted by GitBox <gi...@apache.org>.
aleksraiden commented on issue #836:
URL: https://github.com/apache/incubator-kvrocks/issues/836#issuecomment-1255958014

   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). 


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on issue #836: Updating C++ 17 and start implementation of RocksDB 7.х

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on issue #836:
URL: https://github.com/apache/incubator-kvrocks/issues/836#issuecomment-1256107413

   > 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 code is involved in rocksdb.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] PragmaTwice commented on issue #836: Updating C++ 17 and start implementation of RocksDB 7.х

Posted by GitBox <gi...@apache.org>.
PragmaTwice commented on issue #836:
URL: https://github.com/apache/incubator-kvrocks/issues/836#issuecomment-1240789124

   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 to the new standard and semantic changes, and their impact on kvrocks
   - behavior differences of dependency libraries across standards


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


[GitHub] [incubator-kvrocks] tisonkun commented on issue #836: Updating C++ 17 and start implementation of RocksDB 7.х

Posted by GitBox <gi...@apache.org>.
tisonkun commented on issue #836:
URL: https://github.com/apache/incubator-kvrocks/issues/836#issuecomment-1240858942

   @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 and issue.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org