[GitHub] [incubator-pegasus] acelyc111 commented on a diff in pull request #1108: fix: rocksdb options not changed even if update in Pegasus config file

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

acelyc111 commented on code in PR #1108:
URL: https://github.com/apache/incubator-pegasus/pull/1108#discussion_r942289900


##########
src/server/pegasus_server_impl.h:
##########
@@ -325,6 +328,15 @@ class pegasus_server_impl : public pegasus_read_service
         return dsn::rand::next_u64(base_value - gap, base_value + gap);
     }
 
+    // return true if value in range of [0.75, 1.25] * base_value
+    bool check_value_if_nearby(uint64_t base_value, uint64_t check_value)
+    {
+        uint64_t gap = base_value / 4;
+        uint64_t actual_gap =
+            (base_value < check_value) ? check_value - base_value : base_value - check_value;

Review Comment:
   std::abs(check_value - base_value);



##########
src/server/pegasus_server_impl.cpp:
##########
@@ -2634,6 +2634,10 @@ void pegasus_server_impl::update_usage_scenario(const std::map<std::string, std:
                            old_usage_scenario,
                            new_usage_scenario);
         }
+    } else {
+        // When an old db is opened and the conf is changed, the options related to usage scenario
+        // need to be recalculated with new values.
+        recalculate_usage_scenario(_tmp_data_cf_opts);

Review Comment:
   Usage scenario is not changed, maybe naming as recalculate_cf_options?



##########
src/server/pegasus_server_impl.cpp:
##########
@@ -3007,6 +3011,112 @@ void pegasus_server_impl::reset_usage_scenario_options(
     target_opts->max_write_buffer_number = base_opts.max_write_buffer_number;
 }
 
+void pegasus_server_impl::recalculate_usage_scenario(const rocksdb::ColumnFamilyOptions &cur_opts)
+{
+    std::unordered_map<std::string, std::string> new_options;
+    if (ROCKSDB_ENV_USAGE_SCENARIO_NORMAL == _usage_scenario ||
+        ROCKSDB_ENV_USAGE_SCENARIO_PREFER_WRITE == _usage_scenario) {
+        if (ROCKSDB_ENV_USAGE_SCENARIO_NORMAL == _usage_scenario) {
+            if (!check_value_if_nearby(_data_cf_opts.write_buffer_size,
+                                       cur_opts.write_buffer_size)) {
+                new_options["write_buffer_size"] =
+                    std::to_string(get_random_nearby(_data_cf_opts.write_buffer_size));
+            }

Review Comment:
   There are many duplicate code, use a macro here will be easier to read.
   ```
   #define UPDATE_OPTION_IF_NEEDED(option, new_value) \
     ...
   ```
   `#` and `##` can be used in this macro.



-- 
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: dev-unsubscribe@pegasus.apache.org

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


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@pegasus.apache.org
For additional commands, e-mail: dev-help@pegasus.apache.org