diff --git a/configs/milvus.yaml b/configs/milvus.yaml index 06b42ecfeb..c8a3e448cb 100644 --- a/configs/milvus.yaml +++ b/configs/milvus.yaml @@ -936,6 +936,7 @@ common: enabledOptimizeExpr: true # Indicates whether to enable optimize expr enabledJSONKeyStats: false # Indicates sealedsegment whether to enable JSON key stats enabledGrowingSegmentJSONKeyStats: false # Indicates growingsegment whether to enable JSON key stats + enableConfigParamTypeCheck: true # Indicates whether to enable config param type check # QuotaConfig, configurations of Milvus quota and limits. # By default, we enable: diff --git a/internal/core/src/common/Common.cpp b/internal/core/src/common/Common.cpp index 0dc7f8f581..83604d5fc8 100644 --- a/internal/core/src/common/Common.cpp +++ b/internal/core/src/common/Common.cpp @@ -32,6 +32,7 @@ bool OPTIMIZE_EXPR_ENABLED = DEFAULT_OPTIMIZE_EXPR_ENABLED; int64_t JSON_KEY_STATS_COMMIT_INTERVAL = DEFAULT_JSON_KEY_STATS_COMMIT_INTERVAL; bool GROWING_JSON_KEY_STATS_ENABLED = DEFAULT_GROWING_JSON_KEY_STATS_ENABLED; +bool CONFIG_PARAM_TYPE_CHECK_ENABLED = DEFAULT_CONFIG_PARAM_TYPE_CHECK_ENABLED; void SetIndexSliceSize(const int64_t size) { @@ -91,4 +92,10 @@ SetDefaultGrowingJSONKeyStatsEnable(bool val) { GROWING_JSON_KEY_STATS_ENABLED); } +void +SetDefaultConfigParamTypeCheck(bool val) { + CONFIG_PARAM_TYPE_CHECK_ENABLED = val; + LOG_INFO("set default config param type check enabled: {}", + CONFIG_PARAM_TYPE_CHECK_ENABLED); +} } // namespace milvus diff --git a/internal/core/src/common/Common.h b/internal/core/src/common/Common.h index 91dcbb66a4..978d93ff5e 100644 --- a/internal/core/src/common/Common.h +++ b/internal/core/src/common/Common.h @@ -32,6 +32,8 @@ extern int64_t EXEC_EVAL_EXPR_BATCH_SIZE; extern int64_t JSON_KEY_STATS_COMMIT_INTERVAL; extern bool OPTIMIZE_EXPR_ENABLED; extern bool GROWING_JSON_KEY_STATS_ENABLED; +extern bool CONFIG_PARAM_TYPE_CHECK_ENABLED; + void SetIndexSliceSize(const int64_t size); @@ -59,6 +61,9 @@ SetDefaultJSONKeyStatsCommitInterval(int64_t val); void SetDefaultGrowingJSONKeyStatsEnable(bool val); +void +SetDefaultConfigParamTypeCheck(bool val); + struct BufferView { struct Element { const char* data_; diff --git a/internal/core/src/common/Consts.h b/internal/core/src/common/Consts.h index 4861eadf9a..74d23accc5 100644 --- a/internal/core/src/common/Consts.h +++ b/internal/core/src/common/Consts.h @@ -86,6 +86,7 @@ const int64_t DEFAULT_CONVERT_OR_TO_IN_NUMERIC_LIMIT = 150; const int64_t DEFAULT_JSON_INDEX_MEMORY_BUDGET = 16777216; // bytes, 16MB const bool DEFAULT_GROWING_JSON_KEY_STATS_ENABLED = false; const int64_t DEFAULT_JSON_KEY_STATS_COMMIT_INTERVAL = 200; +const bool DEFAULT_CONFIG_PARAM_TYPE_CHECK_ENABLED = true; // index config related const std::string SEGMENT_INSERT_FILES_KEY = "segment_insert_files"; diff --git a/internal/core/src/common/init_c.cpp b/internal/core/src/common/init_c.cpp index 9a77f1889e..7d22071c2b 100644 --- a/internal/core/src/common/init_c.cpp +++ b/internal/core/src/common/init_c.cpp @@ -25,7 +25,8 @@ #include "common/Tracer.h" #include "log/Log.h" -std::once_flag flag1, flag2, flag3, flag4, flag5, flag6, flag7, flag8, flag9; +std::once_flag flag1, flag2, flag3, flag4, flag5, flag6, flag7, flag8, flag9, + flag10; std::once_flag traceFlag; void @@ -102,6 +103,14 @@ InitDefaultGrowingJSONKeyStatsEnable(bool val) { val); } +void +InitDefaultConfigParamTypeCheck(bool val) { + std::call_once( + flag10, + [](bool val) { milvus::SetDefaultConfigParamTypeCheck(val); }, + val); +} + void InitTrace(CTraceConfig* config) { auto traceConfig = milvus::tracer::TraceConfig{config->exporter, diff --git a/internal/core/src/common/init_c.h b/internal/core/src/common/init_c.h index b51faf1494..78864a9a2a 100644 --- a/internal/core/src/common/init_c.h +++ b/internal/core/src/common/init_c.h @@ -57,6 +57,9 @@ InitDefaultJSONKeyStatsCommitInterval(int64_t val); void InitDefaultGrowingJSONKeyStatsEnable(bool val); +void +InitDefaultConfigParamTypeCheck(bool val); + #ifdef __cplusplus }; #endif diff --git a/internal/core/src/index/Utils.h b/internal/core/src/index/Utils.h index cbb8783a6d..cb16000ff4 100644 --- a/internal/core/src/index/Utils.h +++ b/internal/core/src/index/Utils.h @@ -28,6 +28,7 @@ #include #include +#include "common/Common.h" #include "common/Types.h" #include "common/FieldData.h" #include "common/QueryInfo.h" @@ -35,6 +36,7 @@ #include "index/IndexInfo.h" #include "storage/Types.h" #include "storage/DataCodec.h" +#include "log/Log.h" namespace milvus::index { @@ -83,26 +85,39 @@ void inline CheckParameter(Config& conf, template inline std::optional GetValueFromConfig(const Config& cfg, const std::string& key) { - if (cfg.contains(key)) { - if (cfg.at(key).is_null()) { + if (!cfg.contains(key)) { + return std::nullopt; + } + + const auto& value = cfg.at(key); + if (value.is_null()) { + return std::nullopt; + } + + try { + if constexpr (std::is_same_v) { + if (value.is_boolean()) { + return value.get(); + } + // compatibility for boolean string + return boost::algorithm::to_lower_copy(value.get()) == + "true"; + } + return value.get(); + } catch (const nlohmann::json::type_error& e) { + if (!CONFIG_PARAM_TYPE_CHECK_ENABLED) { + LOG_WARN("config type mismatch for key {}: {}", key, e.what()); return std::nullopt; } - try { - // compatibility for boolean string - if constexpr (std::is_same_v) { - if (cfg.at(key).is_boolean()) { - return cfg.at(key).get(); - } - return boost::algorithm::to_lower_copy( - cfg.at(key).get()) == "true"; - } - return cfg.at(key).get(); - } catch (std::exception& e) { - PanicInfo(ErrorCode::UnexpectedError, - "get value from config for key {} failed, error: {}", - key, - e.what()); - } + PanicInfo(ErrorCode::UnexpectedError, + "config type error for key {}: {}", + key, + e.what()); + } catch (const std::exception& e) { + PanicInfo(ErrorCode::UnexpectedError, + "Unexpected error for key {}: {}", + key, + e.what()); } return std::nullopt; } diff --git a/internal/core/unittest/test_c_api.cpp b/internal/core/unittest/test_c_api.cpp index 6ac990613f..5d0b8887d3 100644 --- a/internal/core/unittest/test_c_api.cpp +++ b/internal/core/unittest/test_c_api.cpp @@ -4617,7 +4617,7 @@ TEST(CApiTest, TestGetValueFromConfig) { GetValueFromConfig(cfg, "d"); } catch (const std::exception& e) { std::cout << e.what() << std::endl; - ASSERT_EQ(std::string(e.what()).find("get value from config for key") != + ASSERT_EQ(std::string(e.what()).find("config type error for key") != std::string::npos, true); } @@ -4625,3 +4625,26 @@ TEST(CApiTest, TestGetValueFromConfig) { auto e_value = GetValueFromConfig(cfg, "e"); ASSERT_FALSE(e_value.has_value()); } + +TEST(CApiTest, TestGetValueFromConfig_Without_Type_Check) { + nlohmann::json cfg = nlohmann::json::parse( + R"({"a" : 100, "b" : true, "c" : "true", "d" : 1.234, "e" : "1.234", "f" : null})"); + SetDefaultConfigParamTypeCheck(false); + auto a_value = GetValueFromConfig(cfg, "a"); + ASSERT_EQ(a_value.value(), 100); + std::cout << "a_value: " << a_value.value() << std::endl; + + auto b_value = GetValueFromConfig(cfg, "b"); + ASSERT_TRUE(b_value.value()); + std::cout << "b_value: " << b_value.value() << std::endl; + auto c_value = GetValueFromConfig(cfg, "c"); + ASSERT_TRUE(c_value.value()); + std::cout << "c_value: " << c_value.value() << std::endl; + auto d_value = GetValueFromConfig(cfg, "d"); + ASSERT_NEAR(d_value.value(), 1.234, 0.001); + std::cout << "d_value: " << d_value.value() << std::endl; + auto e_value = GetValueFromConfig(cfg, "e"); + ASSERT_FALSE(e_value.has_value()); + auto f_value = GetValueFromConfig(cfg, "f"); + ASSERT_FALSE(f_value.has_value()); +} diff --git a/internal/querynodev2/server.go b/internal/querynodev2/server.go index 2281719eae..9fcc9409b7 100644 --- a/internal/querynodev2/server.go +++ b/internal/querynodev2/server.go @@ -259,6 +259,9 @@ func (node *QueryNode) InitSegcore() error { cGpuMemoryPoolMaxSize := C.uint32_t(paramtable.Get().GpuConfig.MaxSize.GetAsUint32()) C.SegcoreSetKnowhereGpuMemoryPoolSize(cGpuMemoryPoolInitSize, cGpuMemoryPoolMaxSize) + cEnableConfigParamTypeCheck := C.bool(paramtable.Get().CommonCfg.EnableConfigParamTypeCheck.GetAsBool()) + C.InitDefaultConfigParamTypeCheck(cEnableConfigParamTypeCheck) + localDataRootPath := filepath.Join(paramtable.Get().LocalStorageCfg.Path.GetValue(), typeutil.QueryNodeRole) initcore.InitLocalChunkManager(localDataRootPath) diff --git a/pkg/util/paramtable/component_param.go b/pkg/util/paramtable/component_param.go index 6d1755ab38..d9be8a266c 100644 --- a/pkg/util/paramtable/component_param.go +++ b/pkg/util/paramtable/component_param.go @@ -302,6 +302,8 @@ type commonConfig struct { EnabledOptimizeExpr ParamItem `refreshable:"true"` EnabledJSONKeyStats ParamItem `refreshable:"true"` EnabledGrowingSegmentJSONKeyStats ParamItem `refreshable:"true"` + + EnableConfigParamTypeCheck ParamItem `refreshable:"true"` } func (p *commonConfig) init(base *BaseTable) { @@ -1030,6 +1032,15 @@ This helps Milvus-CDC synchronize incremental data`, Export: true, } p.EnabledGrowingSegmentJSONKeyStats.Init(base.mgr) + + p.EnableConfigParamTypeCheck = ParamItem{ + Key: "common.enableConfigParamTypeCheck", + Version: "2.5.5", + DefaultValue: "true", + Doc: "Indicates whether to enable config param type check", + Export: true, + } + p.EnableConfigParamTypeCheck.Init(base.mgr) } type gpuConfig struct {