acelyc111 commented on code in PR #1651:
URL:
https://github.com/apache/incubator-pegasus/pull/1651#discussion_r1365357300
##########
src/server/pegasus_server_impl.h:
##########
@@ -366,8 +366,15 @@ class pegasus_server_impl : public pegasus_read_service
void reset_usage_scenario_options(const rocksdb::ColumnFamilyOptions
&base_opts,
rocksdb::ColumnFamilyOptions
*target_opts);
+ void reset_allow_ingest_behind_option(const rocksdb::DBOptions
&base_db_opt,
Review Comment:
> When ordering function parameters, put all input-only parameters before
any output parameters.
From https://google.github.io/styleguide/cppguide.html#Inputs_and_Outputs
##########
src/server/pegasus_server_impl.cpp:
##########
@@ -3166,6 +3171,19 @@ void pegasus_server_impl::reset_usage_scenario_options(
target_opts->max_write_buffer_number = base_opts.max_write_buffer_number;
}
+void pegasus_server_impl::reset_allow_ingest_behind_option(
+ const rocksdb::DBOptions &base_db_opt,
+ rocksdb::DBOptions *target_db_opt,
+ const std::map<std::string, std::string> &envs)
+{
+ if (envs.empty()) {
+ // for reopen db during load balance learning
+ target_db_opt->allow_ingest_behind = base_db_opt.allow_ingest_behind;
+ } else {
+ target_db_opt->allow_ingest_behind = parse_allow_ingest_behind(envs);
+ }
Review Comment:
Is there any problem if just assign `base_db_opt` to `target_db_opt`, then
update `target_db_opt` if `envs` is not empty? I guess there maybe some other
configs are needed to load from the existing disk option file.
##########
src/server/pegasus_server_impl.h:
##########
@@ -366,8 +366,15 @@ class pegasus_server_impl : public pegasus_read_service
void reset_usage_scenario_options(const rocksdb::ColumnFamilyOptions
&base_opts,
rocksdb::ColumnFamilyOptions
*target_opts);
+ void reset_allow_ingest_behind_option(const rocksdb::DBOptions
&base_db_opt,
+ rocksdb::DBOptions *target_db_opt,
+ const std::map<std::string,
std::string> &envs);
+
void reset_rocksdb_options(const rocksdb::ColumnFamilyOptions &base_opts,
- rocksdb::ColumnFamilyOptions *target_opts);
+ rocksdb::ColumnFamilyOptions *target_opts,
Review Comment:
How about renaming base_opts to base_cf_opts, target_opts to target_cf_opts
? To distinguish *_db_opt.
--
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]
