From 78080acea45449f42e473a8fb3fa838d1b4f53c0 Mon Sep 17 00:00:00 2001
From: Pengfan Lu <lu_peng_fan@163.com>
Date: Mon, 29 May 2023 05:09:36 +0000
Subject: [PATCH 01/10] refactor:Feature:Online Query and Dynamic Modification
 of Table-level RocksDB Options (#1488)

https://github.com/apache/incubator-pegasus/issues/1488

Complete the dynamic setting function of num_levels and write_buffer_size option.I use rocksdb.write_buffer_size as a dynamically modifiable parameter and rocksdb.num_levels as a non-dynamically modifiable parameter to test my idea.

Pegasus shell case:
create lpf -e rocksdb.num_levels=12,rocksdb.write_buffer_size=100
create lpf -e rocksdb.num_levels=5,rocksdb.write_buffer_size=100
create lpf -e rocksdb.num_levels=5,rocksdb.write_buffer_size=33554432
set_app_envs rocksdb.write_buffer_size  67108864
set_app_envs rocksdb.num_levels 4
get_app_envs
---
 src/base/pegasus_const.cpp              | 11 +++++
 src/base/pegasus_const.h                |  9 ++++
 src/common/replica_envs.h               |  7 +++
 src/common/replication_common.cpp       |  9 ++++
 src/meta/app_env_validator.cpp          | 55 ++++++++++++++++++++++-
 src/meta/app_env_validator.h            |  2 +
 src/meta/server_state.cpp               |  9 ++++
 src/meta/test/meta_app_envs_test.cpp    | 16 +++++++
 src/server/pegasus_server_impl.cpp      | 58 +++++++++++++++++++++++++
 src/server/pegasus_server_impl.h        |  7 +++
 src/server/pegasus_server_impl_init.cpp |  1 -
 11 files changed, 182 insertions(+), 2 deletions(-)

diff --git a/src/base/pegasus_const.cpp b/src/base/pegasus_const.cpp
index c93c0baf58..9fc4581ec6 100644
--- a/src/base/pegasus_const.cpp
+++ b/src/base/pegasus_const.cpp
@@ -101,4 +101,15 @@ const std::string USER_SPECIFIED_COMPACTION("user_specified_compaction");
 const std::string READ_SIZE_THROTTLING("replica.read_throttling_by_size");
 
 const std::string ROCKSDB_ALLOW_INGEST_BEHIND("rocksdb.allow_ingest_behind");
+
+const std::string ROCKSDB_WRITE_BUFFER_SIZE("rocksdb.write_buffer_size");
+
+const std::string ROCKSDB_NUM_LEVELS("rocksdb.num_levels");
+
+const std::set<std::string> ROCKSDB_DYNAMIC_OPTIONS = {
+    ROCKSDB_WRITE_BUFFER_SIZE,
+};
+const std::set<std::string> ROCKSDB_STATIC_OPTIONS = {
+    ROCKSDB_NUM_LEVELS,
+};
 } // namespace pegasus
diff --git a/src/base/pegasus_const.h b/src/base/pegasus_const.h
index c2a47ce519..7385c79411 100644
--- a/src/base/pegasus_const.h
+++ b/src/base/pegasus_const.h
@@ -19,6 +19,7 @@
 
 #pragma once
 
+#include <set>
 #include <string>
 
 namespace pegasus {
@@ -72,4 +73,12 @@ extern const std::string USER_SPECIFIED_COMPACTION;
 extern const std::string READ_SIZE_THROTTLING;
 
 extern const std::string ROCKSDB_ALLOW_INGEST_BEHIND;
+
+extern const std::string ROCKSDB_WRITE_BUFFER_SIZE;
+
+extern const std::string ROCKSDB_NUM_LEVELS;
+
+extern const std::set<std::string> ROCKSDB_DYNAMIC_OPTIONS;
+
+extern const std::set<std::string> ROCKSDB_STATIC_OPTIONS;
 } // namespace pegasus
diff --git a/src/common/replica_envs.h b/src/common/replica_envs.h
index 4db367a7fa..9b156391b3 100644
--- a/src/common/replica_envs.h
+++ b/src/common/replica_envs.h
@@ -28,6 +28,7 @@
 
 #include <cstdint>
 #include <string>
+#include <set>
 
 namespace dsn {
 namespace replication {
@@ -64,6 +65,12 @@ class replica_envs
     static const std::string USER_SPECIFIED_COMPACTION;
     static const std::string ROCKSDB_ALLOW_INGEST_BEHIND;
     static const std::string UPDATE_MAX_REPLICA_COUNT;
+    static const std::string ROCKSDB_WRITE_BUFFER_SIZE;
+    static const std::string ROCKSDB_NUM_LEVELS;
+    static const std::string VALUE_VERSION;
+
+    static const std::set<std::string> ROCKSDB_DYNAMIC_OPTIONS;
+    static const std::set<std::string> ROCKSDB_STATIC_OPTIONS;
 };
 
 } // namespace replication
diff --git a/src/common/replication_common.cpp b/src/common/replication_common.cpp
index 63617797ac..8313cd95ba 100644
--- a/src/common/replication_common.cpp
+++ b/src/common/replication_common.cpp
@@ -394,6 +394,15 @@ const std::string replica_envs::USER_SPECIFIED_COMPACTION("user_specified_compac
 const std::string replica_envs::BACKUP_REQUEST_QPS_THROTTLING("replica.backup_request_throttling");
 const std::string replica_envs::ROCKSDB_ALLOW_INGEST_BEHIND("rocksdb.allow_ingest_behind");
 const std::string replica_envs::UPDATE_MAX_REPLICA_COUNT("max_replica_count.update");
+const std::string replica_envs::ROCKSDB_WRITE_BUFFER_SIZE("rocksdb.write_buffer_size");
+const std::string replica_envs::ROCKSDB_NUM_LEVELS("rocksdb.num_levels");
+const std::string replica_envs::VALUE_VERSION("value_version");
 
+const std::set<std::string> replica_envs::ROCKSDB_DYNAMIC_OPTIONS = {
+    replica_envs::ROCKSDB_WRITE_BUFFER_SIZE,
+};
+const std::set<std::string> replica_envs::ROCKSDB_STATIC_OPTIONS = {
+    replica_envs::ROCKSDB_NUM_LEVELS,
+};
 } // namespace replication
 } // namespace dsn
diff --git a/src/meta/app_env_validator.cpp b/src/meta/app_env_validator.cpp
index 41f9c299ce..7f69d43923 100644
--- a/src/meta/app_env_validator.cpp
+++ b/src/meta/app_env_validator.cpp
@@ -31,6 +31,23 @@
 
 namespace dsn {
 namespace replication {
+bool validate_app_envs(const std::map<std::string, std::string> &envs)
+{
+    if (envs.size() == 0)
+        return true;
+    // check app envs information
+    std::string hint_message;
+    bool all_envs_vaild = true;
+    for (auto &it : envs) {
+        if (!validate_app_env(it.first, it.second, hint_message)) {
+            LOG_WARNING(
+                "app env {}={} is invaild, hint_message:{}", it.first, it.second, hint_message);
+            all_envs_vaild = false;
+            break;
+        }
+    }
+    return all_envs_vaild;
+}
 
 bool validate_app_env(const std::string &env_name,
                       const std::string &env_value,
@@ -151,6 +168,36 @@ bool check_bool_value(const std::string &env_value, std::string &hint_message)
     return true;
 }
 
+bool check_rocksdb_write_buffer_size(const std::string &env_value, std::string &hint_message)
+{
+    size_t val = 0;
+
+    if (!dsn::buf2uint64(env_value, val)) {
+        hint_message = fmt::format("rocksdb.write_buffer_size cannot set this val: {}", env_value);
+        return false;
+    }
+    if (val < (32 << 20) || val > (512 << 20)) {
+        hint_message =
+            fmt::format("rocksdb.write_buffer_size suggest set val in range [33554432, 536870912]");
+        return false;
+    }
+    return true;
+}
+bool check_rocksdb_num_levels(const std::string &env_value, std::string &hint_message)
+{
+    int32_t val = 0;
+
+    if (!dsn::buf2int32(env_value, val)) {
+        hint_message = fmt::format("rocksdb.num_levels cannot set this val:", env_value);
+        return false;
+    }
+    if (val < 1 || val > 10) {
+        hint_message = fmt::format("rocksdb.num_levels suggest set val in range [1 , 10]");
+        return false;
+    }
+    return true;
+}
+
 bool app_env_validator::validate_app_env(const std::string &env_name,
                                          const std::string &env_value,
                                          std::string &hint_message)
@@ -198,6 +245,10 @@ void app_env_validator::register_all_validators()
          std::bind(&check_bool_value, std::placeholders::_1, std::placeholders::_2)},
         {replica_envs::DENY_CLIENT_REQUEST,
          std::bind(&check_deny_client, std::placeholders::_1, std::placeholders::_2)},
+        {replica_envs::ROCKSDB_WRITE_BUFFER_SIZE,
+         std::bind(&check_rocksdb_write_buffer_size, std::placeholders::_1, std::placeholders::_2)},
+        {replica_envs::ROCKSDB_NUM_LEVELS,
+         std::bind(&check_rocksdb_num_levels, std::placeholders::_1, std::placeholders::_2)},
         // TODO(zhaoliwei): not implemented
         {replica_envs::BUSINESS_INFO, nullptr},
         {replica_envs::TABLE_LEVEL_DEFAULT_TTL, nullptr},
@@ -213,7 +264,9 @@ void app_env_validator::register_all_validators()
         {replica_envs::MANUAL_COMPACT_PERIODIC_TARGET_LEVEL, nullptr},
         {replica_envs::MANUAL_COMPACT_PERIODIC_BOTTOMMOST_LEVEL_COMPACTION, nullptr},
         {replica_envs::REPLICA_ACCESS_CONTROLLER_ALLOWED_USERS, nullptr},
-        {replica_envs::REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES, nullptr}};
+        {replica_envs::REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES, nullptr},
+        {replica_envs::VALUE_VERSION, nullptr},
+    };
 }
 
 } // namespace replication
