Feature:Online Query and Dynamic Modification of Table-level RocksDB …

…Options (#1488) #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
apache · Jun 9, 2023 · b6c0c14 · b6c0c14
1 parent 2acae8f
commit b6c0c14
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 15 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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,10 @@ 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 +2669,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);
+            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);