ninsmiracle commented on code in PR #1651:
URL:
https://github.com/apache/incubator-pegasus/pull/1651#discussion_r1365428982
##########
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:
When I read this slice of code,I think all of the db_option should copy to
the new db after we reopen db. Because in my opinion,we only reopen db when
replicas learn. And I think learnee's rocksDB should be total similar with
learner's rocksDB.
But I see the current logic only deal with the parameter
`allow_ingest_behind`，and I'm not very sure that would be a special design for
some feature? As we know, `learn` this a very important part for pegasus,and
some large feature base on it.
##########
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:
When I read this slice of code,I think all of the db_option should copy to
the new db after we reopen db. Because in my opinion,we only reopen db when
replicas learn. And I think learnee's rocksDB should be total similar with
learner's rocksDB.
But I see the current logic only deal with the parameter
`allow_ingest_behind`，and I'm not very sure that would be a special design for
some feature? As we know, `learn` this a very important part for pegasus,and
some large feature base on it.
--
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]
