acelyc111 commented on a change in pull request #4704: URL: https://github.com/apache/incubator-doris/pull/4704#discussion_r502145777
########## File path: be/src/common/configbase.cpp ########## @@ -227,6 +231,34 @@ bool Properties::get(const char* key, const char* defstr, T& retval) const { return strtox(valstr, retval); } +void Properties::set(const std::string& key, const std::string& val) { + file_conf_map.emplace(key, val); +} + +bool Properties::dump(const std::string& conffile) { + std::vector<std::string> files = { conffile }; + Status st = FileSystemUtil::remove_paths(files); + if (!st.ok()) { + return false; + } + st = FileSystemUtil::create_file(conffile); + if (!st.ok()) { + return false; + } + + std::ofstream out(conffile); + out << "# THIS IS AN AUTO GENERATED CONFIG FILE.\n"; + out << "# You can modify this file manually, and the configurations in this file\n"; + out << "# will overwrite the configurations in fe.conf\n"; Review comment: ```suggestion out << "# will overwrite the configurations in be.conf\n"; ``` ########## File path: docs/en/administrator-guide/config/be_config.md ########## @@ -30,51 +30,70 @@ under the License. This document mainly introduces the relevant configuration items of BE. +The BE configuration file `be.conf` is usually stored in the `conf/` directory of the BE deployment path. In version 0.14, another configuration file `be_custom.conf` will be introduced. The configuration file is used to record the configuration items that are dynamically configured and persisted by the user during operation. + +After the BE process is started, it will read the configuration items in `be.conf` first, and then read the configuration items in `be_custom.conf`. The configuration items in `be_custom.conf` will overwrite the same configuration items in `be.conf`. + ## View configuration items -(TODO) + +Users can view the current configuration items by visiting BE's web page: + +`http://be_host:be_webserver_port/varz` ## Set configuration items There are two ways to configure BE configuration items: 1. Static configuration - Add and set configuration items in the `conf/be.conf` file. The configuration items in `be.conf` will be read when BE starts. Configuration items not in `be.conf` will use default values. + Add and set configuration items in the `conf/be.conf` file. The configuration items in `be.conf` will be read when BE starts. Configuration items not in `be.conf` will use default values. 2. Dynamic configuration - After BE starts, the configuration items can be dynamically set with the following commands. + After BE starts, the configuration items can be dynamically set with the following commands. + + ``` + curl -X POST http://{be_ip}:{be_http_port}/api/update_config?{key}={value}' + ``` - ```curl -X POST http://{be_ip}:{be_http_port}/api/update_config?{key}={value}'``` + In version 0.13 and before, the configuration items modified in this way will become invalid after the BE process restarts. In 0.14 and later versions, the modified configuration can be persisted through the following command. The modified configuration items are stored in the `be_custom.conf` file. - **Configuration items modified in this way will become invalid after the BE process restarts. ** + ``` + curl -X POST http://{be_ip}:{be_http_port}/api/update_config?{key}={value}&persis=true' Review comment: ```suggestion curl -X POST http://{be_ip}:{be_http_port}/api/update_config?{key}={value}&persist=true' ``` ########## File path: docs/en/administrator-guide/config/be_config.md ########## @@ -30,51 +30,70 @@ under the License. This document mainly introduces the relevant configuration items of BE. +The BE configuration file `be.conf` is usually stored in the `conf/` directory of the BE deployment path. In version 0.14, another configuration file `be_custom.conf` will be introduced. The configuration file is used to record the configuration items that are dynamically configured and persisted by the user during operation. Review comment: ... is used for recording ... ? ########## File path: be/src/http/action/update_config_action.cpp ########## @@ -38,25 +38,53 @@ namespace doris { const static std::string HEADER_JSON = "application/json"; +const static std::string PERSIST_PARAM = "persist"; + void UpdateConfigAction::handle(HttpRequest* req) { LOG(INFO) << req->debug_string(); Status s; std::string msg; - if (req->params()->size() != 1) { + // We only support set one config at a time, and along with a optional param "persist". + // So the number of query params should at most be 2. + if (req->params()->size() > 2 || req->params()->size() < 1) { s = Status::InvalidArgument(""); msg = "Now only support to set a single config once, via 'config_name=new_value'"; Review comment: I think you should also describe the optional parameter `persist` in error message. ########## File path: be/src/http/action/update_config_action.cpp ########## @@ -38,25 +38,53 @@ namespace doris { const static std::string HEADER_JSON = "application/json"; +const static std::string PERSIST_PARAM = "persist"; + void UpdateConfigAction::handle(HttpRequest* req) { LOG(INFO) << req->debug_string(); Status s; std::string msg; - if (req->params()->size() != 1) { + // We only support set one config at a time, and along with a optional param "persist". + // So the number of query params should at most be 2. + if (req->params()->size() > 2 || req->params()->size() < 1) { s = Status::InvalidArgument(""); msg = "Now only support to set a single config once, via 'config_name=new_value'"; } else { - DCHECK(req->params()->size() == 1); - const std::string& config = req->params()->begin()->first; - const std::string& new_value = req->params()->begin()->second; - s = config::set_config(config, new_value); - if (s.ok()) { - LOG(INFO) << "set_config " << config << "=" << new_value << " success"; - } else { - LOG(WARNING) << "set_config " << config << "=" << new_value << " failed"; - msg = strings::Substitute("set $0=$1 failed, reason: $2", config, new_value, - s.to_string()); + if (req->params()->size() == 1) { + const std::string& config = req->params()->begin()->first; + const std::string& new_value = req->params()->begin()->second; + s = config::set_config(config, new_value, false); + if (s.ok()) { + LOG(INFO) << "set_config " << config << "=" << new_value << " success"; + } else { + LOG(WARNING) << "set_config " << config << "=" << new_value << " failed"; + msg = strings::Substitute("set $0=$1 failed, reason: $2", config, new_value, + s.to_string()); + } + } else if (req->params()->size() == 2) { + if (req->params()->find(PERSIST_PARAM) == req->params()->end()) { + s = Status::InvalidArgument(""); + msg = "Now only support to set a single config once, via 'config_name=new_value'"; Review comment: Same ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@doris.apache.org For additional commands, e-mail: commits-h...@doris.apache.org