You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2021/04/03 08:20:45 UTC

[GitHub] [incubator-doris] acelyc111 opened a new pull request #5597: [Enhance] Support to configure RocksDB to improve tablet meta store

acelyc111 opened a new pull request #5597:
URL: https://github.com/apache/incubator-doris/pull/5597


   ## Proposed changes
   
   Supported options:
   - rocksdb_thread_count: Thread count of RocksDB uses for background flush and compaction
   - rocksdb_block_cache_mb: Block cache size
   - rocksdb_compression_type: compression type, now support LZ4 and snappy
   
   And also do some refactor:
   - Remove some  deprecated meta store related code which never used now
   - Call OptimizeForPointLookup() to optimize options include bloom filter and block cache
   
   ## Types of changes
   
   What types of changes does your code introduce to Doris?
   _Put an `x` in the boxes that apply_
   
   - [ ] Bugfix (non-breaking change which fixes an issue)
   - [] New feature (non-breaking change which adds functionality)
   - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
   - [ ] Documentation Update (if none of the other choices apply)
   - [x] Code refactor (Modify the code structure, format the code, etc...)
   
   ## Checklist
   
   _Put an `x` in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code._
   
   - [ ] I have created an issue on (Fix #ISSUE) and described the bug/feature there in detail
   - [x] Compiling and unit tests pass locally with my changes
   - [ ] I have added tests that prove my fix is effective or that my feature works
   - [ ] If these changes need document changes, I have updated the document
   - [ ] Any dependent changes have been merged
   


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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] acelyc111 commented on a change in pull request #5597: [Enhance] Support to configure RocksDB to improve tablet meta store

Posted by GitBox <gi...@apache.org>.
acelyc111 commented on a change in pull request #5597:
URL: https://github.com/apache/incubator-doris/pull/5597#discussion_r608305362



##########
File path: be/src/common/config.h
##########
@@ -516,10 +514,23 @@ CONF_Int32(flush_thread_num_per_store, "2");
 // config for tablet meta checkpoint
 CONF_mInt32(tablet_meta_checkpoint_min_new_rowsets_num, "10");
 CONF_mInt32(tablet_meta_checkpoint_min_interval_secs, "600");
+// Thread count of RocksDB uses for background flush and compaction, -1 means the number of cores.
+CONF_mInt32(rocksdb_thread_count, "-1");
+CONF_mInt32(rocksdb_block_cache_mb, "1024");
+CONF_Validator(rocksdb_block_cache_mb, [](int value) -> bool {
+  return value > 0;
+});
+CONF_String(rocksdb_compression_type, "LZ4");

Review comment:
       > Use options.compression to specify the compression to use. By default it is Snappy. We believe LZ4 is almost always better than Snappy. We leave Snappy as default to avoid unexpected compatibility problems to previous users.
   
   https://github.com/facebook/rocksdb/wiki/Compression
   
   We encountered a problem that snappy decompress cost too much time in product environment.
   Of course, the root cause is that the rocksdb value is too large, some tablet meta which have thounds of rowsets may reach hundreds of MB. But I think we can improve it by use LZ4.




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #5597: [Enhance] Support to configure RocksDB to improve tablet meta store

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #5597:
URL: https://github.com/apache/incubator-doris/pull/5597#discussion_r608290139



##########
File path: be/src/common/config.h
##########
@@ -516,10 +514,23 @@ CONF_Int32(flush_thread_num_per_store, "2");
 // config for tablet meta checkpoint
 CONF_mInt32(tablet_meta_checkpoint_min_new_rowsets_num, "10");
 CONF_mInt32(tablet_meta_checkpoint_min_interval_secs, "600");
+// Thread count of RocksDB uses for background flush and compaction, -1 means the number of cores.
+CONF_mInt32(rocksdb_thread_count, "-1");
+CONF_mInt32(rocksdb_block_cache_mb, "1024");
+CONF_Validator(rocksdb_block_cache_mb, [](int value) -> bool {
+  return value > 0;
+});
+CONF_String(rocksdb_compression_type, "LZ4");

Review comment:
       What is the difference between LZ4 and snappy? Is it necessary to config this?

##########
File path: be/src/common/config.h
##########
@@ -516,10 +514,23 @@ CONF_Int32(flush_thread_num_per_store, "2");
 // config for tablet meta checkpoint
 CONF_mInt32(tablet_meta_checkpoint_min_new_rowsets_num, "10");
 CONF_mInt32(tablet_meta_checkpoint_min_interval_secs, "600");
+// Thread count of RocksDB uses for background flush and compaction, -1 means the number of cores.
+CONF_mInt32(rocksdb_thread_count, "-1");
+CONF_mInt32(rocksdb_block_cache_mb, "1024");

Review comment:
       Is 1024MB the default value of rocksdb cache? Maybe it is too big?
   BTW, `rocksdb_thread_count` and `rocksdb_block_cache_mb` are not mutable configuration, use `CONF_Int32` instead.




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] acelyc111 commented on a change in pull request #5597: [Enhance] Support to configure RocksDB to improve tablet meta store

Posted by GitBox <gi...@apache.org>.
acelyc111 commented on a change in pull request #5597:
URL: https://github.com/apache/incubator-doris/pull/5597#discussion_r608370908



##########
File path: be/src/common/config.h
##########
@@ -516,10 +514,23 @@ CONF_Int32(flush_thread_num_per_store, "2");
 // config for tablet meta checkpoint
 CONF_mInt32(tablet_meta_checkpoint_min_new_rowsets_num, "10");
 CONF_mInt32(tablet_meta_checkpoint_min_interval_secs, "600");
+// Thread count of RocksDB uses for background flush and compaction, -1 means the number of cores.
+CONF_mInt32(rocksdb_thread_count, "-1");
+CONF_mInt32(rocksdb_block_cache_mb, "1024");

Review comment:
       The default cache is 8M which is too small.
   I've updated code to make the block cache shared by all rocksdb instances, and it's capacity is 64M now.




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

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



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org