[Feature][Config] Support persistence of configuration items modified at runtime

be/src/common/config.h

@@ -22,6 +22,9 @@
 
 namespace doris {
 namespace config {
+    // Dir of custom config file
+    CONF_String(custom_config_dir, "${DORIS_HOME}/conf");
+
     // cluster id
     CONF_Int32(cluster_id, "-1");
     // port on which ImpalaInternalService is exported

be/src/common/configbase.cpp

@@ -30,14 +30,15 @@
 
 #include "common/status.h"
 #include "gutil/strings/substitute.h"
+#include "util/filesystem_util.h"
 
 namespace doris {
 namespace config {
 
 std::map<std::string, Register::Field>* Register::_s_field_map = nullptr;
 std::map<std::string, std::string>* full_conf_map = nullptr;
 
-Properties props;
+std::mutex custom_conf_lock;
 
 // trim string
 std::string& trim(std::string& s) {
@@ -171,7 +172,7 @@ bool strtox(const std::string& valstr, std::string& retval) {
 }
 
 // load conf file
-bool Properties::load(const char* filename) {
+bool Properties::load(const char* filename, bool must_exist) {
     // if filename is null, use the empty props
     if (filename == nullptr) {
         return true;
@@ -180,8 +181,11 @@ bool Properties::load(const char* filename) {
     // open the conf file
     std::ifstream input(filename);
     if (!input.is_open()) {
-        std::cerr << "config::load() failed to open the file:" << filename << std::endl;
-        return false;
+        if (must_exist) {
+            std::cerr << "config::load() failed to open the file:" << filename << std::endl;
+            return false;
+        }
+        return true;
     }
 
     // load properties
@@ -217,16 +221,54 @@ bool Properties::load(const char* filename) {
 }
 
 template <typename T>
-bool Properties::get(const char* key, const char* defstr, T& retval) const {
+bool Properties::get_or_default(const char* key, const char* defstr, T& retval) const {
     const auto& it = file_conf_map.find(std::string(key));
-    std::string valstr = it != file_conf_map.end() ? it->second : std::string(defstr);
+    std::string valstr;
+    if (it == file_conf_map.end()) {
+        if (defstr == nullptr) {
+            // Not found in conf map, and no default value need to be set, just return
+            return true;
+        } else {
+            valstr = std::string(defstr);
+        }
+    } else {
+        valstr = it->second;
+    }
     trim(valstr);
     if (!replaceenv(valstr)) {
         return false;
     }
     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 be.conf\n";
+    out << "\n";
+
+    for (auto const& iter : file_conf_map) {
+        out << iter.first << " = " << iter.second << "\n";
+    }
+    out.close();
+    return true;
+}
+
 template <typename T>
 bool update(const std::string& value, T& retval) {
     std::string valstr(value);
@@ -249,24 +291,27 @@ std::ostream& operator<<(std::ostream& out, const std::vector<T>& v) {
     return out;
 }
 
-#define SET_FIELD(FIELD, TYPE, FILL_CONFMAP)                                                       \
-    if (strcmp((FIELD).type, #TYPE) == 0) {                                                        \
-        if (!props.get((FIELD).name, (FIELD).defval, *reinterpret_cast<TYPE*>((FIELD).storage))) { \
-            std::cerr << "config field error: " << (FIELD).name << std::endl;                      \
-            return false;                                                                          \
-        }                                                                                          \
-        if (FILL_CONFMAP) {                                                                        \
-            std::ostringstream oss;                                                                \
-            oss << (*reinterpret_cast<TYPE*>((FIELD).storage));                                    \
-            (*full_conf_map)[(FIELD).name] = oss.str();                                            \
-        }                                                                                          \
-        continue;                                                                                  \
+#define SET_FIELD(FIELD, TYPE, FILL_CONFMAP, SET_TO_DEFAULT)                                                  \
+    if (strcmp((FIELD).type, #TYPE) == 0) {                                                                   \
+        if (!props.get_or_default(                                                                            \
+                (FIELD).name, ((SET_TO_DEFAULT) ? (FIELD).defval : nullptr),                                  \
+                *reinterpret_cast<TYPE*>((FIELD).storage))) {                                                 \
+            std::cerr << "config field error: " << (FIELD).name << std::endl;                                 \
+            return false;                                                                                     \
+        }                                                                                                     \
+        if (FILL_CONFMAP) {                                                                                   \
+            std::ostringstream oss;                                                                           \
+            oss << (*reinterpret_cast<TYPE*>((FIELD).storage));                                               \
+            (*full_conf_map)[(FIELD).name] = oss.str();                                                       \
+        }                                                                                                     \
+        continue;                                                                                             \
     }
 
 // init conf fields
-bool init(const char* filename, bool fillconfmap) {
+bool init(const char* conf_file, bool fillconfmap, bool must_exist, bool set_to_default) {
+    Properties props;
     // load properties file
-    if (!props.load(filename)) {
+    if (!props.load(conf_file, must_exist)) {
         return false;
     }
     // fill full_conf_map ?
@@ -276,24 +321,24 @@ bool init(const char* filename, bool fillconfmap) {
 
     // set conf fields
     for (const auto& it : *Register::_s_field_map) {
-        SET_FIELD(it.second, bool, fillconfmap);
-        SET_FIELD(it.second, int16_t, fillconfmap);
-        SET_FIELD(it.second, int32_t, fillconfmap);
-        SET_FIELD(it.second, int64_t, fillconfmap);
-        SET_FIELD(it.second, double, fillconfmap);
-        SET_FIELD(it.second, std::string, fillconfmap);
-        SET_FIELD(it.second, std::vector<bool>, fillconfmap);
-        SET_FIELD(it.second, std::vector<int16_t>, fillconfmap);
-        SET_FIELD(it.second, std::vector<int32_t>, fillconfmap);
-        SET_FIELD(it.second, std::vector<int64_t>, fillconfmap);
-        SET_FIELD(it.second, std::vector<double>, fillconfmap);
-        SET_FIELD(it.second, std::vector<std::string>, fillconfmap);
+        SET_FIELD(it.second, bool, fillconfmap, set_to_default);
+        SET_FIELD(it.second, int16_t, fillconfmap, set_to_default);
+        SET_FIELD(it.second, int32_t, fillconfmap, set_to_default);
+        SET_FIELD(it.second, int64_t, fillconfmap, set_to_default);
+        SET_FIELD(it.second, double, fillconfmap, set_to_default);
+        SET_FIELD(it.second, std::string, fillconfmap, set_to_default);
+        SET_FIELD(it.second, std::vector<bool>, fillconfmap, set_to_default);
+        SET_FIELD(it.second, std::vector<int16_t>, fillconfmap, set_to_default);
+        SET_FIELD(it.second, std::vector<int32_t>, fillconfmap, set_to_default);
+        SET_FIELD(it.second, std::vector<int64_t>, fillconfmap, set_to_default);
+        SET_FIELD(it.second, std::vector<double>, fillconfmap, set_to_default);
+        SET_FIELD(it.second, std::vector<std::string>, fillconfmap, set_to_default);
     }
 
     return true;
 }
 
-#define UPDATE_FIELD(FIELD, VALUE, TYPE)                                             \
+#define UPDATE_FIELD(FIELD, VALUE, TYPE, PERSIST)                                    \
     if (strcmp((FIELD).type, #TYPE) == 0) {                                          \
         if (!update((VALUE), *reinterpret_cast<TYPE*>((FIELD).storage))) {           \
             return Status::InvalidArgument(                                          \
@@ -304,10 +349,37 @@ bool init(const char* filename, bool fillconfmap) {
             oss << (*reinterpret_cast<TYPE*>((FIELD).storage));                      \
             (*full_conf_map)[(FIELD).name] = oss.str();                              \
         }                                                                            \
+        if (PERSIST) {                                                               \
+            persist_config(std::string(FIELD.name), VALUE);                          \
+        }                                                                            \
         return Status::OK();                                                         \
     }
 
-Status set_config(const std::string& field, const std::string& value) {
+
+// write config to be_custom.conf
+// the caller need to make sure that the given config is valid
+bool persist_config(const std::string& field, const std::string& value) {
+    // lock to make sure only one thread can modify the be_custom.conf
+    std::lock_guard<std::mutex> l(custom_conf_lock); 
+
+    static const string conffile = string(getenv("DORIS_HOME")) + "/conf/be_custom.conf";
+    Status st = FileSystemUtil::create_file(conffile);
+    if (!st.ok()) {
+        LOG(WARNING) << "failed to create or open be_custom.conf. " << st.get_error_msg();
+        return false;
+    }
+
+    Properties tmp_props;
+    if (!tmp_props.load(conffile.c_str())) {
+        LOG(WARNING) << "failed to load " << conffile;
+        return false;
+    }
+
+    tmp_props.set(field, value);
+    return tmp_props.dump(conffile);
+}
+
+Status set_config(const std::string& field, const std::string& value, bool need_persist) {
     auto it = Register::_s_field_map->find(field);
     if (it == Register::_s_field_map->end()) {
         return Status::NotFound(strings::Substitute("'$0' is not found", field));
@@ -317,11 +389,11 @@ Status set_config(const std::string& field, const std::string& value) {
         return Status::NotSupported(strings::Substitute("'$0' is not support to modify", field));
     }
 
-    UPDATE_FIELD(it->second, value, bool);
-    UPDATE_FIELD(it->second, value, int16_t);
-    UPDATE_FIELD(it->second, value, int32_t);
-    UPDATE_FIELD(it->second, value, int64_t);
-    UPDATE_FIELD(it->second, value, double);
+    UPDATE_FIELD(it->second, value, bool, need_persist);
+    UPDATE_FIELD(it->second, value, int16_t, need_persist);
+    UPDATE_FIELD(it->second, value, int32_t, need_persist);
+    UPDATE_FIELD(it->second, value, int64_t, need_persist);
+    UPDATE_FIELD(it->second, value, double, need_persist);
 
     // The other types are not thread safe to change dynamically.
     return Status::NotSupported(strings::Substitute(

be/src/common/configbase.h

@@ -21,6 +21,7 @@
 #include <stdint.h>
 
 #include <map>
+#include <mutex>
 #include <string>
 #include <vector>
 
@@ -109,22 +110,38 @@ class Register {
 // configuration properties load from config file.
 class Properties {
 public:
-    bool load(const char* filename);
+    // load conf from file, if must_exist is true and file does not exist, return false
+    bool load(const char* filename, bool must_exist = true);
     template <typename T>
-    bool get(const char* key, const char* defstr, T& retval) const;
+
+    // Find the config value by key from `file_conf_map`.
+    // If found, set `retval` to the config value,
+    // or set `retval` to `defstr`
+    bool get_or_default(const char* key, const char* defstr, T& retval) const;
+
+    void set(const std::string& key, const std::string& val);
+
+    // dump props to conf file
+    bool dump(const std::string& conffile);
 
 private:
     std::map<std::string, std::string> file_conf_map;
 };
 
-extern Properties props;
-
 // full configurations.
 extern std::map<std::string, std::string>* full_conf_map;
 
-bool init(const char* filename, bool fillconfmap = false);
+extern std::mutex custom_conf_lock;
+
+// Init the config from `conf_file`.
+// If fillconfmap is true, the updated config will also update the `full_conf_map`.
+// If must_exist is true and `conf_file` does not exist, this function will return false.
+// If set_to_default is true, the config value will be set to default value if not found in `conf_file`.
+bool init(const char* conf_file, bool fillconfmap = false, bool must_exist = true, bool set_to_default = true);
+
+Status set_config(const std::string& field, const std::string& value, bool need_persist = false);
 
-Status set_config(const std::string& field, const std::string& value);
+bool persist_config(const std::string& field, const std::string& value);
 
 } // namespace config
 } // namespace doris

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'";
+        msg = "Now only support to set a single config once, via 'config_name=new_value', and with an optional parameter 'persist'.";
     } 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', and with an optional parameter 'persist'.";
+            } else {
+                bool need_persist = false;
+                if (req->params()->find(PERSIST_PARAM)->second.compare("true") == 0) {
+                    need_persist = true;
+                }
+                for (auto const& iter : *(req->params())) {
+                    if (iter.first.compare(PERSIST_PARAM) == 0) {
+                        continue;
+                    }
+                    s = config::set_config(iter.first, iter.second, need_persist);
+                    if (s.ok()) {
+                        LOG(INFO) << "set_config " << iter.first << "=" << iter.second << " success. persist: " << need_persist;
+                    } else {
+                        LOG(WARNING) << "set_config " << iter.first << "=" << iter.second << " failed";
+                        msg = strings::Substitute("set $0=$1 failed, reason: $2", iter.first, iter.second,
+                                s.to_string());
+                    }
+                }
+            }
         }
     }
 

be/src/service/doris_main.cpp

@@ -119,12 +119,22 @@ int main(int argc, char** argv) {
         exit(-1);
     }
 
+    // init config.
+    // the config in be_custom.conf will overwrite the config in be.conf
+    // Must init custom config after init config, separately.
+    // Because the path of custom config file is defined in be.conf
     string conffile = string(getenv("DORIS_HOME")) + "/conf/be.conf";
-    if (!doris::config::init(conffile.c_str(), true)) {
+    if (!doris::config::init(conffile.c_str(), true, true, true)) {
         fprintf(stderr, "error read config file. \n");
         return -1;
     }
 
+    string custom_conffile = doris::config::custom_config_dir + "/be_custom.conf";
+    if (!doris::config::init(custom_conffile.c_str(), true, false, false)) {
+        fprintf(stderr, "error read custom config file. \n");
+        return -1;
+    }
+
 #if !defined(ADDRESS_SANITIZER) && !defined(LEAK_SANITIZER) && !defined(THREAD_SANITIZER)
     // Aggressive decommit is required so that unused pages in the TCMalloc page heap are
     // not backed by physical pages and do not contribute towards memory consumption.