You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by GitBox <gi...@apache.org> on 2020/10/07 07:10:32 UTC

[GitHub] [incubator-doris] morningman opened a new pull request #4704: [Feature][Config] Support persistence of configuration items modified at runtime

morningman opened a new pull request #4704:
URL: https://github.com/apache/incubator-doris/pull/4704


   ## Proposed changes
   
   Support persistence of configuration items modified at runtime via HTTP API.
   
   ```
   FE:
   GET /api/_set_config?key=value&persist=true
   
   BE
   POST /api/update_config?key=value&persist=true
   ```
   
   The modified config will be saved in `fe_custom.conf` or `be_custom.conf`
   
   ## Types of changes
   
   - [x] New feature (non-breaking change which adds functionality)
   
   ## Checklist
   
   - [x] I have create an issue on (Fix #4703), and have described the bug/feature there in detail
   - [x] If this change need a document change, I have updated the document


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman merged pull request #4704: [Feature][Config] Support persistence of configuration items modified at runtime

Posted by GitBox <gi...@apache.org>.
morningman merged pull request #4704:
URL: https://github.com/apache/incubator-doris/pull/4704


   


----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] acelyc111 commented on a change in pull request #4704: [Feature][Config] Support persistence of configuration items modified at runtime

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] acelyc111 commented on a change in pull request #4704: [Feature][Config] Support persistence of configuration items modified at runtime

Posted by GitBox <gi...@apache.org>.
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #4704: [Feature][Config] Support persistence of configuration items modified at runtime

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #4704:
URL: https://github.com/apache/incubator-doris/pull/4704#discussion_r502742064



##########
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:
       This is translated by Google Translator... So, I think it is correct ^^




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #4704: [Feature][Config] Support persistence of configuration items modified at runtime

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #4704:
URL: https://github.com/apache/incubator-doris/pull/4704#discussion_r502742507



##########
File path: fe/fe-core/src/main/java/org/apache/doris/common/ConfigBase.java
##########
@@ -284,4 +308,31 @@ public synchronized static boolean checkIsMasterOnly(String key) {
 
         return anno.masterOnly();
     }