diff --git a/src/meta/app_env_validator.h b/src/meta/app_env_validator.h
index c401c39598..d963df0513 100644
--- a/src/meta/app_env_validator.h
+++ b/src/meta/app_env_validator.h
@@ -25,6 +25,8 @@
 namespace dsn {
 namespace replication {
 
+bool validate_app_envs(const std::map<std::string, std::string> &envs);
+
 bool validate_app_env(const std::string &env_name,
                       const std::string &env_value,
                       std::string &hint_message);
diff --git a/src/meta/server_state.cpp b/src/meta/server_state.cpp
index ecf66a92bc..374875fee4 100644
--- a/src/meta/server_state.cpp
+++ b/src/meta/server_state.cpp
@@ -1151,6 +1151,9 @@ void server_state::create_app(dsn::message_ex *msg)
                !validate_target_max_replica_count(request.options.replica_count)) {
         response.err = ERR_INVALID_PARAMETERS;
         will_create_app = false;
+    } else if (!validate_app_envs(request.options.envs)) {
+        response.err = ERR_INVALID_PARAMETERS;
+        will_create_app = false;
     } else {
         zauto_write_lock l(_lock);
         app = get_app(request.app_name);
@@ -2754,6 +2757,12 @@ void server_state::set_app_envs(const app_env_rpc &env_rpc)
             return;
         }
 
+        if (replica_envs::ROCKSDB_STATIC_OPTIONS.find(keys[i]) !=
+            replica_envs::ROCKSDB_STATIC_OPTIONS.end()) {
+            env_rpc.response().err = ERR_INVALID_PARAMETERS;
+            env_rpc.response().hint_message = "static rocksdb option only can set by create app";
+            return;
+        }
         os << keys[i] << "=" << values[i];
     }
     LOG_INFO("set app envs for app({}) from remote({}): kvs = {}",
diff --git a/src/meta/test/meta_app_envs_test.cpp b/src/meta/test/meta_app_envs_test.cpp
index 255930320d..68c561d061 100644
--- a/src/meta/test/meta_app_envs_test.cpp
+++ b/src/meta/test/meta_app_envs_test.cpp
@@ -142,6 +142,22 @@ TEST_F(meta_app_envs_test, update_app_envs_test)
         {replica_envs::MANUAL_COMPACT_ONCE_TARGET_LEVEL, "80", ERR_OK, "", "80"},
         {replica_envs::MANUAL_COMPACT_PERIODIC_TRIGGER_TIME, "90", ERR_OK, "", "90"},
         {replica_envs::MANUAL_COMPACT_PERIODIC_TARGET_LEVEL, "100", ERR_OK, "", "100"},
+        {replica_envs::ROCKSDB_WRITE_BUFFER_SIZE,
+         "100",
+         ERR_INVALID_PARAMETERS,
+         "rocksdb.write_buffer_size suggest set val in range [33554432, 536870912]",
+         "67108864"},
+        {replica_envs::ROCKSDB_WRITE_BUFFER_SIZE,
+         "636870912",
+         ERR_INVALID_PARAMETERS,
+         "rocksdb.write_buffer_size suggest set val in range [33554432, 536870912]",
+         "536870912"},
+        {replica_envs::ROCKSDB_WRITE_BUFFER_SIZE, "67108864", ERR_OK, "", "67108864"},
+        {replica_envs::ROCKSDB_NUM_LEVELS,
+         "5",
+         ERR_INVALID_PARAMETERS,
+         "static rocksdb option only can set by create app",
+         "5"},
         {replica_envs::MANUAL_COMPACT_PERIODIC_BOTTOMMOST_LEVEL_COMPACTION,
          "200",
          ERR_OK,
diff --git a/src/server/pegasus_server_impl.cpp b/src/server/pegasus_server_impl.cpp
index e6e9d48edc..9cfd8cd500 100644
--- a/src/server/pegasus_server_impl.cpp
+++ b/src/server/pegasus_server_impl.cpp
@@ -2625,6 +2625,61 @@ pegasus_server_impl::get_restore_dir_from_env(const std::map<std::string, std::s
     return res;
 }
 
+void pegasus_server_impl::update_rocksdb_dynamic_options(
+    const std::map<std::string, std::string> &envs)
+{
+    if (envs.size() == 0)
+        return;
+
+    std::unordered_map<std::string, std::string> new_options;
+    for (auto &option : ROCKSDB_DYNAMIC_OPTIONS) {
+        auto find = envs.find(option);
+        if (option.compare(ROCKSDB_WRITE_BUFFER_SIZE) == 0 && find != envs.end()) {
+            new_options["write_buffer_size"] = find->second;
+        }
+    }
+
+    // doing set option
+    if (new_options.size() != 0 && !set_options(new_options)) {
+        LOG_WARNING("Set options fails");
+    }
+}
+
+void pegasus_server_impl::update_rocksdb_options_before_create_replica(
+    const std::map<std::string, std::string> &envs)
+{
+    if (envs.size() == 0)
+        return;
+
+    for (auto &option : ROCKSDB_STATIC_OPTIONS) {
+        auto find = envs.find(option);
+        bool is_set = false;
+        if (option.compare(ROCKSDB_NUM_LEVELS) == 0 && find != envs.end()) {
+            dsn::buf2int32(find->second, _data_cf_opts.num_levels);
+            is_set = true;
+        }
+
+        if (is_set)
+            LOG_INFO("Reset {} \"{}\" succeed", find->first, find->second);
+        else
+            LOG_WARNING("Reset {} \"{}\" failed", find->first, find->second);
+    }
+
+    for (auto &option : ROCKSDB_DYNAMIC_OPTIONS) {
+        auto find = envs.find(option);
+        bool is_set = false;
+
+        if (option.compare(ROCKSDB_WRITE_BUFFER_SIZE) == 0 && find != envs.end()) {
+            dsn::buf2uint64(find->second, _data_cf_opts.write_buffer_size);
+            is_set = true;
+        }
+        if (is_set)
+            LOG_INFO("Reset {} \"{}\" succeed", find->first, find->second);
+        else
+            LOG_WARNING("Reset {} \"{}\" failed", find->first, find->second);
+    }
+}
+
 void pegasus_server_impl::update_app_envs(const std::map<std::string, std::string> &envs)
 {
     update_usage_scenario(envs);
@@ -2637,6 +2692,7 @@ void pegasus_server_impl::update_app_envs(const std::map<std::string, std::strin
     _manual_compact_svc.start_manual_compact_if_needed(envs);
 
     update_throttling_controller(envs);
+    update_rocksdb_dynamic_options(envs);
 }
 
 void pegasus_server_impl::update_app_envs_before_open_db(
@@ -2650,6 +2706,7 @@ void pegasus_server_impl::update_app_envs_before_open_db(
     update_validate_partition_hash(envs);
     update_user_specified_compaction(envs);
     _manual_compact_svc.start_manual_compact_if_needed(envs);
+    update_rocksdb_options_before_create_replica(envs);
 }
 
 void pegasus_server_impl::query_app_envs(/*out*/ std::map<std::string, std::string> &envs)
@@ -3054,6 +3111,7 @@ void pegasus_server_impl::reset_usage_scenario_options(
     target_opts->max_compaction_bytes = base_opts.max_compaction_bytes;
     target_opts->write_buffer_size = base_opts.write_buffer_size;
     target_opts->max_write_buffer_number = base_opts.max_write_buffer_number;
+    target_opts->num_levels = base_opts.num_levels;
 }
 
 void pegasus_server_impl::recalculate_data_cf_options(
diff --git a/src/server/pegasus_server_impl.h b/src/server/pegasus_server_impl.h
index d156acdce6..faa4fdb5a5 100644
--- a/src/server/pegasus_server_impl.h
+++ b/src/server/pegasus_server_impl.h
@@ -22,6 +22,7 @@
 #include <gtest/gtest_prod.h>
 #include <rocksdb/options.h>
 #include <rocksdb/slice.h>
+#include <rocksdb/table.h>
 #include <rrdb/rrdb_types.h>
 #include <stdint.h>
 #include <atomic>
@@ -328,6 +329,11 @@ class pegasus_server_impl : public pegasus_read_service
 
     void update_user_specified_compaction(const std::map<std::string, std::string> &envs);
 
+    void update_rocksdb_dynamic_options(const std::map<std::string, std::string> &envs);
+
+    void
+    update_rocksdb_options_before_create_replica(const std::map<std::string, std::string> &envs);
+
     void update_throttling_controller(const std::map<std::string, std::string> &envs);
 
     bool parse_allow_ingest_behind(const std::map<std::string, std::string> &envs);
@@ -468,6 +474,7 @@ class pegasus_server_impl : public pegasus_read_service
     // Dynamically calculate the value of current data_cf option according to the conf module file
     // and usage scenario
     rocksdb::ColumnFamilyOptions _table_data_cf_opts;
+    rocksdb::BlockBasedTableOptions tbl_opts;
     rocksdb::ColumnFamilyOptions _meta_cf_opts;
     rocksdb::ReadOptions _data_cf_rd_opts;
     std::string _usage_scenario;
diff --git a/src/server/pegasus_server_impl_init.cpp b/src/server/pegasus_server_impl_init.cpp
index 896f8ab945..bad78c9eb1 100644
--- a/src/server/pegasus_server_impl_init.cpp
+++ b/src/server/pegasus_server_impl_init.cpp
@@ -466,7 +466,6 @@ pegasus_server_impl::pegasus_server_impl(dsn::replication::replica *r)
     CHECK(parse_compression_types("none", _meta_cf_opts.compression_per_level),
           "parse rocksdb_compression_type failed.");
 
-    rocksdb::BlockBasedTableOptions tbl_opts;
     tbl_opts.read_amp_bytes_per_bit = FLAGS_read_amp_bytes_per_bit;
 
     if (FLAGS_rocksdb_disable_table_block_cache) {

From 2acae8f1b40c23035631bda171d9a3774adf1e09 Mon Sep 17 00:00:00 2001
From: Pengfan Lu <lu_peng_fan@163.com>
Date: Wed, 7 Jun 2023 12:00:04 +0000
Subject: [PATCH 02/10] hava a bug from num_levels

---
 src/meta/app_env_validator.cpp     | 2 +-
 src/server/pegasus_server_impl.cpp | 8 ++------
 2 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/src/meta/app_env_validator.cpp b/src/meta/app_env_validator.cpp
index 7f69d43923..de82c8f0af 100644
--- a/src/meta/app_env_validator.cpp
+++ b/src/meta/app_env_validator.cpp
@@ -170,7 +170,7 @@ bool check_bool_value(const std::string &env_value, std::string &hint_message)
 
 bool check_rocksdb_write_buffer_size(const std::string &env_value, std::string &hint_message)
 {
-    size_t val = 0;
+    uint64_t val = 0;
 
     if (!dsn::buf2uint64(env_value, val)) {
         hint_message = fmt::format("rocksdb.write_buffer_size cannot set this val: {}", env_value);
diff --git a/src/server/pegasus_server_impl.cpp b/src/server/pegasus_server_impl.cpp
index 9cfd8cd500..9c845a4903 100644
--- a/src/server/pegasus_server_impl.cpp
+++ b/src/server/pegasus_server_impl.cpp
@@ -2651,7 +2651,7 @@ void pegasus_server_impl::update_rocksdb_options_before_create_replica(
     if (envs.size() == 0)
         return;
 
-    for (auto &option : ROCKSDB_STATIC_OPTIONS) {
+    for (auto &option : pegasus::ROCKSDB_STATIC_OPTIONS) {
         auto find = envs.find(option);
         bool is_set = false;
         if (option.compare(ROCKSDB_NUM_LEVELS) == 0 && find != envs.end()) {
@@ -2661,11 +2661,9 @@ void pegasus_server_impl::update_rocksdb_options_before_create_replica(
 
         if (is_set)
             LOG_INFO("Reset {} \"{}\" succeed", find->first, find->second);
-        else
-            LOG_WARNING("Reset {} \"{}\" failed", find->first, find->second);
     }
 
-    for (auto &option : ROCKSDB_DYNAMIC_OPTIONS) {
+    for (auto &option : pegasus::ROCKSDB_DYNAMIC_OPTIONS) {
         auto find = envs.find(option);
         bool is_set = false;
 
@@ -2675,8 +2673,6 @@ void pegasus_server_impl::update_rocksdb_options_before_create_replica(
         }
         if (is_set)
             LOG_INFO("Reset {} \"{}\" succeed", find->first, find->second);
-        else
-            LOG_WARNING("Reset {} \"{}\" failed", find->first, find->second);
     }
 }
 

From 61259259634298cf447c2d35a7af9d5cf234a5d2 Mon Sep 17 00:00:00 2001
From: Pengfan Lu <lu_peng_fan@163.com>
Date: Fri, 9 Jun 2023 05:29:05 +0000
Subject: [PATCH 03/10] Feature:Online Query and Dynamic Modification of
 Table-level RocksDB Options (#1488)
 https://github.com/apache/incubator-pegasus/issues/1488 Currently, I only
 perform strict validation of app envs during the create_app command process.
 However, in the set_app_envs command, there is no validation for invalid
 envs, and they are simply not processed.

- Manual test (add detailed scripts or steps below)
Pegasus shell case:
create lpf -e rocksdb.num_levels=12,rocksdb.write_buffer_size=100
create lpf -e rocksdb.num_levels=5,rocksdb.write_buffer_size=100
create lpf -e rocksdb.num_levels=5,rocksdb.write_buffer_size=33554432
set_app_envs rocksdb.write_buffer_size  67108864
set_app_envs rocksdb.write_buffer_size  100
get_app_envs
---
 src/common/replication_common.cpp    |  1 +
 src/meta/app_env_validator.cpp       |  7 ++++++-
 src/meta/server_state.cpp            |  6 ------
 src/meta/test/meta_app_envs_test.cpp |  5 -----
 src/server/pegasus_server_impl.cpp   | 13 +++++++++----
 5 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/src/common/replication_common.cpp b/src/common/replication_common.cpp
index 8313cd95ba..11899c2e62 100644
--- a/src/common/replication_common.cpp
+++ b/src/common/replication_common.cpp
@@ -29,6 +29,7 @@
 #include <algorithm>
 #include <fstream>
 #include <memory>
+#include <set>
 
 #include "common/gpid.h"
 #include "common/replica_envs.h"
diff --git a/src/meta/app_env_validator.cpp b/src/meta/app_env_validator.cpp
index de82c8f0af..7a64e83fa5 100644
--- a/src/meta/app_env_validator.cpp
+++ b/src/meta/app_env_validator.cpp
@@ -35,10 +35,15 @@ bool validate_app_envs(const std::map<std::string, std::string> &envs)
 {
     if (envs.size() == 0)
         return true;
-    // check app envs information
+    // check rocksdb app envs information
     std::string hint_message;
     bool all_envs_vaild = true;
     for (auto &it : envs) {
+        if (replica_envs::ROCKSDB_STATIC_OPTIONS.find(it.first) ==
+                replica_envs::ROCKSDB_STATIC_OPTIONS.end() &&
+            replica_envs::ROCKSDB_DYNAMIC_OPTIONS.find(it.first) ==
+                replica_envs::ROCKSDB_DYNAMIC_OPTIONS.end())
+            continue;
         if (!validate_app_env(it.first, it.second, hint_message)) {
             LOG_WARNING(
                 "app env {}={} is invaild, hint_message:{}", it.first, it.second, hint_message);
diff --git a/src/meta/server_state.cpp b/src/meta/server_state.cpp
index 374875fee4..2b6c6d9108 100644
--- a/src/meta/server_state.cpp
+++ b/src/meta/server_state.cpp
@@ -2757,12 +2757,6 @@ void server_state::set_app_envs(const app_env_rpc &env_rpc)
             return;
         }
 
-        if (replica_envs::ROCKSDB_STATIC_OPTIONS.find(keys[i]) !=
-            replica_envs::ROCKSDB_STATIC_OPTIONS.end()) {
-            env_rpc.response().err = ERR_INVALID_PARAMETERS;
-            env_rpc.response().hint_message = "static rocksdb option only can set by create app";
-            return;
-        }
         os << keys[i] << "=" << values[i];
     }
     LOG_INFO("set app envs for app({}) from remote({}): kvs = {}",
diff --git a/src/meta/test/meta_app_envs_test.cpp b/src/meta/test/meta_app_envs_test.cpp
index 68c561d061..c115f80770 100644
--- a/src/meta/test/meta_app_envs_test.cpp
+++ b/src/meta/test/meta_app_envs_test.cpp
@@ -153,11 +153,6 @@ TEST_F(meta_app_envs_test, update_app_envs_test)
          "rocksdb.write_buffer_size suggest set val in range [33554432, 536870912]",
          "536870912"},
         {replica_envs::ROCKSDB_WRITE_BUFFER_SIZE, "67108864", ERR_OK, "", "67108864"},
-        {replica_envs::ROCKSDB_NUM_LEVELS,
-         "5",
-         ERR_INVALID_PARAMETERS,
-         "static rocksdb option only can set by create app",
-         "5"},
         {replica_envs::MANUAL_COMPACT_PERIODIC_BOTTOMMOST_LEVEL_COMPACTION,
          "200",
          ERR_OK,
diff --git a/src/server/pegasus_server_impl.cpp b/src/server/pegasus_server_impl.cpp
index 9c845a4903..050d955642 100644
--- a/src/server/pegasus_server_impl.cpp
+++ b/src/server/pegasus_server_impl.cpp
@@ -42,6 +42,7 @@
 #include <list>
 #include <mutex>
 #include <ostream>
+#include <set>
 
 #include "base/pegasus_key_schema.h"
 #include "base/pegasus_utils.h"
@@ -2655,8 +2656,11 @@ void pegasus_server_impl::update_rocksdb_options_before_create_replica(
         auto find = envs.find(option);
         bool is_set = false;
         if (option.compare(ROCKSDB_NUM_LEVELS) == 0 && find != envs.end()) {
-            dsn::buf2int32(find->second, _data_cf_opts.num_levels);
+            int32_t val = 0;
+            if (!dsn::buf2int32(find->second, val))
+                continue;
             is_set = true;
+            _data_cf_opts.num_levels = val;
         }
 
         if (is_set)
@@ -2666,10 +2670,11 @@ void pegasus_server_impl::update_rocksdb_options_before_create_replica(
     for (auto &option : pegasus::ROCKSDB_DYNAMIC_OPTIONS) {
         auto find = envs.find(option);
         bool is_set = false;
-
         if (option.compare(ROCKSDB_WRITE_BUFFER_SIZE) == 0 && find != envs.end()) {
-            dsn::buf2uint64(find->second, _data_cf_opts.write_buffer_size);
-            is_set = true;
+            uint64_t val = 0;
+            if (!dsn::buf2uint64(find->second, val))
+                continue;
+            _data_cf_opts.write_buffer_size = static_cast<size_t>(val);
         }
         if (is_set)
             LOG_INFO("Reset {} \"{}\" succeed", find->first, find->second);

From 97ea9363e15699e1224d113d8d73dd8e3001fe65 Mon Sep 17 00:00:00 2001
From: Pengfan Lu <lu_peng_fan@163.com>
Date: Mon, 12 Jun 2023 07:05:14 +0000
Subject: [PATCH 04/10] fix iwyu bug

---
 src/meta/app_env_validator.cpp | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/meta/app_env_validator.cpp b/src/meta/app_env_validator.cpp
index 7a64e83fa5..76c92672a9 100644
--- a/src/meta/app_env_validator.cpp
+++ b/src/meta/app_env_validator.cpp
@@ -20,6 +20,7 @@
 #include <fmt/core.h>
 #include <stdint.h>
 #include <memory>
+#include <set>
 #include <utility>
 #include <vector>
 

From 98816c4d6e9ddbc3c036df5bf415562e3d8b43f2 Mon Sep 17 00:00:00 2001
From: Pengfan Lu <lu_peng_fan@163.com>
Date: Tue, 13 Jun 2023 03:41:39 +0000
Subject: [PATCH 05/10] reply hycdong's comment, and adjust my code.

---
 src/common/replica_envs.h          |  1 -
 src/common/replication_common.cpp  |  1 -
 src/meta/app_env_validator.cpp     |  7 +++---
 src/server/pegasus_server_impl.cpp | 38 +++++++++++++++++++++---------
 src/server/pegasus_server_impl.h   |  6 +++--
 5 files changed, 34 insertions(+), 19 deletions(-)

diff --git a/src/common/replica_envs.h b/src/common/replica_envs.h
index 9b156391b3..1db2e5399f 100644
--- a/src/common/replica_envs.h
+++ b/src/common/replica_envs.h
@@ -67,7 +67,6 @@ class replica_envs
     static const std::string UPDATE_MAX_REPLICA_COUNT;
     static const std::string ROCKSDB_WRITE_BUFFER_SIZE;
     static const std::string ROCKSDB_NUM_LEVELS;
-    static const std::string VALUE_VERSION;
 
     static const std::set<std::string> ROCKSDB_DYNAMIC_OPTIONS;
     static const std::set<std::string> ROCKSDB_STATIC_OPTIONS;
diff --git a/src/common/replication_common.cpp b/src/common/replication_common.cpp
index 11899c2e62..1afe9d49fe 100644
--- a/src/common/replication_common.cpp
+++ b/src/common/replication_common.cpp
@@ -397,7 +397,6 @@ const std::string replica_envs::ROCKSDB_ALLOW_INGEST_BEHIND("rocksdb.allow_inges
 const std::string replica_envs::UPDATE_MAX_REPLICA_COUNT("max_replica_count.update");
 const std::string replica_envs::ROCKSDB_WRITE_BUFFER_SIZE("rocksdb.write_buffer_size");
 const std::string replica_envs::ROCKSDB_NUM_LEVELS("rocksdb.num_levels");
-const std::string replica_envs::VALUE_VERSION("value_version");
 
 const std::set<std::string> replica_envs::ROCKSDB_DYNAMIC_OPTIONS = {
     replica_envs::ROCKSDB_WRITE_BUFFER_SIZE,
diff --git a/src/meta/app_env_validator.cpp b/src/meta/app_env_validator.cpp
index 76c92672a9..f426af48d3 100644
--- a/src/meta/app_env_validator.cpp
+++ b/src/meta/app_env_validator.cpp
@@ -36,10 +36,10 @@ bool validate_app_envs(const std::map<std::string, std::string> &envs)
 {
     if (envs.size() == 0)
         return true;
-    // check rocksdb app envs information
+    // only check rocksdb app envs currently
     std::string hint_message;
     bool all_envs_vaild = true;
-    for (auto &it : envs) {
+    for (const auto &it : envs) {
         if (replica_envs::ROCKSDB_STATIC_OPTIONS.find(it.first) ==
                 replica_envs::ROCKSDB_STATIC_OPTIONS.end() &&
             replica_envs::ROCKSDB_DYNAMIC_OPTIONS.find(it.first) ==
@@ -194,7 +194,7 @@ bool check_rocksdb_num_levels(const std::string &env_value, std::string &hint_me
     int32_t val = 0;
 
     if (!dsn::buf2int32(env_value, val)) {
-        hint_message = fmt::format("rocksdb.num_levels cannot set this val:", env_value);
+        hint_message = fmt::format("rocksdb.num_levels cannot set this val: {}", env_value);
         return false;
     }
     if (val < 1 || val > 10) {
@@ -271,7 +271,6 @@ void app_env_validator::register_all_validators()
         {replica_envs::MANUAL_COMPACT_PERIODIC_BOTTOMMOST_LEVEL_COMPACTION, nullptr},
         {replica_envs::REPLICA_ACCESS_CONTROLLER_ALLOWED_USERS, nullptr},
         {replica_envs::REPLICA_ACCESS_CONTROLLER_RANGER_POLICIES, nullptr},
-        {replica_envs::VALUE_VERSION, nullptr},
     };
 }
 
diff --git a/src/server/pegasus_server_impl.cpp b/src/server/pegasus_server_impl.cpp
index 050d955642..430efc8641 100644
--- a/src/server/pegasus_server_impl.cpp
+++ b/src/server/pegasus_server_impl.cpp
@@ -1664,7 +1664,7 @@ dsn::error_code pegasus_server_impl::start(int argc, char **argv)
             // We don't use `loaded_data_cf_opts` directly because pointer-typed options will
             // only be initialized with default values when calling 'LoadLatestOptions', see
             // 'rocksdb/utilities/options_util.h'.
-            reset_usage_scenario_options(loaded_data_cf_opts, &_table_data_cf_opts);
+            reset_rocksdb_options(loaded_data_cf_opts, &_table_data_cf_opts);
             _db_opts.allow_ingest_behind = parse_allow_ingest_behind(envs);
         }
     } else {
@@ -2633,7 +2633,7 @@ void pegasus_server_impl::update_rocksdb_dynamic_options(
         return;
 
     std::unordered_map<std::string, std::string> new_options;
-    for (auto &option : ROCKSDB_DYNAMIC_OPTIONS) {
+    for (const auto &option : ROCKSDB_DYNAMIC_OPTIONS) {
         auto find = envs.find(option);
         if (option.compare(ROCKSDB_WRITE_BUFFER_SIZE) == 0 && find != envs.end()) {
             new_options["write_buffer_size"] = find->second;
@@ -2641,18 +2641,18 @@ void pegasus_server_impl::update_rocksdb_dynamic_options(
     }
 
     // doing set option
-    if (new_options.size() != 0 && !set_options(new_options)) {
-        LOG_WARNING("Set options fails");
+    if (new_options.size() > 0 && set_options(new_options)) {
+        LOG_INFO("Set rocksdb dynamic options success");
     }
 }
 
-void pegasus_server_impl::update_rocksdb_options_before_create_replica(
+void pegasus_server_impl::set_rocksdb_options_before_creating(
     const std::map<std::string, std::string> &envs)
 {
     if (envs.size() == 0)
         return;
 
-    for (auto &option : pegasus::ROCKSDB_STATIC_OPTIONS) {
+    for (const auto &option : pegasus::ROCKSDB_STATIC_OPTIONS) {
         auto find = envs.find(option);
         bool is_set = false;
         if (option.compare(ROCKSDB_NUM_LEVELS) == 0 && find != envs.end()) {
@@ -2664,20 +2664,21 @@ void pegasus_server_impl::update_rocksdb_options_before_create_replica(
         }
 
         if (is_set)
-            LOG_INFO("Reset {} \"{}\" succeed", find->first, find->second);
+            LOG_INFO("Set {} \"{}\" succeed", find->first, find->second);
     }
 
-    for (auto &option : pegasus::ROCKSDB_DYNAMIC_OPTIONS) {
+    for (const auto &option : pegasus::ROCKSDB_DYNAMIC_OPTIONS) {
         auto find = envs.find(option);
         bool is_set = false;
         if (option.compare(ROCKSDB_WRITE_BUFFER_SIZE) == 0 && find != envs.end()) {
             uint64_t val = 0;
             if (!dsn::buf2uint64(find->second, val))
                 continue;
+            is_set = true;
             _data_cf_opts.write_buffer_size = static_cast<size_t>(val);
         }
         if (is_set)
-            LOG_INFO("Reset {} \"{}\" succeed", find->first, find->second);
+            LOG_INFO("Set {} \"{}\" succeed", find->first, find->second);
     }
 }
 
@@ -2707,7 +2708,7 @@ void pegasus_server_impl::update_app_envs_before_open_db(
     update_validate_partition_hash(envs);
     update_user_specified_compaction(envs);
     _manual_compact_svc.start_manual_compact_if_needed(envs);
-    update_rocksdb_options_before_create_replica(envs);
+    set_rocksdb_options_before_creating(envs);
 }
 
 void pegasus_server_impl::query_app_envs(/*out*/ std::map<std::string, std::string> &envs)
@@ -3096,6 +3097,22 @@ bool pegasus_server_impl::set_usage_scenario(const std::string &usage_scenario)
     }
 }
 
+void pegasus_server_impl::reset_rocksdb_options(const rocksdb::ColumnFamilyOptions &base_opts,
+                                                rocksdb::ColumnFamilyOptions *target_opts)
+{
+    // Reset rocksdb option includes two aspects:
+    // 1. Set usage_scenario related rocksdb options
+    // 2. Rocksdb option set in app envs, consists of ROCKSDB_DYNAMIC_OPTIONS and
+    // ROCKSDB_STATIC_OPTIONS
+
+    // aspect 1:
+    reset_usage_scenario_options(base_opts, target_opts);
+
+    // aspect 2:
+    target_opts->num_levels = base_opts.num_levels;
+    target_opts->write_buffer_size = base_opts.write_buffer_size;
+}
+
 void pegasus_server_impl::reset_usage_scenario_options(
     const rocksdb::ColumnFamilyOptions &base_opts, rocksdb::ColumnFamilyOptions *target_opts)
 {
@@ -3112,7 +3129,6 @@ void pegasus_server_impl::reset_usage_scenario_options(
     target_opts->max_compaction_bytes = base_opts.max_compaction_bytes;
     target_opts->write_buffer_size = base_opts.write_buffer_size;
     target_opts->max_write_buffer_number = base_opts.max_write_buffer_number;
-    target_opts->num_levels = base_opts.num_levels;
 }
 
 void pegasus_server_impl::recalculate_data_cf_options(
diff --git a/src/server/pegasus_server_impl.h b/src/server/pegasus_server_impl.h
index faa4fdb5a5..1abe14f84d 100644
--- a/src/server/pegasus_server_impl.h
+++ b/src/server/pegasus_server_impl.h
@@ -331,8 +331,7 @@ class pegasus_server_impl : public pegasus_read_service
 
     void update_rocksdb_dynamic_options(const std::map<std::string, std::string> &envs);
 
-    void
-    update_rocksdb_options_before_create_replica(const std::map<std::string, std::string> &envs);
+    void set_rocksdb_options_before_creating(const std::map<std::string, std::string> &envs);
 
     void update_throttling_controller(const std::map<std::string, std::string> &envs);
 
@@ -365,6 +364,9 @@ class pegasus_server_impl : public pegasus_read_service
     void reset_usage_scenario_options(const rocksdb::ColumnFamilyOptions &base_opts,
                                       rocksdb::ColumnFamilyOptions *target_opts);
 
+    void reset_rocksdb_options(const rocksdb::ColumnFamilyOptions &base_opts,
+                               rocksdb::ColumnFamilyOptions *target_opts);
+
     // return true if successfully set
     bool set_options(const std::unordered_map<std::string, std::string> &new_options);
 

From 79b77aaa40cd1941698371df151fd39e6334376c Mon Sep 17 00:00:00 2001
From: Pengfan Lu <lu_peng_fan@163.com>
Date: Mon, 19 Jun 2023 05:52:13 +0000
Subject: [PATCH 06/10] Thank you empiredan for the suggestion, which I took
 into my notes and adjusted my code.

---
 src/meta/app_env_validator.cpp          | 14 ++++----
 src/server/pegasus_server_impl.cpp      | 31 +++++++++++++----
 src/server/pegasus_server_impl.h        |  2 +-
 src/server/pegasus_server_impl_init.cpp | 46 ++++++++++++-------------
 4 files changed, 54 insertions(+), 39 deletions(-)

diff --git a/src/meta/app_env_validator.cpp b/src/meta/app_env_validator.cpp
index f426af48d3..fbc8eab467 100644
--- a/src/meta/app_env_validator.cpp
+++ b/src/meta/app_env_validator.cpp
@@ -34,25 +34,23 @@ namespace dsn {
 namespace replication {
 bool validate_app_envs(const std::map<std::string, std::string> &envs)
 {
-    if (envs.size() == 0)
-        return true;
     // only check rocksdb app envs currently
-    std::string hint_message;
-    bool all_envs_vaild = true;
+
     for (const auto &it : envs) {
         if (replica_envs::ROCKSDB_STATIC_OPTIONS.find(it.first) ==
                 replica_envs::ROCKSDB_STATIC_OPTIONS.end() &&
             replica_envs::ROCKSDB_DYNAMIC_OPTIONS.find(it.first) ==
-                replica_envs::ROCKSDB_DYNAMIC_OPTIONS.end())
+                replica_envs::ROCKSDB_DYNAMIC_OPTIONS.end()) {
             continue;
+        }
+        std::string hint_message;
         if (!validate_app_env(it.first, it.second, hint_message)) {
             LOG_WARNING(
                 "app env {}={} is invaild, hint_message:{}", it.first, it.second, hint_message);
-            all_envs_vaild = false;
-            break;
+            return false;
         }
     }
-    return all_envs_vaild;
+    return true;
 }
 
 bool validate_app_env(const std::string &env_name,
diff --git a/src/server/pegasus_server_impl.cpp b/src/server/pegasus_server_impl.cpp
index 430efc8641..81c109f10e 100644
--- a/src/server/pegasus_server_impl.cpp
+++ b/src/server/pegasus_server_impl.cpp
@@ -2629,19 +2629,29 @@ pegasus_server_impl::get_restore_dir_from_env(const std::map<std::string, std::s
 void pegasus_server_impl::update_rocksdb_dynamic_options(
     const std::map<std::string, std::string> &envs)
 {
-    if (envs.size() == 0)
+    if (envs.size() == 0) {
         return;
+    }
+    auto extract_option = [](const std::string &option) -> std::string {
+        std::stringstream ss(option);
+        std::string prefix, rocksdb_opt;
+        std::getline(ss, prefix, '.');
+        std::getline(ss, rocksdb_opt);
+        LOG_INFO("Extract rocksdb dynamic opt ({}) from ({})", rocksdb_opt, option);
+        return rocksdb_opt;
+    };
 
     std::unordered_map<std::string, std::string> new_options;
     for (const auto &option : ROCKSDB_DYNAMIC_OPTIONS) {
         auto find = envs.find(option);
-        if (option.compare(ROCKSDB_WRITE_BUFFER_SIZE) == 0 && find != envs.end()) {
-            new_options["write_buffer_size"] = find->second;
+        if (find == envs.end()) {
+            continue;
         }
+        new_options[extract_option(option)] = find->second;
     }
 
     // doing set option
-    if (new_options.size() > 0 && set_options(new_options)) {
+    if (new_options.empty() && set_options(new_options)) {
         LOG_INFO("Set rocksdb dynamic options success");
     }
 }
@@ -2649,13 +2659,17 @@ void pegasus_server_impl::update_rocksdb_dynamic_options(
 void pegasus_server_impl::set_rocksdb_options_before_creating(
     const std::map<std::string, std::string> &envs)
 {
-    if (envs.size() == 0)
+    if (envs.size() == 0) {
         return;
+    }
 
     for (const auto &option : pegasus::ROCKSDB_STATIC_OPTIONS) {
         auto find = envs.find(option);
+        if (find == envs.end()) {
+            continue;
+        }
         bool is_set = false;
-        if (option.compare(ROCKSDB_NUM_LEVELS) == 0 && find != envs.end()) {
+        if (option.compare(ROCKSDB_NUM_LEVELS) == 0) {
             int32_t val = 0;
             if (!dsn::buf2int32(find->second, val))
                 continue;
@@ -2669,8 +2683,11 @@ void pegasus_server_impl::set_rocksdb_options_before_creating(
 
     for (const auto &option : pegasus::ROCKSDB_DYNAMIC_OPTIONS) {
         auto find = envs.find(option);
+        if (find == envs.end()) {
+            continue;
+        }
         bool is_set = false;
-        if (option.compare(ROCKSDB_WRITE_BUFFER_SIZE) == 0 && find != envs.end()) {
+        if (option.compare(ROCKSDB_WRITE_BUFFER_SIZE) == 0) {
             uint64_t val = 0;
             if (!dsn::buf2uint64(find->second, val))
                 continue;
diff --git a/src/server/pegasus_server_impl.h b/src/server/pegasus_server_impl.h
index 1abe14f84d..c9c5b90ef6 100644
--- a/src/server/pegasus_server_impl.h
+++ b/src/server/pegasus_server_impl.h
@@ -476,7 +476,7 @@ class pegasus_server_impl : public pegasus_read_service
     // Dynamically calculate the value of current data_cf option according to the conf module file
     // and usage scenario
     rocksdb::ColumnFamilyOptions _table_data_cf_opts;
-    rocksdb::BlockBasedTableOptions tbl_opts;
+    rocksdb::BlockBasedTableOptions _tbl_opts;
     rocksdb::ColumnFamilyOptions _meta_cf_opts;
     rocksdb::ReadOptions _data_cf_rd_opts;
     std::string _usage_scenario;
diff --git a/src/server/pegasus_server_impl_init.cpp b/src/server/pegasus_server_impl_init.cpp
index bad78c9eb1..c457ebd9fa 100644
--- a/src/server/pegasus_server_impl_init.cpp
+++ b/src/server/pegasus_server_impl_init.cpp
@@ -137,7 +137,7 @@ DSN_DEFINE_bool(pegasus.server,
 DSN_DEFINE_bool(pegasus.server,
                 rocksdb_disable_table_block_cache,
                 false,
-                "rocksdb tbl_opts.no_block_cache");
+                "rocksdb _tbl_opts.no_block_cache");
 DSN_DEFINE_bool(pegasus.server,
                 rocksdb_enable_write_buffer_manager,
                 false,
@@ -466,11 +466,11 @@ pegasus_server_impl::pegasus_server_impl(dsn::replication::replica *r)
     CHECK(parse_compression_types("none", _meta_cf_opts.compression_per_level),
           "parse rocksdb_compression_type failed.");
 
-    tbl_opts.read_amp_bytes_per_bit = FLAGS_read_amp_bytes_per_bit;
+    _tbl_opts.read_amp_bytes_per_bit = FLAGS_read_amp_bytes_per_bit;
 
     if (FLAGS_rocksdb_disable_table_block_cache) {
-        tbl_opts.no_block_cache = true;
-        tbl_opts.block_restart_interval = 4;
+        _tbl_opts.no_block_cache = true;
+        _tbl_opts.block_restart_interval = 4;
     } else {
         // If block cache is enabled, all replicas on this server will share the same block cache
         // object. It's convenient to control the total memory used by this server, and the LRU
@@ -483,7 +483,7 @@ pegasus_server_impl::pegasus_server_impl(dsn::replication::replica *r)
         });
 
         // every replica has the same block cache
-        tbl_opts.block_cache = _s_block_cache;
+        _tbl_opts.block_cache = _s_block_cache;
     }
 
     // FLAGS_rocksdb_limiter_max_write_megabytes_per_sec <= 0 means close the rate limit.
@@ -519,7 +519,7 @@ pegasus_server_impl::pegasus_server_impl(dsn::replication::replica *r)
                             FLAGS_rocksdb_total_size_across_write_buffer);
             _s_write_buffer_manager = std::make_shared<rocksdb::WriteBufferManager>(
                 static_cast<size_t>(FLAGS_rocksdb_total_size_across_write_buffer),
-                tbl_opts.block_cache);
+                _tbl_opts.block_cache);
         });
         _db_opts.write_buffer_manager = _s_write_buffer_manager;
     }
@@ -540,33 +540,33 @@ pegasus_server_impl::pegasus_server_impl(dsn::replication::replica *r)
     CHECK(index_type_item != INDEX_TYPE_STRING_MAP.end(),
           "[pegasus.server]rocksdb_index_type should be one among binary_search, "
           "hash_search, two_level_index_search or binary_search_with_first_key.");
-    tbl_opts.index_type = index_type_item->second;
+    _tbl_opts.index_type = index_type_item->second;
     LOG_INFO_PREFIX("rocksdb_index_type = {}", FLAGS_rocksdb_index_type);
 
-    tbl_opts.partition_filters = FLAGS_rocksdb_partition_filters;
+    _tbl_opts.partition_filters = FLAGS_rocksdb_partition_filters;
     // TODO(yingchun): clean up these useless log ?
-    LOG_INFO_PREFIX("rocksdb_partition_filters = {}", tbl_opts.partition_filters);
+    LOG_INFO_PREFIX("rocksdb_partition_filters = {}", _tbl_opts.partition_filters);
 
-    tbl_opts.metadata_block_size = FLAGS_rocksdb_metadata_block_size;
-    LOG_INFO_PREFIX("rocksdb_metadata_block_size = {}", tbl_opts.metadata_block_size);
+    _tbl_opts.metadata_block_size = FLAGS_rocksdb_metadata_block_size;
+    LOG_INFO_PREFIX("rocksdb_metadata_block_size = {}", _tbl_opts.metadata_block_size);
 
-    tbl_opts.cache_index_and_filter_blocks = FLAGS_rocksdb_cache_index_and_filter_blocks;
+    _tbl_opts.cache_index_and_filter_blocks = FLAGS_rocksdb_cache_index_and_filter_blocks;
     LOG_INFO_PREFIX("rocksdb_cache_index_and_filter_blocks = {}",
-                    tbl_opts.cache_index_and_filter_blocks);
+                    _tbl_opts.cache_index_and_filter_blocks);
 
-    tbl_opts.pin_top_level_index_and_filter = FLAGS_rocksdb_pin_top_level_index_and_filter;
+    _tbl_opts.pin_top_level_index_and_filter = FLAGS_rocksdb_pin_top_level_index_and_filter;
     LOG_INFO_PREFIX("rocksdb_pin_top_level_index_and_filter = {}",
-                    tbl_opts.pin_top_level_index_and_filter);
+                    _tbl_opts.pin_top_level_index_and_filter);
 
-    tbl_opts.cache_index_and_filter_blocks_with_high_priority =
+    _tbl_opts.cache_index_and_filter_blocks_with_high_priority =
         FLAGS_rocksdb_cache_index_and_filter_blocks_with_high_priority;
     LOG_INFO_PREFIX("rocksdb_cache_index_and_filter_blocks_with_high_priority = {}",
-                    tbl_opts.cache_index_and_filter_blocks_with_high_priority);
+                    _tbl_opts.cache_index_and_filter_blocks_with_high_priority);
 
-    tbl_opts.pin_l0_filter_and_index_blocks_in_cache =
+    _tbl_opts.pin_l0_filter_and_index_blocks_in_cache =
         FLAGS_rocksdb_pin_l0_filter_and_index_blocks_in_cache;
     LOG_INFO_PREFIX("rocksdb_pin_l0_filter_and_index_blocks_in_cache = {}",
-                    tbl_opts.pin_l0_filter_and_index_blocks_in_cache);
+                    _tbl_opts.pin_l0_filter_and_index_blocks_in_cache);
 
     // Bloom filter configurations.
     if (!FLAGS_rocksdb_disable_bloom_filter) {
@@ -583,8 +583,8 @@ pegasus_server_impl::pegasus_server_impl(dsn::replication::replica *r)
         //                                 50         |      0.225453      |      ~0.00003
         // Recommend using no more than three decimal digits after the decimal point, as in 6.667.
         // More details: https://github.com/facebook/rocksdb/wiki/RocksDB-Bloom-Filter
-        tbl_opts.format_version = FLAGS_rocksdb_format_version;
-        tbl_opts.filter_policy.reset(
+        _tbl_opts.format_version = FLAGS_rocksdb_format_version;
+        _tbl_opts.filter_policy.reset(
             rocksdb::NewBloomFilterPolicy(FLAGS_rocksdb_bloom_filter_bits_per_key, false));
 
         if (dsn::utils::equals(FLAGS_rocksdb_filter_type, "prefix")) {
@@ -595,8 +595,8 @@ pegasus_server_impl::pegasus_server_impl(dsn::replication::replica *r)
         }
     }
 
-    _data_cf_opts.table_factory.reset(NewBlockBasedTableFactory(tbl_opts));
-    _meta_cf_opts.table_factory.reset(NewBlockBasedTableFactory(tbl_opts));
+    _data_cf_opts.table_factory.reset(NewBlockBasedTableFactory(_tbl_opts));
+    _meta_cf_opts.table_factory.reset(NewBlockBasedTableFactory(_tbl_opts));
 
     _key_ttl_compaction_filter_factory = std::make_shared<KeyWithTTLCompactionFilterFactory>();
     _data_cf_opts.compaction_filter_factory = _key_ttl_compaction_filter_factory;

From 4b838e2d9ee338dc0d3ca86f69d76cb08f4529f2 Mon Sep 17 00:00:00 2001
From: Pengfan Lu <lu_peng_fan@163.com>
Date: Thu, 29 Jun 2023 10:30:37 +0000
Subject: [PATCH 07/10] Thank you empiredan for the very detailed suggestion, I
 modified my code. Mainly the following aspects: 1. Improve the unit test to
 ensure that the new option can also be added correctly. 2. Add getter and
 setter methods for option.

---
 src/base/pegasus_const.cpp                   |  37 ++++
 src/base/pegasus_const.h                     |  13 ++
 src/meta/app_env_validator.cpp               |   4 +-
 src/meta/test/meta_app_envs_test.cpp         |  17 +-
 src/meta/test/meta_app_operation_test.cpp    | 169 ++++++++++++++-----
 src/server/pegasus_server_impl.cpp           |  65 +++----
 src/server/test/pegasus_server_impl_test.cpp |  59 +++++++
 7 files changed, 289 insertions(+), 75 deletions(-)

diff --git a/src/base/pegasus_const.cpp b/src/base/pegasus_const.cpp
index 9fc4581ec6..857ce7d824 100644
--- a/src/base/pegasus_const.cpp
+++ b/src/base/pegasus_const.cpp
@@ -19,6 +19,13 @@
 
 #include "pegasus_const.h"
 
+#include <fmt/core.h>
+#include <rocksdb/options.h>
+#include <stddef.h>
+#include <stdint.h>
+
+#include "utils/string_conv.h"
+
 namespace pegasus {
 
 // should be same with items in dsn::backup_restore_constant
@@ -112,4 +119,34 @@ const std::set<std::string> ROCKSDB_DYNAMIC_OPTIONS = {
 const std::set<std::string> ROCKSDB_STATIC_OPTIONS = {
     ROCKSDB_NUM_LEVELS,
 };
+
+const std::unordered_map<std::string, cf_opts_setter> cf_opts_setters = {
+    {ROCKSDB_WRITE_BUFFER_SIZE,
+     [](const std::string &str, rocksdb::ColumnFamilyOptions &option) -> bool {
+         uint64_t val = 0;
+         if (!dsn::buf2uint64(str, val))
+             return false;
+         option.write_buffer_size = static_cast<size_t>(val);
+         return true;
+     }},
+    {ROCKSDB_NUM_LEVELS,
+     [](const std::string &str, rocksdb::ColumnFamilyOptions &option) -> bool {
+         int32_t val = 0;
+         if (!dsn::buf2int32(str, val))
+             return false;
+         option.num_levels = val;
+         return true;
+     }},
+};
+
+const std::unordered_map<std::string, cf_opts_getter> cf_opts_getters = {
+    {ROCKSDB_WRITE_BUFFER_SIZE,
+     [](/*out*/ std::string &str, const rocksdb::ColumnFamilyOptions &option) {
+         str = fmt::format("{}", option.write_buffer_size);
+     }},
+    {ROCKSDB_NUM_LEVELS,
+     [](/*out*/ std::string &str, const rocksdb::ColumnFamilyOptions &option) {
+         str = fmt::format("{}", option.num_levels);
+     }},
+};
 } // namespace pegasus
diff --git a/src/base/pegasus_const.h b/src/base/pegasus_const.h
index 7385c79411..c4db9662e5 100644
--- a/src/base/pegasus_const.h
+++ b/src/base/pegasus_const.h
@@ -19,8 +19,14 @@
 
 #pragma once
 
+#include <functional>
 #include <set>
 #include <string>
+#include <unordered_map>
+
+namespace rocksdb {
+struct ColumnFamilyOptions;
+} // namespace rocksdb
 
 namespace pegasus {
 
@@ -81,4 +87,11 @@ extern const std::string ROCKSDB_NUM_LEVELS;
 extern const std::set<std::string> ROCKSDB_DYNAMIC_OPTIONS;
 
 extern const std::set<std::string> ROCKSDB_STATIC_OPTIONS;
+
+using cf_opts_setter = std::function<bool(const std::string &, rocksdb::ColumnFamilyOptions &)>;
+extern const std::unordered_map<std::string, cf_opts_setter> cf_opts_setters;
+
+using cf_opts_getter =
+    std::function<void(/*out*/ std::string &, const rocksdb::ColumnFamilyOptions &)>;
+extern const std::unordered_map<std::string, cf_opts_getter> cf_opts_getters;
 } // namespace pegasus
diff --git a/src/meta/app_env_validator.cpp b/src/meta/app_env_validator.cpp
index fbc8eab467..7f197f5a0f 100644
--- a/src/meta/app_env_validator.cpp
+++ b/src/meta/app_env_validator.cpp
@@ -180,9 +180,9 @@ bool check_rocksdb_write_buffer_size(const std::string &env_value, std::string &
         hint_message = fmt::format("rocksdb.write_buffer_size cannot set this val: {}", env_value);
         return false;
     }
-    if (val < (32 << 20) || val > (512 << 20)) {
+    if (val < (16 << 20) || val > (512 << 20)) {
         hint_message =
-            fmt::format("rocksdb.write_buffer_size suggest set val in range [33554432, 536870912]");
+            fmt::format("rocksdb.write_buffer_size suggest set val in range [16777216, 536870912]");
         return false;
     }
     return true;
diff --git a/src/meta/test/meta_app_envs_test.cpp b/src/meta/test/meta_app_envs_test.cpp
index c115f80770..93ffc6d7b3 100644
--- a/src/meta/test/meta_app_envs_test.cpp
+++ b/src/meta/test/meta_app_envs_test.cpp
@@ -29,6 +29,7 @@
 #include <gtest/gtest.h>
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
 
 #include "common/replica_envs.h"
@@ -145,12 +146,12 @@ TEST_F(meta_app_envs_test, update_app_envs_test)
         {replica_envs::ROCKSDB_WRITE_BUFFER_SIZE,
          "100",
          ERR_INVALID_PARAMETERS,
-         "rocksdb.write_buffer_size suggest set val in range [33554432, 536870912]",
+         "rocksdb.write_buffer_size suggest set val in range [16777216, 536870912]",
          "67108864"},
         {replica_envs::ROCKSDB_WRITE_BUFFER_SIZE,
          "636870912",
          ERR_INVALID_PARAMETERS,
-         "rocksdb.write_buffer_size suggest set val in range [33554432, 536870912]",
+         "rocksdb.write_buffer_size suggest set val in range [16777216, 536870912]",
          "536870912"},
         {replica_envs::ROCKSDB_WRITE_BUFFER_SIZE, "67108864", ERR_OK, "", "67108864"},
         {replica_envs::MANUAL_COMPACT_PERIODIC_BOTTOMMOST_LEVEL_COMPACTION,
@@ -203,6 +204,18 @@ TEST_F(meta_app_envs_test, update_app_envs_test)
             ASSERT_EQ(app->envs.at(test.env_key), test.expect_value);
         }
     }
+
+    {
+        // Make sure all rocksdb options of ROCKSDB_DYNAMIC_OPTIONS are tested.
+        // Hint: Mainly verify the update_rocksdb_dynamic_options function.
+        std::map<std::string, std::string> all_test_envs;
+        for (auto test : tests) {
+            all_test_envs[test.env_key] = test.env_value;
+        }
+        for (const auto &option : replica_envs::ROCKSDB_DYNAMIC_OPTIONS) {
+            ASSERT_TRUE(all_test_envs.find(option) != all_test_envs.end());
+        }
+    }
 }
 
 } // namespace replication
diff --git a/src/meta/test/meta_app_operation_test.cpp b/src/meta/test/meta_app_operation_test.cpp
index c3ddafa72c..92686010ce 100644
--- a/src/meta/test/meta_app_operation_test.cpp
+++ b/src/meta/test/meta_app_operation_test.cpp
@@ -24,6 +24,7 @@
 #include <iostream>
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
 #include <utility>
 #include <vector>
@@ -68,7 +69,8 @@ class meta_app_operation_test : public meta_test_base
     error_code create_app_test(int32_t partition_count,
                                int32_t replica_count,
                                bool success_if_exist,
-                               const std::string &app_name)
+                               const std::string &app_name,
+                               const std::map<std::string, std::string> &envs = {})
     {
         configuration_create_app_request create_request;
         configuration_create_app_response create_response;
@@ -78,6 +80,7 @@ class meta_app_operation_test : public meta_test_base
         create_request.options.replica_count = replica_count;
         create_request.options.success_if_exist = success_if_exist;
         create_request.options.is_stateful = true;
+        create_request.options.envs = envs;
 
         auto result = fake_create_app(_ss.get(), create_request);
         fake_wait_rpc(result, create_response);
@@ -362,6 +365,12 @@ TEST_F(meta_app_operation_test, create_app)
     // - wrong app_status dropping
     // - create succeed with app_status dropped
     // - create succeed with success_if_exist=true
+    // - wrong rocksdb.num_levels (< 1)
+    // - wrong rocksdb.num_levels (> 10)
+    // - create app with rocksdb.num_levels (= 5) succeed
+    // - wrong rocksdb.write_buffer_size (< (16<<20))
+    // - wrong rocksdb.write_buffer_size (> (512<<20))
+    // - create app with rocksdb.write_buffer_size (= (32<<20)) succeed
     struct create_test
     {
         std::string app_name;
@@ -373,43 +382,106 @@ TEST_F(meta_app_operation_test, create_app)
         bool success_if_exist;
         app_status::type before_status;
         error_code expected_err;
-    } tests[] = {{APP_NAME, -1, 3, 2, 3, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 0, 3, 2, 3, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 4, -1, 1, 3, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 4, 0, 1, 3, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 4, 6, 2, 4, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 4, 7, 2, 6, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 4, 6, 2, 5, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 4, 5, 2, 4, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 4, 4, 2, 3, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 4, 6, 2, 6, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 4, 6, 2, 7, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME + "_1", 4, 5, 2, 5, 1, false, app_status::AS_INVALID, ERR_OK},
-                 {APP_NAME + "_2", 4, 5, 2, 6, 1, false, app_status::AS_INVALID, ERR_OK},
-                 {APP_NAME + "_3", 4, 4, 2, 4, 1, false, app_status::AS_INVALID, ERR_OK},
-                 {APP_NAME + "_4", 4, 4, 2, 6, 1, false, app_status::AS_INVALID, ERR_OK},
-                 {APP_NAME + "_5", 4, 3, 2, 4, 1, false, app_status::AS_INVALID, ERR_OK},
-                 {APP_NAME + "_6", 4, 4, 2, 5, 1, false, app_status::AS_INVALID, ERR_OK},
-                 {APP_NAME, 4, 3, 2, 5, 4, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 4, 3, 2, 4, 5, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 4, 3, 2, 4, 4, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 4, 3, 2, 2, 4, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 4, 3, 2, 3, 4, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME, 4, 4, 2, 3, 4, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
-                 {APP_NAME + "_7", 4, 3, 2, 4, 3, false, app_status::AS_INVALID, ERR_OK},
-                 {APP_NAME, 4, 1, 1, 0, 1, false, app_status::AS_INVALID, ERR_STATE_FREEZED},
-                 {APP_NAME, 4, 2, 2, 1, 1, false, app_status::AS_INVALID, ERR_STATE_FREEZED},
-                 {APP_NAME, 4, 3, 3, 2, 1, false, app_status::AS_INVALID, ERR_STATE_FREEZED},
-                 {APP_NAME + "_8", 4, 3, 3, 3, 1, false, app_status::AS_INVALID, ERR_OK},
-                 {APP_NAME + "_9", 4, 1, 1, 1, 1, false, app_status::AS_INVALID, ERR_OK},
-                 {APP_NAME + "_10", 4, 2, 1, 2, 2, false, app_status::AS_INVALID, ERR_OK},
-                 {APP_NAME, 4, 3, 2, 3, 3, false, app_status::AS_INVALID, ERR_OK},
-                 {APP_NAME, 4, 3, 2, 3, 3, false, app_status::AS_INVALID, ERR_APP_EXIST},
-                 {APP_NAME, 4, 3, 2, 3, 3, false, app_status::AS_CREATING, ERR_BUSY_CREATING},
-                 {APP_NAME, 4, 3, 2, 3, 3, false, app_status::AS_RECALLING, ERR_BUSY_CREATING},
-                 {APP_NAME, 4, 3, 2, 3, 3, false, app_status::AS_DROPPING, ERR_BUSY_DROPPING},
-                 {APP_NAME, 4, 3, 2, 3, 3, false, app_status::AS_DROPPED, ERR_OK},
-                 {APP_NAME, 4, 3, 2, 3, 3, true, app_status::AS_INVALID, ERR_OK}};
+        std::map<std::string, std::string> envs = {};
+    } tests[] = {
+        {APP_NAME, -1, 3, 2, 3, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 0, 3, 2, 3, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 4, -1, 1, 3, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 4, 0, 1, 3, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 4, 6, 2, 4, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 4, 7, 2, 6, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 4, 6, 2, 5, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 4, 5, 2, 4, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 4, 4, 2, 3, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 4, 6, 2, 6, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 4, 6, 2, 7, 1, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME + "_1", 4, 5, 2, 5, 1, false, app_status::AS_INVALID, ERR_OK},
+        {APP_NAME + "_2", 4, 5, 2, 6, 1, false, app_status::AS_INVALID, ERR_OK},
+        {APP_NAME + "_3", 4, 4, 2, 4, 1, false, app_status::AS_INVALID, ERR_OK},
+        {APP_NAME + "_4", 4, 4, 2, 6, 1, false, app_status::AS_INVALID, ERR_OK},
+        {APP_NAME + "_5", 4, 3, 2, 4, 1, false, app_status::AS_INVALID, ERR_OK},
+        {APP_NAME + "_6", 4, 4, 2, 5, 1, false, app_status::AS_INVALID, ERR_OK},
+        {APP_NAME, 4, 3, 2, 5, 4, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 4, 3, 2, 4, 5, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 4, 3, 2, 4, 4, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 4, 3, 2, 2, 4, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 4, 3, 2, 3, 4, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME, 4, 4, 2, 3, 4, false, app_status::AS_INVALID, ERR_INVALID_PARAMETERS},
+        {APP_NAME + "_7", 4, 3, 2, 4, 3, false, app_status::AS_INVALID, ERR_OK},
+        {APP_NAME, 4, 1, 1, 0, 1, false, app_status::AS_INVALID, ERR_STATE_FREEZED},
+        {APP_NAME, 4, 2, 2, 1, 1, false, app_status::AS_INVALID, ERR_STATE_FREEZED},
+        {APP_NAME, 4, 3, 3, 2, 1, false, app_status::AS_INVALID, ERR_STATE_FREEZED},
+        {APP_NAME + "_8", 4, 3, 3, 3, 1, false, app_status::AS_INVALID, ERR_OK},
+        {APP_NAME + "_9", 4, 1, 1, 1, 1, false, app_status::AS_INVALID, ERR_OK},
+        {APP_NAME + "_10", 4, 2, 1, 2, 2, false, app_status::AS_INVALID, ERR_OK},
+        {APP_NAME, 4, 3, 2, 3, 3, false, app_status::AS_INVALID, ERR_OK},
+        {APP_NAME, 4, 3, 2, 3, 3, false, app_status::AS_INVALID, ERR_APP_EXIST},
+        {APP_NAME, 4, 3, 2, 3, 3, false, app_status::AS_CREATING, ERR_BUSY_CREATING},
+        {APP_NAME, 4, 3, 2, 3, 3, false, app_status::AS_RECALLING, ERR_BUSY_CREATING},
+        {APP_NAME, 4, 3, 2, 3, 3, false, app_status::AS_DROPPING, ERR_BUSY_DROPPING},
+        {APP_NAME, 4, 3, 2, 3, 3, false, app_status::AS_DROPPED, ERR_OK},
+        {APP_NAME, 4, 3, 2, 3, 3, true, app_status::AS_INVALID, ERR_OK},
+        {APP_NAME,
+         4,
+         3,
+         2,
+         3,
+         3,
+         false,
+         app_status::AS_INVALID,
+         ERR_INVALID_PARAMETERS,
+         {{"rocksdb.num_levels", "0"}}},
+        {APP_NAME,
+         4,
+         3,
+         2,
+         3,
+         3,
+         false,
+         app_status::AS_INVALID,
+         ERR_INVALID_PARAMETERS,
+         {{"rocksdb.num_levels", "11"}}},
+        {APP_NAME + "_11",
+         4,
+         3,
+         2,
+         3,
+         3,
+         false,
+         app_status::AS_INVALID,
+         ERR_OK,
+         {{"rocksdb.num_levels", "5"}}},
+        {APP_NAME,
+         4,
+         3,
+         2,
+         3,
+         3,
+         false,
+         app_status::AS_INVALID,
+         ERR_INVALID_PARAMETERS,
+         {{"rocksdb.write_buffer_size", "1000"}}},
+        {APP_NAME,
+         4,
+         3,
+         2,
+         3,
+         3,
+         false,
+         app_status::AS_INVALID,
+         ERR_INVALID_PARAMETERS,
+         {{"rocksdb.write_buffer_size", "1073741824"}}},
+        {APP_NAME + "_12",
+         4,
+         3,
+         2,
+         3,
+         3,
+         false,
+         app_status::AS_INVALID,
+         ERR_OK,
+         {{"rocksdb.write_buffer_size", "33554432"}}},
+    };
 
     clear_nodes();
 
@@ -453,13 +525,30 @@ TEST_F(meta_app_operation_test, create_app)
         } else if (test.before_status != app_status::AS_INVALID) {
             update_app_status(test.before_status);
         }
-        auto err = create_app_test(
-            test.partition_count, test.replica_count, test.success_if_exist, test.app_name);
+        auto err = create_app_test(test.partition_count,
+                                   test.replica_count,
+                                   test.success_if_exist,
+                                   test.app_name,
+                                   test.envs);
         ASSERT_EQ(err, test.expected_err);
 
         _ms->set_node_state(nodes, true);
     }
 
+    {
+        // Make sure all rocksdb options of ROCKSDB_DYNAMIC_OPTIONS and ROCKSDB_STATIC_OPTIONS are
+        // tested. Hint: Mainly verify the validate_app_envs function.
+        std::map<std::string, std::string> all_test_envs;
+        for (auto test : tests) {
+            all_test_envs.insert(test.envs.begin(), test.envs.end());
+        }
+        for (const auto &option : replica_envs::ROCKSDB_DYNAMIC_OPTIONS) {
+            ASSERT_TRUE(all_test_envs.find(option) != all_test_envs.end());
+        }
+        for (const auto &option : replica_envs::ROCKSDB_STATIC_OPTIONS) {
+            ASSERT_TRUE(all_test_envs.find(option) != all_test_envs.end());
+        }
+    }
     // set FLAGS_min_allowed_replica_count successfully
     res = update_flag("min_allowed_replica_count", "2");
     ASSERT_TRUE(res.is_ok());
diff --git a/src/server/pegasus_server_impl.cpp b/src/server/pegasus_server_impl.cpp
index 81c109f10e..7f6d7fc6c5 100644
--- a/src/server/pegasus_server_impl.cpp
+++ b/src/server/pegasus_server_impl.cpp
@@ -38,6 +38,7 @@
 #include <unistd.h> // IWYU pragma: keep
 #include <algorithm>
 #include <cstdint>
+#include <functional>
 #include <limits>
 #include <list>
 #include <mutex>
@@ -2629,17 +2630,9 @@ pegasus_server_impl::get_restore_dir_from_env(const std::map<std::string, std::s
 void pegasus_server_impl::update_rocksdb_dynamic_options(
     const std::map<std::string, std::string> &envs)
 {
-    if (envs.size() == 0) {
+    if (envs.empty()) {
         return;
     }
-    auto extract_option = [](const std::string &option) -> std::string {
-        std::stringstream ss(option);
-        std::string prefix, rocksdb_opt;
-        std::getline(ss, prefix, '.');
-        std::getline(ss, rocksdb_opt);
-        LOG_INFO("Extract rocksdb dynamic opt ({}) from ({})", rocksdb_opt, option);
-        return rocksdb_opt;
-    };
 
     std::unordered_map<std::string, std::string> new_options;
     for (const auto &option : ROCKSDB_DYNAMIC_OPTIONS) {
@@ -2647,11 +2640,15 @@ void pegasus_server_impl::update_rocksdb_dynamic_options(
         if (find == envs.end()) {
             continue;
         }
-        new_options[extract_option(option)] = find->second;
+
+        std::vector<std::string> args;
+        // split_args example: Parse "write_buffer_size" from "rocksdb.write_buffer_size"
+        dsn::utils::split_args(option.c_str(), args, '.');
+        new_options[args[1]] = find->second;
     }
 
     // doing set option
-    if (new_options.empty() && set_options(new_options)) {
+    if (!new_options.empty() && set_options(new_options)) {
         LOG_INFO("Set rocksdb dynamic options success");
     }
 }
@@ -2659,7 +2656,7 @@ void pegasus_server_impl::update_rocksdb_dynamic_options(
 void pegasus_server_impl::set_rocksdb_options_before_creating(
     const std::map<std::string, std::string> &envs)
 {
-    if (envs.size() == 0) {
+    if (envs.empty()) {
         return;
     }
 
@@ -2668,17 +2665,15 @@ void pegasus_server_impl::set_rocksdb_options_before_creating(
         if (find == envs.end()) {
             continue;
         }
-        bool is_set = false;
-        if (option.compare(ROCKSDB_NUM_LEVELS) == 0) {
-            int32_t val = 0;
-            if (!dsn::buf2int32(find->second, val))
-                continue;
-            is_set = true;
-            _data_cf_opts.num_levels = val;
-        }
 
-        if (is_set)
-            LOG_INFO("Set {} \"{}\" succeed", find->first, find->second);
+        auto setter = cf_opts_setters.find(option);
+        if (setter == cf_opts_setters.end()) {
+            LOG_WARNING("cannot find {} setter function, and set this option fail.", option);
+            continue;
+        }
+        if (setter->second(find->second, _data_cf_opts)) {
+            LOG_INFO_PREFIX("Set {} \"{}\" succeed", find->first, find->second);
+        }
     }
 
     for (const auto &option : pegasus::ROCKSDB_DYNAMIC_OPTIONS) {
@@ -2686,16 +2681,15 @@ void pegasus_server_impl::set_rocksdb_options_before_creating(
         if (find == envs.end()) {
             continue;
         }
-        bool is_set = false;
-        if (option.compare(ROCKSDB_WRITE_BUFFER_SIZE) == 0) {
-            uint64_t val = 0;
-            if (!dsn::buf2uint64(find->second, val))
-                continue;
-            is_set = true;
-            _data_cf_opts.write_buffer_size = static_cast<size_t>(val);
+
+        auto setter = cf_opts_setters.find(option);
+        if (setter == cf_opts_setters.end()) {
+            LOG_WARNING("cannot find {} setter function, and set this option fail.", option);
+            continue;
+        }
+        if (setter->second(find->second, _data_cf_opts)) {
+            LOG_INFO_PREFIX("Set {} \"{}\" succeed", find->first, find->second);
         }
-        if (is_set)
-            LOG_INFO("Set {} \"{}\" succeed", find->first, find->second);
     }
 }
 
@@ -2731,6 +2725,14 @@ void pegasus_server_impl::update_app_envs_before_open_db(
 void pegasus_server_impl::query_app_envs(/*out*/ std::map<std::string, std::string> &envs)
 {
     envs[ROCKSDB_ENV_USAGE_SCENARIO_KEY] = _usage_scenario;
+    // write_buffer_size involves random values (refer to pegasus_server_impl::set_usage_scenario),
+    // so it can only be taken from _data_cf_opts
+    envs[ROCKSDB_WRITE_BUFFER_SIZE] = fmt::format("{}", _data_cf_opts.write_buffer_size);
+
+    // Get Data ColumnFamilyOptions directly from _data_cf
+    rocksdb::ColumnFamilyDescriptor desc;
+    rocksdb::Status s = _data_cf->GetDescriptor(&desc);
+    envs[ROCKSDB_NUM_LEVELS] = fmt::format("{}", desc.options.num_levels);
 }
 
 void pegasus_server_impl::update_usage_scenario(const std::map<std::string, std::string> &envs)
@@ -3117,6 +3119,7 @@ bool pegasus_server_impl::set_usage_scenario(const std::string &usage_scenario)
 void pegasus_server_impl::reset_rocksdb_options(const rocksdb::ColumnFamilyOptions &base_opts,
                                                 rocksdb::ColumnFamilyOptions *target_opts)
 {
+    LOG_INFO_PREFIX("Reset rocksdb envs options");
     // Reset rocksdb option includes two aspects:
     // 1. Set usage_scenario related rocksdb options
     // 2. Rocksdb option set in app envs, consists of ROCKSDB_DYNAMIC_OPTIONS and
diff --git a/src/server/test/pegasus_server_impl_test.cpp b/src/server/test/pegasus_server_impl_test.cpp
index 1363d8054b..ad28a822db 100644
--- a/src/server/test/pegasus_server_impl_test.cpp
+++ b/src/server/test/pegasus_server_impl_test.cpp
@@ -29,7 +29,9 @@
 #include <stdint.h>
 #include <map>
 #include <memory>
+#include <set>
 #include <string>
+#include <utility>
 
 #include "pegasus_const.h"
 #include "pegasus_server_test_base.h"
@@ -95,6 +97,51 @@ class pegasus_server_impl_test : public pegasus_server_test_base
             ASSERT_EQ(before_count + test.expect_perf_counter_incr, after_count);
         }
     }
+
+    void test_open_db_with_rocksdb_envs(bool is_restart)
+    {
+        struct create_test
+        {
+            std::string env_key;
+            std::string env_value;
+            std::string expect_value;
+        } tests[] = {
+            {"rocksdb.num_levels", "5", "5"}, {"rocksdb.write_buffer_size", "33554432", "33554432"},
+        };
+
+        std::map<std::string, std::string> all_test_envs;
+        {
+            // Make sure all rocksdb options of ROCKSDB_DYNAMIC_OPTIONS and ROCKSDB_STATIC_OPTIONS
+            // are tested.
+            for (auto test : tests) {
+                all_test_envs[test.env_key] = test.env_value;
+            }
+            for (const auto &option : pegasus::ROCKSDB_DYNAMIC_OPTIONS) {
+                ASSERT_TRUE(all_test_envs.find(option) != all_test_envs.end());
+            }
+            for (const auto &option : pegasus::ROCKSDB_STATIC_OPTIONS) {
+                ASSERT_TRUE(all_test_envs.find(option) != all_test_envs.end());
+            }
+        }
+
+        start(all_test_envs);
+        if (is_restart) {
+            _server->stop(false);
+            start();
+        }
+
+        std::map<std::string, std::string> query_envs;
+        _server->query_app_envs(query_envs);
+        for (auto test : tests) {
+            auto iter = query_envs.find(test.env_key);
+            if (iter != query_envs.end()) {
+                ASSERT_EQ(iter->second, test.expect_value);
+            } else {
+                ASSERT_EQ(test.env_key,
+                          fmt::format("query_app_envs not supported {}", test.env_key));
+            }
+        }
+    }
 };
 
 TEST_F(pegasus_server_impl_test, test_table_level_slow_query)
@@ -137,6 +184,18 @@ TEST_F(pegasus_server_impl_test, test_open_db_with_app_envs)
     ASSERT_EQ(ROCKSDB_ENV_USAGE_SCENARIO_BULK_LOAD, _server->_usage_scenario);
 }
 
+TEST_F(pegasus_server_impl_test, test_open_db_with_rocksdb_envs)
+{
+    // Hint: Verify the set_rocksdb_options_before_creating function by boolean is_restart=false.
+    test_open_db_with_rocksdb_envs(false);
+}
+
+TEST_F(pegasus_server_impl_test, test_restart_db_with_rocksdb_envs)
+{
+    // Hint: Verify the reset_rocksdb_options function by boolean is_restart=true.
+    test_open_db_with_rocksdb_envs(true);
+}
+
 TEST_F(pegasus_server_impl_test, test_stop_db_twice)
 {
     start();

From d5cdf98cafb74e5fae4a7e7b5c0c89395b0d0b40 Mon Sep 17 00:00:00 2001
From: Pengfan Lu <lu_peng_fan@163.com>
Date: Tue, 11 Jul 2023 08:45:46 +0000
Subject: [PATCH 08/10] Reply to empiredan's suggestion and modify my code.

---
 src/base/pegasus_const.cpp                   | 10 ++++---
 src/base/pegasus_const.h                     |  2 +-
 src/meta/test/meta_app_envs_test.cpp         |  2 +-
 src/meta/test/meta_app_operation_test.cpp    | 24 ++++++++++++++++-
 src/server/pegasus_server_impl.cpp           | 28 ++++++++++++++++++--
 src/server/test/pegasus_server_impl_test.cpp |  9 +++----
 6 files changed, 61 insertions(+), 14 deletions(-)

diff --git a/src/base/pegasus_const.cpp b/src/base/pegasus_const.cpp
index 857ce7d824..fce60886bf 100644
--- a/src/base/pegasus_const.cpp
+++ b/src/base/pegasus_const.cpp
@@ -124,16 +124,18 @@ const std::unordered_map<std::string, cf_opts_setter> cf_opts_setters = {
     {ROCKSDB_WRITE_BUFFER_SIZE,
      [](const std::string &str, rocksdb::ColumnFamilyOptions &option) -> bool {
          uint64_t val = 0;
-         if (!dsn::buf2uint64(str, val))
+         if (!dsn::buf2uint64(str, val)) {
              return false;
+         }
          option.write_buffer_size = static_cast<size_t>(val);
          return true;
      }},
     {ROCKSDB_NUM_LEVELS,
      [](const std::string &str, rocksdb::ColumnFamilyOptions &option) -> bool {
          int32_t val = 0;
-         if (!dsn::buf2int32(str, val))
+         if (!dsn::buf2int32(str, val)) {
              return false;
+         }
          option.num_levels = val;
          return true;
      }},
@@ -141,11 +143,11 @@ const std::unordered_map<std::string, cf_opts_setter> cf_opts_setters = {
 
 const std::unordered_map<std::string, cf_opts_getter> cf_opts_getters = {
     {ROCKSDB_WRITE_BUFFER_SIZE,
-     [](/*out*/ std::string &str, const rocksdb::ColumnFamilyOptions &option) {
+     [](const rocksdb::ColumnFamilyOptions &option, /*out*/ std::string &str) {
          str = fmt::format("{}", option.write_buffer_size);
      }},
     {ROCKSDB_NUM_LEVELS,
-     [](/*out*/ std::string &str, const rocksdb::ColumnFamilyOptions &option) {
+     [](const rocksdb::ColumnFamilyOptions &option, /*out*/ std::string &str) {
          str = fmt::format("{}", option.num_levels);
      }},
 };
diff --git a/src/base/pegasus_const.h b/src/base/pegasus_const.h
index c4db9662e5..e9326dfaa9 100644
--- a/src/base/pegasus_const.h
+++ b/src/base/pegasus_const.h
@@ -92,6 +92,6 @@ using cf_opts_setter = std::function<bool(const std::string &, rocksdb::ColumnFa
 extern const std::unordered_map<std::string, cf_opts_setter> cf_opts_setters;
 
 using cf_opts_getter =
-    std::function<void(/*out*/ std::string &, const rocksdb::ColumnFamilyOptions &)>;
+    std::function<void(const rocksdb::ColumnFamilyOptions &, /*out*/ std::string &)>;
 extern const std::unordered_map<std::string, cf_opts_getter> cf_opts_getters;
 } // namespace pegasus
diff --git a/src/meta/test/meta_app_envs_test.cpp b/src/meta/test/meta_app_envs_test.cpp
index 93ffc6d7b3..d5dc21edac 100644
--- a/src/meta/test/meta_app_envs_test.cpp
+++ b/src/meta/test/meta_app_envs_test.cpp
@@ -209,7 +209,7 @@ TEST_F(meta_app_envs_test, update_app_envs_test)
         // Make sure all rocksdb options of ROCKSDB_DYNAMIC_OPTIONS are tested.
         // Hint: Mainly verify the update_rocksdb_dynamic_options function.
         std::map<std::string, std::string> all_test_envs;
-        for (auto test : tests) {
+        for (const auto &test : tests) {
             all_test_envs[test.env_key] = test.env_value;
         }
         for (const auto &option : replica_envs::ROCKSDB_DYNAMIC_OPTIONS) {
diff --git a/src/meta/test/meta_app_operation_test.cpp b/src/meta/test/meta_app_operation_test.cpp
index 92686010ce..e0afc1baa5 100644
--- a/src/meta/test/meta_app_operation_test.cpp
+++ b/src/meta/test/meta_app_operation_test.cpp
@@ -367,9 +367,11 @@ TEST_F(meta_app_operation_test, create_app)
     // - create succeed with success_if_exist=true
     // - wrong rocksdb.num_levels (< 1)
     // - wrong rocksdb.num_levels (> 10)
+    // - wrong rocksdb.num_levels (non-digital character)
     // - create app with rocksdb.num_levels (= 5) succeed
     // - wrong rocksdb.write_buffer_size (< (16<<20))
     // - wrong rocksdb.write_buffer_size (> (512<<20))
+    // - wrong rocksdb.write_buffer_size (non-digital character)
     // - create app with rocksdb.write_buffer_size (= (32<<20)) succeed
     struct create_test
     {
@@ -441,6 +443,16 @@ TEST_F(meta_app_operation_test, create_app)
          app_status::AS_INVALID,
          ERR_INVALID_PARAMETERS,
          {{"rocksdb.num_levels", "11"}}},
+        {APP_NAME + "_11",
+         4,
+         3,
+         2,
+         3,
+         3,
+         false,
+         app_status::AS_INVALID,
+         ERR_INVALID_PARAMETERS,
+         {{"rocksdb.num_levels", "5i"}}},
         {APP_NAME + "_11",
          4,
          3,
@@ -471,6 +483,16 @@ TEST_F(meta_app_operation_test, create_app)
          app_status::AS_INVALID,
          ERR_INVALID_PARAMETERS,
          {{"rocksdb.write_buffer_size", "1073741824"}}},
+        {APP_NAME,
+         4,
+         3,
+         2,
+         3,
+         3,
+         false,
+         app_status::AS_INVALID,
+         ERR_INVALID_PARAMETERS,
+         {{"rocksdb.write_buffer_size", "n33554432"}}},
         {APP_NAME + "_12",
          4,
          3,
@@ -539,7 +561,7 @@ TEST_F(meta_app_operation_test, create_app)
         // Make sure all rocksdb options of ROCKSDB_DYNAMIC_OPTIONS and ROCKSDB_STATIC_OPTIONS are
         // tested. Hint: Mainly verify the validate_app_envs function.
         std::map<std::string, std::string> all_test_envs;
-        for (auto test : tests) {
+        for (const auto &test : tests) {
             all_test_envs.insert(test.envs.begin(), test.envs.end());
         }
         for (const auto &option : replica_envs::ROCKSDB_DYNAMIC_OPTIONS) {
diff --git a/src/server/pegasus_server_impl.cpp b/src/server/pegasus_server_impl.cpp
index 7f6d7fc6c5..4b2d3d106b 100644
--- a/src/server/pegasus_server_impl.cpp
+++ b/src/server/pegasus_server_impl.cpp
@@ -2644,6 +2644,7 @@ void pegasus_server_impl::update_rocksdb_dynamic_options(
         std::vector<std::string> args;
         // split_args example: Parse "write_buffer_size" from "rocksdb.write_buffer_size"
         dsn::utils::split_args(option.c_str(), args, '.');
+        CHECK_EQ(args.size(), 2);
         new_options[args[1]] = find->second;
     }
 
@@ -2731,8 +2732,31 @@ void pegasus_server_impl::query_app_envs(/*out*/ std::map<std::string, std::stri
 
     // Get Data ColumnFamilyOptions directly from _data_cf
     rocksdb::ColumnFamilyDescriptor desc;
-    rocksdb::Status s = _data_cf->GetDescriptor(&desc);
-    envs[ROCKSDB_NUM_LEVELS] = fmt::format("{}", desc.options.num_levels);
+    auto s = _data_cf->GetDescriptor(&desc);
+    CHECK_TRUE(s.ok());
+    for (const auto &option : pegasus::ROCKSDB_STATIC_OPTIONS) {
+        auto getter = cf_opts_getters.find(option);
+        if (getter == cf_opts_getters.end()) {
+            LOG_WARNING("cannot find {} getter function, and get this option fail.", option);
+            continue;
+        }
+        std::string option_val;
+        getter->second(desc.options, option_val);
+        envs[option] = option_val;
+    }
+    for (const auto &option : pegasus::ROCKSDB_DYNAMIC_OPTIONS) {
+        if (option.compare(ROCKSDB_WRITE_BUFFER_SIZE) == 0) {
+            continue;
+        }
+        auto getter = cf_opts_getters.find(option);
+        if (getter == cf_opts_getters.end()) {
+            LOG_WARNING("cannot find {} getter function, and get this option fail.", option);
+            continue;
+        }
+        std::string option_val;
+        getter->second(desc.options, option_val);
+        envs[option] = option_val;
+    }
 }
 
 void pegasus_server_impl::update_usage_scenario(const std::map<std::string, std::string> &envs)
diff --git a/src/server/test/pegasus_server_impl_test.cpp b/src/server/test/pegasus_server_impl_test.cpp
index ad28a822db..9776571ae9 100644
--- a/src/server/test/pegasus_server_impl_test.cpp
+++ b/src/server/test/pegasus_server_impl_test.cpp
@@ -113,7 +113,7 @@ class pegasus_server_impl_test : public pegasus_server_test_base
         {
             // Make sure all rocksdb options of ROCKSDB_DYNAMIC_OPTIONS and ROCKSDB_STATIC_OPTIONS
             // are tested.
-            for (auto test : tests) {
+            for (const auto &test : tests) {
                 all_test_envs[test.env_key] = test.env_value;
             }
             for (const auto &option : pegasus::ROCKSDB_DYNAMIC_OPTIONS) {
@@ -132,13 +132,12 @@ class pegasus_server_impl_test : public pegasus_server_test_base
 
         std::map<std::string, std::string> query_envs;
         _server->query_app_envs(query_envs);
-        for (auto test : tests) {
-            auto iter = query_envs.find(test.env_key);
+        for (const auto &test : tests) {
+            const auto &iter = query_envs.find(test.env_key);
             if (iter != query_envs.end()) {
                 ASSERT_EQ(iter->second, test.expect_value);
             } else {
-                ASSERT_EQ(test.env_key,
-                          fmt::format("query_app_envs not supported {}", test.env_key));
+                ASSERT_TRUE(false) << fmt::format("query_app_envs not supported {}", test.env_key);
             }
         }
     }

From 31886f1447a956a1dcd76c1c25914de48b35476d Mon Sep 17 00:00:00 2001
From: Pengfan Lu <lu_peng_fan@163.com>
Date: Sat, 29 Jul 2023 13:16:02 +0000
Subject: [PATCH 09/10] Reply to empiredan's suggestion and modify my code.

---
 src/base/pegasus_const.cpp         |  5 ++---
 src/server/pegasus_server_impl.cpp | 25 +++++++++----------------
 2 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/src/base/pegasus_const.cpp b/src/base/pegasus_const.cpp
index fce60886bf..2a788cd24d 100644
--- a/src/base/pegasus_const.cpp
+++ b/src/base/pegasus_const.cpp
@@ -19,7 +19,6 @@
 
 #include "pegasus_const.h"
 
-#include <fmt/core.h>
 #include <rocksdb/options.h>
 #include <stddef.h>
 #include <stdint.h>
@@ -144,11 +143,11 @@ const std::unordered_map<std::string, cf_opts_setter> cf_opts_setters = {
 const std::unordered_map<std::string, cf_opts_getter> cf_opts_getters = {
     {ROCKSDB_WRITE_BUFFER_SIZE,
      [](const rocksdb::ColumnFamilyOptions &option, /*out*/ std::string &str) {
-         str = fmt::format("{}", option.write_buffer_size);
+         str = std::to_string(option.write_buffer_size);
      }},
     {ROCKSDB_NUM_LEVELS,
      [](const rocksdb::ColumnFamilyOptions &option, /*out*/ std::string &str) {
-         str = fmt::format("{}", option.num_levels);
+         str = std::to_string(option.num_levels);
      }},
 };
 } // namespace pegasus
diff --git a/src/server/pegasus_server_impl.cpp b/src/server/pegasus_server_impl.cpp
index 4b2d3d106b..982a7de7b0 100644
--- a/src/server/pegasus_server_impl.cpp
+++ b/src/server/pegasus_server_impl.cpp
@@ -2636,7 +2636,7 @@ void pegasus_server_impl::update_rocksdb_dynamic_options(
 
     std::unordered_map<std::string, std::string> new_options;
     for (const auto &option : ROCKSDB_DYNAMIC_OPTIONS) {
-        auto find = envs.find(option);
+        const auto &find = envs.find(option);
         if (find == envs.end()) {
             continue;
         }
@@ -2662,12 +2662,12 @@ void pegasus_server_impl::set_rocksdb_options_before_creating(
     }
 
     for (const auto &option : pegasus::ROCKSDB_STATIC_OPTIONS) {
-        auto find = envs.find(option);
+        const auto &find = envs.find(option);
         if (find == envs.end()) {
             continue;
         }
 
-        auto setter = cf_opts_setters.find(option);
+        const auto &setter = cf_opts_setters.find(option);
         if (setter == cf_opts_setters.end()) {
             LOG_WARNING("cannot find {} setter function, and set this option fail.", option);
             continue;
@@ -2678,12 +2678,12 @@ void pegasus_server_impl::set_rocksdb_options_before_creating(
     }
 
     for (const auto &option : pegasus::ROCKSDB_DYNAMIC_OPTIONS) {
-        auto find = envs.find(option);
+        const auto &find = envs.find(option);
         if (find == envs.end()) {
             continue;
         }
 
-        auto setter = cf_opts_setters.find(option);
+        const auto &setter = cf_opts_setters.find(option);
         if (setter == cf_opts_setters.end()) {
             LOG_WARNING("cannot find {} setter function, and set this option fail.", option);
             continue;
@@ -2728,18 +2728,14 @@ void pegasus_server_impl::query_app_envs(/*out*/ std::map<std::string, std::stri
     envs[ROCKSDB_ENV_USAGE_SCENARIO_KEY] = _usage_scenario;
     // write_buffer_size involves random values (refer to pegasus_server_impl::set_usage_scenario),
     // so it can only be taken from _data_cf_opts
-    envs[ROCKSDB_WRITE_BUFFER_SIZE] = fmt::format("{}", _data_cf_opts.write_buffer_size);
+    envs[ROCKSDB_WRITE_BUFFER_SIZE] = std::to_string(_data_cf_opts.write_buffer_size);
 
     // Get Data ColumnFamilyOptions directly from _data_cf
     rocksdb::ColumnFamilyDescriptor desc;
-    auto s = _data_cf->GetDescriptor(&desc);
-    CHECK_TRUE(s.ok());
+    CHECK_TRUE(_data_cf->GetDescriptor(&desc).ok());
     for (const auto &option : pegasus::ROCKSDB_STATIC_OPTIONS) {
         auto getter = cf_opts_getters.find(option);
-        if (getter == cf_opts_getters.end()) {
-            LOG_WARNING("cannot find {} getter function, and get this option fail.", option);
-            continue;
-        }
+        CHECK_TRUE(getter != cf_opts_getters.end());
         std::string option_val;
         getter->second(desc.options, option_val);
         envs[option] = option_val;
@@ -2749,10 +2745,7 @@ void pegasus_server_impl::query_app_envs(/*out*/ std::map<std::string, std::stri
             continue;
         }
         auto getter = cf_opts_getters.find(option);
-        if (getter == cf_opts_getters.end()) {
-            LOG_WARNING("cannot find {} getter function, and get this option fail.", option);
-            continue;
-        }
+        CHECK_TRUE(getter != cf_opts_getters.end());
         std::string option_val;
         getter->second(desc.options, option_val);
         envs[option] = option_val;

From 4a17c4daa5f4aa39a14870a35d0b206fdd78aa3c Mon Sep 17 00:00:00 2001
From: lupengfan <lu_peng_fan@163.com>
Date: Tue, 1 Aug 2023 10:54:39 +0800
Subject: [PATCH 10/10] fix iwyu

---
 src/server/pegasus_server_impl.cpp | 10 ++--------
 src/utils/simple_logger.cpp        |  1 +
 2 files changed, 3 insertions(+), 8 deletions(-)

diff --git a/src/server/pegasus_server_impl.cpp b/src/server/pegasus_server_impl.cpp
index 982a7de7b0..998b4873f7 100644
--- a/src/server/pegasus_server_impl.cpp
+++ b/src/server/pegasus_server_impl.cpp
@@ -2668,10 +2668,7 @@ void pegasus_server_impl::set_rocksdb_options_before_creating(
         }
 
         const auto &setter = cf_opts_setters.find(option);
-        if (setter == cf_opts_setters.end()) {
-            LOG_WARNING("cannot find {} setter function, and set this option fail.", option);
-            continue;
-        }
+        CHECK_TRUE(setter != cf_opts_setters.end());
         if (setter->second(find->second, _data_cf_opts)) {
             LOG_INFO_PREFIX("Set {} \"{}\" succeed", find->first, find->second);
         }
@@ -2684,10 +2681,7 @@ void pegasus_server_impl::set_rocksdb_options_before_creating(
         }
 
         const auto &setter = cf_opts_setters.find(option);
-        if (setter == cf_opts_setters.end()) {
-            LOG_WARNING("cannot find {} setter function, and set this option fail.", option);
-            continue;
-        }
+        CHECK_TRUE(setter != cf_opts_setters.end());
         if (setter->second(find->second, _data_cf_opts)) {
             LOG_INFO_PREFIX("Set {} \"{}\" succeed", find->first, find->second);
         }
diff --git a/src/utils/simple_logger.cpp b/src/utils/simple_logger.cpp
index f982670186..142c3de233 100644
--- a/src/utils/simple_logger.cpp
+++ b/src/utils/simple_logger.cpp
@@ -41,6 +41,7 @@
 #include "utils/filesystem.h"
 #include "utils/flags.h"
 #include "utils/fmt_logging.h"
+#include "utils/ports.h"
 #include "utils/process_utils.h"
 #include "utils/strings.h"
 #include "utils/time_utils.h"