empiredan commented on code in PR #1511:
URL:
https://github.com/apache/incubator-pegasus/pull/1511#discussion_r1229214592
##########
src/server/pegasus_server_impl.h:
##########
@@ -468,6 +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;
Review Comment:
```suggestion
rocksdb::BlockBasedTableOptions _tbl_opts;
```
##########
src/meta/app_env_validator.cpp:
##########
@@ -31,6 +32,28 @@
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())
+ 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);
+ all_envs_vaild = false;
+ break;
+ }
+ }
+ return all_envs_vaild;
Review Comment:
```suggestion
return true;
```
##########
src/server/pegasus_server_impl.cpp:
##########
@@ -2625,6 +2626,62 @@ 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 (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;
+ }
Review Comment:
```suggestion
if (find == envs.end()) {
continue;
}
// Or, is it possible to extract the postfix of
ROCKSDB_WRITE_BUFFER_SIZE as the key of new_options ?
// For example, `new_options[postfix(option)] = find->second;`
if (option.compare(ROCKSDB_WRITE_BUFFER_SIZE) == 0) {
new_options["write_buffer_size"] = find->second;
}
```
##########
src/meta/app_env_validator.cpp:
##########
@@ -31,6 +32,28 @@
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;
Review Comment:
```suggestion
```
##########
src/meta/app_env_validator.cpp:
##########
@@ -31,6 +32,28 @@
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())
+ continue;
Review Comment:
```suggestion
replica_envs::ROCKSDB_DYNAMIC_OPTIONS.end()) {
continue;
}
```
##########
src/meta/app_env_validator.cpp:
##########
@@ -31,6 +32,28 @@
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())
+ 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);
+ all_envs_vaild = false;
+ break;
Review Comment:
```suggestion
return false;
```
##########
src/server/pegasus_server_impl.cpp:
##########
@@ -2625,6 +2626,62 @@ 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 (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;
+ }
+ }
+
+ // doing set option
+ if (new_options.size() > 0 && set_options(new_options)) {
Review Comment:
```suggestion
if (!new_options.empty() && set_options(new_options)) {
```
##########
src/meta/app_env_validator.cpp:
##########
@@ -31,6 +32,28 @@
namespace dsn {
namespace replication {
+bool validate_app_envs(const std::map<std::string, std::string> &envs)
+{
+ if (envs.size() == 0)
+ return true;
Review Comment:
```suggestion
```
##########
src/meta/app_env_validator.cpp:
##########
@@ -31,6 +32,28 @@
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())
+ continue;
+ if (!validate_app_env(it.first, it.second, hint_message)) {
Review Comment:
```suggestion
std::string hint_message;
if (!validate_app_env(it.first, it.second, hint_message)) {
```
##########
src/server/pegasus_server_impl.cpp:
##########
@@ -2625,6 +2626,62 @@ 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;
Review Comment:
```suggestion
if (envs.empty()) {
return;
}
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