+
+    // overwrite configs to customConfFile.
+    // use synchronized to make sure only one thread modify this file
+    public synchronized static void persistConfig(Map<String, String> customConf) throws IOException {
+        File file = new File(customConfFile);
+        if (!file.exists()) {
+            file.createNewFile();
+        } else {
+            // clear the file content
+            try (PrintWriter writer = new PrintWriter(file)) {
+                writer.print("");
+            }
+        }
+
+        Properties props = new Properties();
+        props.load(new FileReader(customConfFile));
+
+        for (Map.Entry<String, String> entry : customConf.entrySet()) {
+            props.setProperty(entry.getKey(), entry.getValue());
+        }
+
+        try (FileOutputStream fos = new FileOutputStream(file)) {
+            props.store(fos, "THIS IS AN AUTO GENERATED CONFIG FILE. DO NOT EDIT IT!\n\n" +

Review comment:
       OK




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] xy720 commented on a change in pull request #4704: [Feature][Config] Support persistence of configuration items modified at runtime

Posted by GitBox <gi...@apache.org>.
xy720 commented on a change in pull request #4704:
URL: https://github.com/apache/incubator-doris/pull/4704#discussion_r502412386



##########
File path: docs/zh-CN/administrator-guide/config/be_config.md
##########
@@ -45,9 +52,15 @@ BE 的配置项有两种方式进行配置:
 
 	BE 启动后,可以通过一下命令动态设置配置项。
 
-	```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}'
+	```
 
-	**通过该方式修改的配置项将在 BE 进程重启后失效。**
+	在 0.13 版本及之前,通过该方式修改的配置项将在 BE 进程重启后失效。在 0.14 及之后版本中,可以通过以下命令持久化修改后的配置。修改后的配置项存储在 `be_custom.conf` 文件中。
+	
+	```
+	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: fe/fe-core/src/main/java/org/apache/doris/common/ConfigBase.java
##########
@@ -284,4 +308,31 @@ public synchronized static boolean checkIsMasterOnly(String key) {
 
         return anno.masterOnly();
     }
+
+    // overwrite configs to customConfFile.
+    // use synchronized to make sure only one thread modify this file
+    public synchronized static void persistConfig(Map<String, String> customConf) throws IOException {
+        File file = new File(customConfFile);
+        if (!file.exists()) {
+            file.createNewFile();
+        } else {
+            // clear the file content
+            try (PrintWriter writer = new PrintWriter(file)) {
+                writer.print("");
+            }
+        }
+
+        Properties props = new Properties();
+        props.load(new FileReader(customConfFile));
+
+        for (Map.Entry<String, String> entry : customConf.entrySet()) {
+            props.setProperty(entry.getKey(), entry.getValue());
+        }
+
+        try (FileOutputStream fos = new FileOutputStream(file)) {
+            props.store(fos, "THIS IS AN AUTO GENERATED CONFIG FILE. DO NOT EDIT IT!\n\n" +

Review comment:
       I think users can edit this file if they need to.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #4704: [Feature][Config] Support persistence of configuration items modified at runtime

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #4704:
URL: https://github.com/apache/incubator-doris/pull/4704#discussion_r502741974



##########
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:
       ok

##########
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:
       ok




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #4704: [Feature][Config] Support persistence of configuration items modified at runtime

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #4704:
URL: https://github.com/apache/incubator-doris/pull/4704#discussion_r502741884



##########
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:
       ok




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] morningman commented on a change in pull request #4704: [Feature][Config] Support persistence of configuration items modified at runtime

Posted by GitBox <gi...@apache.org>.
morningman commented on a change in pull request #4704:
URL: https://github.com/apache/incubator-doris/pull/4704#discussion_r502742088



##########
File path: docs/zh-CN/administrator-guide/config/be_config.md
##########
@@ -45,9 +52,15 @@ BE 的配置项有两种方式进行配置:
 
 	BE 启动后,可以通过一下命令动态设置配置项。
 
-	```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}'
+	```
 
-	**通过该方式修改的配置项将在 BE 进程重启后失效。**
+	在 0.13 版本及之前,通过该方式修改的配置项将在 BE 进程重启后失效。在 0.14 及之后版本中,可以通过以下命令持久化修改后的配置。修改后的配置项存储在 `be_custom.conf` 文件中。
+	
+	```
+	curl -X POST http://{be_ip}:{be_http_port}/api/update_config?{key}={value}&persis=true'

Review comment:
       OK




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org


[GitHub] [incubator-doris] xy720 commented on a change in pull request #4704: [Feature][Config] Support persistence of configuration items modified at runtime

Posted by GitBox <gi...@apache.org>.
xy720 commented on a change in pull request #4704:
URL: https://github.com/apache/incubator-doris/pull/4704#discussion_r502412386



##########
File path: docs/zh-CN/administrator-guide/config/be_config.md
##########
@@ -45,9 +52,15 @@ BE 的配置项有两种方式进行配置:
 
 	BE 启动后,可以通过一下命令动态设置配置项。
 
-	```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}'
+	```
 
-	**通过该方式修改的配置项将在 BE 进程重启后失效。**
+	在 0.13 版本及之前,通过该方式修改的配置项将在 BE 进程重启后失效。在 0.14 及之后版本中,可以通过以下命令持久化修改后的配置。修改后的配置项存储在 `be_custom.conf` 文件中。
+	
+	```
+	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: fe/fe-core/src/main/java/org/apache/doris/common/ConfigBase.java
##########
@@ -284,4 +308,31 @@ public synchronized static boolean checkIsMasterOnly(String key) {
 
         return anno.masterOnly();
     }
+
+    // overwrite configs to customConfFile.
+    // use synchronized to make sure only one thread modify this file
+    public synchronized static void persistConfig(Map<String, String> customConf) throws IOException {
+        File file = new File(customConfFile);
+        if (!file.exists()) {
+            file.createNewFile();
+        } else {
+            // clear the file content
+            try (PrintWriter writer = new PrintWriter(file)) {
+                writer.print("");
+            }
+        }
+
+        Properties props = new Properties();
+        props.load(new FileReader(customConfFile));
+
+        for (Map.Entry<String, String> entry : customConf.entrySet()) {
+            props.setProperty(entry.getKey(), entry.getValue());
+        }
+
+        try (FileOutputStream fos = new FileOutputStream(file)) {
+            props.store(fos, "THIS IS AN AUTO GENERATED CONFIG FILE. DO NOT EDIT IT!\n\n" +

Review comment:
       I think users can edit this file if they need to.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org