Skip to content

Commit

Permalink
fix: Use param item formatter to avoid SetConfig to overlay (#39597)
Browse files Browse the repository at this point in the history 
Related to #39596

When updating the build param configuration, the `Formatter` could be
used to do so and completed avoid touching the `overlay` config items

---------

Signed-off-by: Congqi Xia <[email protected]>
  • Loading branch information
@congqixia
congqixia authored Jan 27, 2025
1 parent f7b7bf6 commit b3791a6
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 45 deletions.
40 changes: 24 additions & 16 deletions internal/util/indexparamcheck/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -128,11 +128,12 @@ func Test_CheckAutoIndex(t *testing.T) {
mgr.SetConfig("autoIndex.params.build", `{"M": 30,"efConstruction": 360,"index_type": "HNSW"}`)
p := &paramtable.AutoIndexConfig{
IndexParams: paramtable.ParamItem{
Key: "autoIndex.params.build",
Key: "autoIndex.params.build",
Formatter: paramtable.GetBuildParamFormatter(paramtable.FloatVectorDefaultMetricType, "autoIndex.params.build"),
},
}
p.IndexParams.Init(mgr)
p.SetDefaultMetricTypeHelper(p.IndexParams.Key, p.IndexParams.GetAsJSONMap(), schemapb.DataType_FloatVector, mgr)

assert.NotPanics(t, func() {
CheckAutoIndexHelper(p.IndexParams.Key, p.IndexParams.GetAsJSONMap(), schemapb.DataType_FloatVector)
})
Expand All @@ -146,11 +147,12 @@ func Test_CheckAutoIndex(t *testing.T) {
mgr.SetConfig("autoIndex.params.binary.build", `{"nlist": 1024, "index_type": "BIN_IVF_FLAT"}`)
p := &paramtable.AutoIndexConfig{
BinaryIndexParams: paramtable.ParamItem{
Key: "autoIndex.params.binary.build",
Key: "autoIndex.params.binary.build",
Formatter: paramtable.GetBuildParamFormatter(paramtable.BinaryVectorDefaultMetricType, "autoIndex.params.binary.build"),
},
}
p.BinaryIndexParams.Init(mgr)
p.SetDefaultMetricTypeHelper(p.BinaryIndexParams.Key, p.BinaryIndexParams.GetAsJSONMap(), schemapb.DataType_BinaryVector, mgr)

assert.NotPanics(t, func() {
CheckAutoIndexHelper(p.BinaryIndexParams.Key, p.BinaryIndexParams.GetAsJSONMap(), schemapb.DataType_BinaryVector)
})
Expand All @@ -164,11 +166,12 @@ func Test_CheckAutoIndex(t *testing.T) {
mgr.SetConfig("autoIndex.params.sparse.build", `{"index_type": "SPARSE_INVERTED_INDEX", "metric_type": "IP"}`)
p := &paramtable.AutoIndexConfig{
SparseIndexParams: paramtable.ParamItem{
Key: "autoIndex.params.sparse.build",
Key: "autoIndex.params.sparse.build",
Formatter: paramtable.GetBuildParamFormatter(paramtable.SparseFloatVectorDefaultMetricType, "autoIndex.params.sparse.build"),
},
}
p.SparseIndexParams.Init(mgr)
p.SetDefaultMetricTypeHelper(p.SparseIndexParams.Key, p.SparseIndexParams.GetAsJSONMap(), schemapb.DataType_SparseFloatVector, mgr)

assert.NotPanics(t, func() {
CheckAutoIndexHelper(p.SparseIndexParams.Key, p.SparseIndexParams.GetAsJSONMap(), schemapb.DataType_SparseFloatVector)
})
Expand All @@ -182,11 +185,12 @@ func Test_CheckAutoIndex(t *testing.T) {
mgr.SetConfig("autoIndex.params.build", `{"nlist": 30, "index_type": "IVF_FLAT"}`)
p := &paramtable.AutoIndexConfig{
IndexParams: paramtable.ParamItem{
Key: "autoIndex.params.build",
Key: "autoIndex.params.build",
Formatter: paramtable.GetBuildParamFormatter(paramtable.FloatVectorDefaultMetricType, "autoIndex.params.build"),
},
}
p.IndexParams.Init(mgr)
p.SetDefaultMetricTypeHelper(p.IndexParams.Key, p.IndexParams.GetAsJSONMap(), schemapb.DataType_FloatVector, mgr)

assert.NotPanics(t, func() {
CheckAutoIndexHelper(p.IndexParams.Key, p.IndexParams.GetAsJSONMap(), schemapb.DataType_FloatVector)
})
Expand All @@ -200,11 +204,12 @@ func Test_CheckAutoIndex(t *testing.T) {
mgr.SetConfig("autoIndex.params.build", `{"nlist": 30, "index_type": "IVF_FLAT"}`)
p := &paramtable.AutoIndexConfig{
IndexParams: paramtable.ParamItem{
Key: "autoIndex.params.build",
Key: "autoIndex.params.build",
Formatter: paramtable.GetBuildParamFormatter(paramtable.FloatVectorDefaultMetricType, "autoIndex.params.build"),
},
}
p.IndexParams.Init(mgr)
p.SetDefaultMetricTypeHelper(p.IndexParams.Key, p.IndexParams.GetAsJSONMap(), schemapb.DataType_FloatVector, mgr)

assert.NotPanics(t, func() {
CheckAutoIndexHelper(p.IndexParams.Key, p.IndexParams.GetAsJSONMap(), schemapb.DataType_FloatVector)
})
Expand All @@ -218,11 +223,12 @@ func Test_CheckAutoIndex(t *testing.T) {
mgr.SetConfig("autoIndex.params.build", `{"index_type": "DISKANN"}`)
p := &paramtable.AutoIndexConfig{
IndexParams: paramtable.ParamItem{
Key: "autoIndex.params.build",
Key: "autoIndex.params.build",
Formatter: paramtable.GetBuildParamFormatter(paramtable.FloatVectorDefaultMetricType, "autoIndex.params.build"),
},
}
p.IndexParams.Init(mgr)
p.SetDefaultMetricTypeHelper(p.IndexParams.Key, p.IndexParams.GetAsJSONMap(), schemapb.DataType_FloatVector, mgr)

assert.NotPanics(t, func() {
CheckAutoIndexHelper(p.IndexParams.Key, p.IndexParams.GetAsJSONMap(), schemapb.DataType_FloatVector)
})
Expand All @@ -236,11 +242,12 @@ func Test_CheckAutoIndex(t *testing.T) {
mgr.SetConfig("autoIndex.params.build", `{"index_type": "BIN_FLAT"}`)
p := &paramtable.AutoIndexConfig{
IndexParams: paramtable.ParamItem{
Key: "autoIndex.params.build",
Key: "autoIndex.params.build",
Formatter: paramtable.GetBuildParamFormatter(paramtable.BinaryVectorDefaultMetricType, "autoIndex.params.binary.build"),
},
}
p.IndexParams.Init(mgr)
p.SetDefaultMetricTypeHelper(p.IndexParams.Key, p.IndexParams.GetAsJSONMap(), schemapb.DataType_BinaryVector, mgr)

assert.NotPanics(t, func() {
CheckAutoIndexHelper(p.IndexParams.Key, p.IndexParams.GetAsJSONMap(), schemapb.DataType_BinaryVector)
})
Expand All @@ -254,11 +261,12 @@ func Test_CheckAutoIndex(t *testing.T) {
mgr.SetConfig("autoIndex.params.build", `{"nlist": 30, "index_type": "BIN_IVF_FLAT"}`)
p := &paramtable.AutoIndexConfig{
IndexParams: paramtable.ParamItem{
Key: "autoIndex.params.build",
Key: "autoIndex.params.build",
Formatter: paramtable.GetBuildParamFormatter(paramtable.BinaryVectorDefaultMetricType, "autoIndex.params.binary.build"),
},
}
p.IndexParams.Init(mgr)
p.SetDefaultMetricTypeHelper(p.IndexParams.Key, p.IndexParams.GetAsJSONMap(), schemapb.DataType_BinaryVector, mgr)

assert.NotPanics(t, func() {
CheckAutoIndexHelper(p.IndexParams.Key, p.IndexParams.GetAsJSONMap(), schemapb.DataType_BinaryVector)
})
Expand Down
47 changes: 21 additions & 26 deletions pkg/util/paramtable/autoindex_param.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,12 @@
package paramtable

import (
"fmt"
"strconv"

"github.com/cockroachdb/errors"

"github.com/milvus-io/milvus-proto/go-api/v2/schemapb"
"github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/config"
"github.com/milvus-io/milvus/pkg/util/funcutil"
"github.com/milvus-io/milvus/pkg/util/metric"
"github.com/milvus-io/milvus/pkg/util/typeutil"
Expand Down Expand Up @@ -90,6 +90,7 @@ func (p *AutoIndexConfig) init(base *BaseTable) {
Key: "autoIndex.params.build",
Version: "2.2.0",
DefaultValue: `{"M": 18,"efConstruction": 240,"index_type": "HNSW", "metric_type": "COSINE"}`,
Formatter: GetBuildParamFormatter(FloatVectorDefaultMetricType, "autoIndex.params.build"),
Export: true,
}
p.IndexParams.Init(base.mgr)
Expand All @@ -98,6 +99,7 @@ func (p *AutoIndexConfig) init(base *BaseTable) {
Key: "autoIndex.params.sparse.build",
Version: "2.4.5",
DefaultValue: `{"index_type": "SPARSE_INVERTED_INDEX", "metric_type": "IP"}`,
Formatter: GetBuildParamFormatter(SparseFloatVectorDefaultMetricType, "autoIndex.params.sparse.build"),
Export: true,
}
p.SparseIndexParams.Init(base.mgr)
Expand All @@ -106,6 +108,7 @@ func (p *AutoIndexConfig) init(base *BaseTable) {
Key: "autoIndex.params.binary.build",
Version: "2.4.5",
DefaultValue: `{"nlist": 1024, "index_type": "BIN_IVF_FLAT", "metric_type": "HAMMING"}`,
Formatter: GetBuildParamFormatter(BinaryVectorDefaultMetricType, "autoIndex.params.sparse.build"),
Export: true,
}
p.BinaryIndexParams.Init(base.mgr)
Expand Down Expand Up @@ -158,8 +161,6 @@ func (p *AutoIndexConfig) init(base *BaseTable) {
}
p.AutoIndexTuningConfig.Init(base.mgr)

p.SetDefaultMetricType(base.mgr)

p.ScalarAutoIndexEnable = ParamItem{
Key: "scalarAutoIndex.enable",
Version: "2.4.0",
Expand Down Expand Up @@ -245,13 +246,6 @@ func (p *AutoIndexConfig) init(base *BaseTable) {
p.ScalarBoolIndexType.Init(base.mgr)
}

// SetDefaultMetricType The config check logic has been moved to internal package; only set defulat metric here
func (p *AutoIndexConfig) SetDefaultMetricType(mgr *config.Manager) {
p.SetDefaultMetricTypeHelper(p.IndexParams.Key, p.IndexParams.GetAsJSONMap(), schemapb.DataType_FloatVector, mgr)
p.SetDefaultMetricTypeHelper(p.BinaryIndexParams.Key, p.BinaryIndexParams.GetAsJSONMap(), schemapb.DataType_BinaryVector, mgr)
p.SetDefaultMetricTypeHelper(p.SparseIndexParams.Key, p.SparseIndexParams.GetAsJSONMap(), schemapb.DataType_SparseFloatVector, mgr)
}

func setDefaultIfNotExist(params map[string]string, key string, defaultValue string) {
_, exist := params[key]
if !exist {
Expand All @@ -278,20 +272,21 @@ func SetDefaultMetricTypeIfNotExist(dType schemapb.DataType, params map[string]s
}
}

func (p *AutoIndexConfig) SetDefaultMetricTypeHelper(key string, m map[string]string, dtype schemapb.DataType, mgr *config.Manager) {
if m == nil {
panic(fmt.Sprintf("%s invalid, should be json format", key))
}

SetDefaultMetricTypeIfNotExist(dtype, m)

p.reset(key, m, mgr)
}

func (p *AutoIndexConfig) reset(key string, m map[string]string, mgr *config.Manager) {
ret, err := funcutil.MapToJSON(m)
if err != nil {
panic(fmt.Sprintf("%s: convert to json failed, parameters invalid, error: %s", key, err.Error()))
func GetBuildParamFormatter(defaultMetricsType metric.MetricType, tag string) func(string) string {
return func(originValue string) string {
m, err := funcutil.JSONToMap(originValue)
if err != nil {
panic(errors.Wrapf(err, "failed to parse %s config value", tag))
}
_, ok := m[common.MetricTypeKey]
if ok {
return originValue
}
m[common.MetricTypeKey] = defaultMetricsType
ret, err := funcutil.MapToJSON(m)
if err != nil {
panic(errors.Wrapf(err, "failed to convert updated %s map to json", tag))
}
return ret
}
mgr.SetConfig(key, ret)
}
7 changes: 4 additions & 3 deletions pkg/util/paramtable/autoindex_param_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (

"github.com/stretchr/testify/assert"

"github.com/milvus-io/milvus-proto/go-api/v2/schemapb"
"github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/config"
)
Expand Down Expand Up @@ -135,12 +134,14 @@ func Test_autoIndexConfig_panicIfNotValid(t *testing.T) {
mgr.SetConfig("autoIndex.params.build", "not in json format")
p := &AutoIndexConfig{
IndexParams: ParamItem{
Key: "autoIndex.params.build",
Key: "autoIndex.params.build",
Formatter: GetBuildParamFormatter(FloatVectorDefaultMetricType, "autoIndex.params.build"),
},
}
p.IndexParams.Init(mgr)

assert.Panics(t, func() {
p.SetDefaultMetricTypeHelper(p.IndexParams.Key, p.IndexParams.GetAsJSONMap(), schemapb.DataType_FloatVector, mgr)
p.IndexParams.GetAsJSONMap()
})
})
}
Expand Down

0 comments on commit b3791a6

Please sign in to comment.