[GitHub] [incubator-doris] acelyc111 commented on a change in pull request #5158: [Config] change some static config to dynamic config and delete some unused config

[GitBox <gi...@apache.org>]

acelyc111 commented on a change in pull request #5158:
URL: https://github.com/apache/incubator-doris/pull/5158#discussion_r549598924



##########
File path: be/src/common/config.h
##########
@@ -424,8 +378,7 @@ CONF_Bool(enable_quadratic_probing, "false");
 CONF_String(pprof_profile_dir, "${DORIS_HOME}/log");
 
 // for partition
-// CONF_Bool(enable_partitioned_hash_join, "false")
-CONF_Bool(enable_partitioned_aggregation, "true");
+CONF_mBool(enable_partitioned_aggregation, "true");

Review comment:
       There is a memory leak risk, `enable_partitioned_aggregation` is true at first and then changed to false, memory allocated has no chance to free.

##########
File path: be/src/common/config.h
##########
@@ -472,7 +412,7 @@ CONF_String(buffer_pool_clean_pages_limit, "20G");
 CONF_mInt64(memory_maintenance_sleep_time_s, "10");
 
 // Alignment
-CONF_Int32(memory_max_alignment, "16");
+CONF_mInt32(memory_max_alignment, "16");

Review comment:
       I think you can remove this config, and define it as a static const value, there is no reason to update it. 

##########
File path: be/src/common/config.h
##########
@@ -256,21 +222,18 @@ CONF_Int32(file_descriptor_cache_capacity, "32768");
 // modify them upon necessity
 CONF_Int32(min_file_descriptor_number, "60000");
 CONF_Int64(index_stream_cache_capacity, "10737418240");
-// CONF_Int64(max_packed_row_block_size, "20971520");
 
 // Cache for storage page size
 CONF_String(storage_page_cache_limit, "20%");
 // whether to disable page cache feature in storage
-CONF_Bool(disable_storage_page_cache, "false");
+CONF_mBool(disable_storage_page_cache, "false");

Review comment:
       There is a case that `disable_storage_page_cache ` is true at first, then changed to false, will the memory used by cache has no chance to free?

##########
File path: be/src/common/config.h
##########
@@ -482,8 +422,8 @@ CONF_mInt64(write_buffer_size, "104857600");
 // NOTICE(cmy): set these default values very large because we don't want to
 // impact the load performance when user upgrading Doris.
 // user should set these configs properly if necessary.
-CONF_Int64(load_process_max_memory_limit_bytes, "107374182400"); // 100GB
-CONF_Int32(load_process_max_memory_limit_percent, "80");         // 80%
+CONF_mInt64(load_process_max_memory_limit_bytes, "107374182400"); // 100GB
+CONF_mInt32(load_process_max_memory_limit_percent, "80");         // 80%

Review comment:
       These 2 configs are only changed in LoadChannelMgr's init function, which will only be called only once when server start, they are immutable.




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