morningman commented on code in PR #28640:
URL: https://github.com/apache/doris/pull/28640#discussion_r1432726349
##########
be/src/common/config.cpp:
##########
@@ -235,8 +236,14 @@ DEFINE_Bool(doris_enable_scanner_thread_pool_per_disk,
"true");
DEFINE_mInt64(doris_blocking_priority_queue_wait_timeout_ms, "500");
// number of scanner thread pool size for olap table
// and the min thread num of remote scanner thread pool
-DEFINE_Int32(doris_scanner_thread_pool_thread_num, "48");
-DEFINE_Int32(doris_max_remote_scanner_thread_pool_thread_num, "-1");
+DEFINE_Int32(doris_scanner_thread_pool_thread_num, "-1");
+DEFINE_Validator(doris_scanner_thread_pool_thread_num, [](const int config) ->
bool {
+ if (config == -1) {
+ CpuInfo::init();
+ doris_scanner_thread_pool_thread_num = std::max(48,
CpuInfo::num_cores() * 4);
Review Comment:
std::max ?
##########
be/src/io/fs/buffered_reader.h:
##########
@@ -131,8 +131,8 @@ class MergeRangeFileReader : public io::FileReader {
static constexpr size_t READ_SLICE_SIZE = 8 * 1024 * 1024; // 8MB
static constexpr size_t BOX_SIZE = 1 * 1024 * 1024; // 1MB
static constexpr size_t SMALL_IO = 2 * 1024 * 1024; // 2MB
- static constexpr size_t HDFS_MIN_IO_SIZE = 4 * 1024; // 4KB
- static constexpr size_t OSS_MIN_IO_SIZE = 512 * 1024; // 512KB
+ static constexpr size_t HDFS_MIN_IO_SIZE = 8 * 1024; // 8KB
Review Comment:
How about make it a config? I am not sure it is suitable for all cases
##########
be/src/common/config.h:
##########
@@ -283,10 +283,7 @@ DECLARE_Bool(doris_enable_scanner_thread_pool_per_disk);
DECLARE_mInt64(doris_blocking_priority_queue_wait_timeout_ms);
// number of scanner thread pool size for olap table
// and the min thread num of remote scanner thread pool
-DECLARE_Int32(doris_scanner_thread_pool_thread_num);
-// max number of remote scanner thread pool size
-// if equal to -1, value is std::max(512, CpuInfo::num_cores() * 10)
-DECLARE_Int32(doris_max_remote_scanner_thread_pool_thread_num);
+DECLARE_mInt32(doris_scanner_thread_pool_thread_num);
Review Comment:
I suggest not removing the old config, for compatibility.
Because there may some user who set these configs already, we have to make
sure it can keep same value after upgrade
##########
be/src/vec/exec/scan/scanner_scheduler.cpp:
##########
@@ -123,9 +123,7 @@ Status ScannerScheduler::init(ExecEnv* env) {
config::doris_scanner_thread_pool_queue_size, "local_scan");
// 3. remote scan thread pool
- _remote_thread_pool_max_size =
config::doris_max_remote_scanner_thread_pool_thread_num != -1
- ?
config::doris_max_remote_scanner_thread_pool_thread_num
Review Comment:
`std::max(512, CpuInfo::num_cores() * 10)`
this value is for object storage, think it twice.
--
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: commits-unsubscr...@doris.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org
For additional commands, e-mail: commits-h...@doris.apache.org
