[Enhancement] don't print error if the configvar name appears to be a…

…n ENV var name (#50247) suppress errors such as Ignored unknown config: JAVA_OPTS Signed-off-by: Kevin Xiaohua Cai <[email protected]>
StarRocks · Aug 26, 2024 · 738a2ac · 738a2ac
1 parent 55516f0
commit 738a2ac
Show file tree

Hide file tree

Showing 2 changed files with 52 additions and 2 deletions.
diff --git a/be/src/common/configbase.cpp b/be/src/common/configbase.cpp
@@ -153,7 +153,12 @@ inline bool parse_key_value_pairs(std::istream& input) {
 
         auto op_field = Field::get(kv.first);
         if (!op_field.has_value()) {
-            std::cerr << fmt::format("Ignored unknown config: {}\n", kv.first);
+            // A valid env var name should be: [A-Z_][A-Z0-9_]*
+            static const std::string ENV_CHARSET("ABCDEFGHIJKLMNOPQRSTUVWXYZ01234567890_");
+            if (kv.first.find_first_not_of(ENV_CHARSET) != std::string::npos) {
+                // only report error when the var name appears not to be valid, not strict here.
+                std::cerr << fmt::format("Ignored unknown config: {}\n", kv.first);
+            }
             continue;
         }
         auto field = op_field.value();

diff --git a/be/test/common/config_test.cpp b/be/test/common/config_test.cpp
@@ -22,7 +22,9 @@
 #include <gmock/gmock.h> // EXPECT_THAT, ElementsAre
 #include <gtest/gtest.h>
 
+#include <iostream>
 #include <sstream>
+#include <streambuf>
 #include <thread>
 
 #include "common/status.h"
@@ -33,6 +35,20 @@ using namespace config;
 
 using namespace ::testing;
 
+namespace {
+
+class ostream_redirect {
+public:
+    ostream_redirect(std::ostream& os, std::streambuf* buf) : _os(os), _buf(os.rdbuf(buf)) {}
+    ~ostream_redirect() { _os.rdbuf(_buf); }
+
+private:
+    std::ostream& _os;
+    std::streambuf* _buf;
+};
+
+} // namespace
+
 class ConfigTest : public testing::Test {
     void SetUp() override { config::TEST_clear_configs(); }
 };
@@ -56,7 +72,6 @@ TEST_F(ConfigTest, test_init) {
     CONF_Int32s(cfg_int32s, "10,20,30");
     CONF_Int64s(cfg_int64s, "100,200,300");
     CONF_Strings(cfg_strings, "s1,s2,s3");
-    CONF_String(cfg_string_env, "prefix/${ConfigTestEnv1}/suffix");
     CONF_Bool(cfg_bool_env, "false");
     CONF_String_enum(cfg_string_enum, "true", "true,false");
     // Invalid config file name
@@ -98,6 +113,36 @@ TEST_F(ConfigTest, test_init) {
         EXPECT_FALSE(config::init(ss));
     }
 
+    // ignore env var config
+    {
+        std::stringstream ss;
+        ss << R"DEL(
+           JAVA_OPTS = -Xmx4g
+           )DEL";
+        std::stringstream stringbuf;
+        ostream_redirect cerrbuf(std::cerr, stringbuf.rdbuf());
+        EXPECT_TRUE(config::init(ss));
+        std::string err_text = stringbuf.str();
+        // no error message prompted
+        EXPECT_TRUE(err_text.empty()) << "ErrorText: " << err_text;
+    }
+
+    // contains the eror message about the unknown configvar `java_opts`
+    {
+        std::stringstream ss;
+        ss << R"DEL(
+           java_opts = -Xmx4g
+           )DEL";
+        std::stringstream stringbuf;
+        ostream_redirect cerrbuf(std::cerr, stringbuf.rdbuf());
+        EXPECT_TRUE(config::init(ss));
+        std::string err_text = stringbuf.str();
+        // error prompted for Unknown config `java_opts`
+        EXPECT_NE(std::string::npos, err_text.find("java_opts"));
+    }
+
+    // Move the definition here so that it won't fail other tests due to non-existence of ${ConfigTestEnv1}
+    CONF_String(cfg_string_env, "prefix/${ConfigTestEnv1}/suffix");
     // Valid input
     {
         std::stringstream ss;