Skip to content

Commit

Permalink
add unify vector index config checker to replace the config checker
Browse files Browse the repository at this point in the history
Signed-off-by: xianliang.li <[email protected]>
  • Loading branch information
foxspy committed Oct 21, 2024
1 parent 346510e commit 49d96c4
Show file tree
Hide file tree
Showing 93 changed files with 1,213 additions and 782 deletions.
1 change: 1 addition & 0 deletions client/index/disk_ann.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func NewDiskANNIndex(metricType MetricType) Index {
return &diskANNIndex{
baseIndex: baseIndex{
metricType: metricType,
indexType: DISKANN,
},
}
}
2 changes: 2 additions & 0 deletions client/index/flat.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ func NewFlatIndex(metricType MetricType) Index {
return flatIndex{
baseIndex: baseIndex{
metricType: metricType,
indexType: Flat,
},
}
}
Expand All @@ -54,6 +55,7 @@ func NewBinFlatIndex(metricType MetricType) Index {
return binFlatIndex{
baseIndex: baseIndex{
metricType: metricType,
indexType: BinFlat,
},
}
}
2 changes: 2 additions & 0 deletions cmd/components/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import (
"github.com/milvus-io/milvus-proto/go-api/v2/milvuspb"
grpcproxy "github.com/milvus-io/milvus/internal/distributed/proxy"
"github.com/milvus-io/milvus/internal/util/dependency"
"github.com/milvus-io/milvus/internal/util/indexparamcheck"
"github.com/milvus-io/milvus/pkg/log"
"github.com/milvus-io/milvus/pkg/util/paramtable"
"github.com/milvus-io/milvus/pkg/util/typeutil"
Expand All @@ -50,6 +51,7 @@ func NewProxy(ctx context.Context, factory dependency.Factory) (*Proxy, error) {
}

func (n *Proxy) Prepare() error {
indexparamcheck.ValidateParamTable()
return n.svr.Prepare()
}

Expand Down
73 changes: 73 additions & 0 deletions internal/core/src/segcore/vector_index_c.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,79 @@
#include "index/IndexFactory.h"
#include "pb/index_cgo_msg.pb.h"

CStatus
ValidateIndexParams(const char* index_type,

Check warning on line 24 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L24

Added line #L24 was not covered by tests
enum CDataType data_type,
const uint8_t* serialized_index_params,
const uint64_t length) {
try {
auto index_params =
std::make_unique<milvus::proto::indexcgo::IndexParams>();
auto res =
index_params->ParseFromArray(serialized_index_params, length);
AssertInfo(res, "Unmarshall index params failed");

Check warning on line 33 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L28-L33

Added lines #L28 - L33 were not covered by tests

knowhere::Json json;

Check warning on line 35 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L35

Added line #L35 was not covered by tests

for (size_t i = 0; i < index_params->params_size(); i++) {
auto& param = index_params->params(i);
json[param.key()] = param.value();

Check warning on line 39 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L37-L39

Added lines #L37 - L39 were not covered by tests
}

milvus::DataType dataType(static_cast<milvus::DataType>(data_type));

Check warning on line 42 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L42

Added line #L42 was not covered by tests

knowhere::Status status;
std::string error_msg;
if (dataType == milvus::DataType::VECTOR_BINARY) {
status = knowhere::IndexStaticFaced<knowhere::bin1>::ConfigCheck(

Check warning on line 47 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L44-L47

Added lines #L44 - L47 were not covered by tests
index_type,
knowhere::Version::GetCurrentVersion().VersionNumber(),

Check warning on line 49 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L49

Added line #L49 was not covered by tests
json,
error_msg);
} else if (dataType == milvus::DataType::VECTOR_FLOAT) {
status = knowhere::IndexStaticFaced<knowhere::fp32>::ConfigCheck(

Check warning on line 53 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L52-L53

Added lines #L52 - L53 were not covered by tests
index_type,
knowhere::Version::GetCurrentVersion().VersionNumber(),

Check warning on line 55 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L55

Added line #L55 was not covered by tests
json,
error_msg);
} else if (dataType == milvus::DataType::VECTOR_BFLOAT16) {
status = knowhere::IndexStaticFaced<knowhere::bf16>::ConfigCheck(

Check warning on line 59 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L58-L59

Added lines #L58 - L59 were not covered by tests
index_type,
knowhere::Version::GetCurrentVersion().VersionNumber(),

Check warning on line 61 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L61

Added line #L61 was not covered by tests
json,
error_msg);
} else if (dataType == milvus::DataType::VECTOR_FLOAT16) {
status = knowhere::IndexStaticFaced<knowhere::fp16>::ConfigCheck(

Check warning on line 65 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L64-L65

Added lines #L64 - L65 were not covered by tests
index_type,
knowhere::Version::GetCurrentVersion().VersionNumber(),

Check warning on line 67 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L67

Added line #L67 was not covered by tests
json,
error_msg);
} else if (dataType == milvus::DataType::VECTOR_SPARSE_FLOAT) {
status = knowhere::IndexStaticFaced<knowhere::fp32>::ConfigCheck(

Check warning on line 71 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L70-L71

Added lines #L70 - L71 were not covered by tests
index_type,
knowhere::Version::GetCurrentVersion().VersionNumber(),

Check warning on line 73 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L73

Added line #L73 was not covered by tests
json,
error_msg);
} else {
status = knowhere::Status::invalid_args;
}
CStatus cStatus;
if (status == knowhere::Status::success) {

Check warning on line 80 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L79-L80

Added lines #L79 - L80 were not covered by tests
cStatus.error_code = milvus::Success;
cStatus.error_msg = "";
} else {
cStatus.error_code = milvus::ConfigInvalid;
cStatus.error_msg = strdup(error_msg.c_str());

Check warning on line 85 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L84-L85

Added lines #L84 - L85 were not covered by tests
}
return cStatus;
} catch (std::exception& e) {
auto cStatus = CStatus();
cStatus.error_code = milvus::UnexpectedError;
cStatus.error_msg = strdup(e.what());
return cStatus;
}

Check warning on line 93 in internal/core/src/segcore/vector_index_c.cpp

View check run for this annotation

Codecov / codecov/patch

internal/core/src/segcore/vector_index_c.cpp#L87-L93

Added lines #L87 - L93 were not covered by tests
}

int
GetIndexListSize() {
return knowhere::IndexFactory::Instance().GetIndexFeatures().size();
Expand Down
6 changes: 6 additions & 0 deletions internal/core/src/segcore/vector_index_c.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@ extern "C" {
#include <stdbool.h>
#include "common/type_c.h"

CStatus
ValidateIndexParams(const char* index_type,
enum CDataType data_type,
const uint8_t* index_params,
const uint64_t length);

int
GetIndexListSize();

Expand Down
2 changes: 1 addition & 1 deletion internal/datacoord/index_meta.go
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,11 @@ import (
"github.com/milvus-io/milvus/internal/metastore/model"
"github.com/milvus-io/milvus/internal/proto/indexpb"
"github.com/milvus-io/milvus/internal/proto/workerpb"
"github.com/milvus-io/milvus/internal/util/indexparamcheck"
"github.com/milvus-io/milvus/internal/util/vecindexmgr"
"github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/log"
"github.com/milvus-io/milvus/pkg/metrics"
"github.com/milvus-io/milvus/pkg/util/indexparamcheck"
"github.com/milvus-io/milvus/pkg/util/indexparams"
"github.com/milvus-io/milvus/pkg/util/timerecord"
"github.com/milvus-io/milvus/pkg/util/typeutil"
Expand Down
2 changes: 1 addition & 1 deletion internal/datacoord/index_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,11 @@ import (
"github.com/milvus-io/milvus/internal/metastore/model"
"github.com/milvus-io/milvus/internal/proto/datapb"
"github.com/milvus-io/milvus/internal/proto/indexpb"
"github.com/milvus-io/milvus/internal/util/indexparamcheck"
"github.com/milvus-io/milvus/internal/util/vecindexmgr"
"github.com/milvus-io/milvus/pkg/log"
"github.com/milvus-io/milvus/pkg/metrics"
"github.com/milvus-io/milvus/pkg/util/funcutil"
"github.com/milvus-io/milvus/pkg/util/indexparamcheck"
"github.com/milvus-io/milvus/pkg/util/merr"
"github.com/milvus-io/milvus/pkg/util/metautil"
"github.com/milvus-io/milvus/pkg/util/paramtable"
Expand Down
6 changes: 3 additions & 3 deletions internal/datacoord/index_service_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,9 @@ import (
"github.com/milvus-io/milvus/internal/proto/indexpb"
"github.com/milvus-io/milvus/internal/proto/workerpb"
"github.com/milvus-io/milvus/internal/storage"
"github.com/milvus-io/milvus/internal/util/indexparamcheck"
"github.com/milvus-io/milvus/internal/util/sessionutil"
"github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/util/indexparamcheck"
"github.com/milvus-io/milvus/pkg/util/merr"
)

Expand Down Expand Up @@ -620,13 +620,13 @@ func TestServer_AlterIndex(t *testing.T) {
s.stateCode.Store(commonpb.StateCode_Healthy)

t.Run("mmap_unsupported", func(t *testing.T) {
indexParams[0].Value = indexparamcheck.IndexRaftCagra
indexParams[0].Value = "GPU_CAGRA"

resp, err := s.AlterIndex(ctx, req)
assert.NoError(t, err)
assert.ErrorIs(t, merr.CheckRPCCall(resp, err), merr.ErrParameterInvalid)

indexParams[0].Value = indexparamcheck.IndexFaissIvfFlat
indexParams[0].Value = "IVF_FLAT"
})

t.Run("param_value_invalied", func(t *testing.T) {
Expand Down
13 changes: 3 additions & 10 deletions internal/proxy/task_index.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,13 +28,13 @@ import (
"github.com/milvus-io/milvus-proto/go-api/v2/schemapb"
"github.com/milvus-io/milvus/internal/proto/indexpb"
"github.com/milvus-io/milvus/internal/types"
"github.com/milvus-io/milvus/internal/util/indexparamcheck"
"github.com/milvus-io/milvus/internal/util/vecindexmgr"
"github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/log"
"github.com/milvus-io/milvus/pkg/mq/msgstream"
"github.com/milvus-io/milvus/pkg/util/commonpbutil"
"github.com/milvus-io/milvus/pkg/util/funcutil"
"github.com/milvus-io/milvus/pkg/util/indexparamcheck"
"github.com/milvus-io/milvus/pkg/util/indexparams"
"github.com/milvus-io/milvus/pkg/util/merr"
"github.com/milvus-io/milvus/pkg/util/metric"
Expand Down Expand Up @@ -475,25 +475,18 @@ func checkTrain(field *schemapb.FieldSchema, indexParams map[string]string) erro
if err := fillDimension(field, indexParams); err != nil {
return err
}
} else {
// used only for checker, should be deleted after checking
indexParams[IsSparseKey] = "true"
}

if err := checker.CheckValidDataType(field); err != nil {
if err := checker.CheckValidDataType(indexType, field); err != nil {
log.Info("create index with invalid data type", zap.Error(err), zap.String("data_type", field.GetDataType().String()))
return err
}

if err := checker.CheckTrain(indexParams); err != nil {
if err := checker.CheckTrain(field.DataType, indexParams); err != nil {
log.Info("create index with invalid parameters", zap.Error(err))
return err
}

if isSparse {
delete(indexParams, IsSparseKey)
}

return nil
}

Expand Down
2 changes: 1 addition & 1 deletion internal/proxy/task_index_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,9 +35,9 @@ import (
"github.com/milvus-io/milvus/internal/mocks"
"github.com/milvus-io/milvus/internal/proto/indexpb"
"github.com/milvus-io/milvus/internal/proto/querypb"
"github.com/milvus-io/milvus/internal/util/indexparamcheck"
"github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/util/funcutil"
"github.com/milvus-io/milvus/pkg/util/indexparamcheck"
"github.com/milvus-io/milvus/pkg/util/merr"
"github.com/milvus-io/milvus/pkg/util/paramtable"
"github.com/milvus-io/milvus/pkg/util/typeutil"
Expand Down
2 changes: 1 addition & 1 deletion internal/proxy/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import (
"github.com/milvus-io/milvus/internal/proto/querypb"
"github.com/milvus-io/milvus/internal/types"
"github.com/milvus-io/milvus/internal/util/hookutil"
"github.com/milvus-io/milvus/internal/util/indexparamcheck"
typeutil2 "github.com/milvus-io/milvus/internal/util/typeutil"
"github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/log"
Expand All @@ -48,7 +49,6 @@ import (
"github.com/milvus-io/milvus/pkg/util/commonpbutil"
"github.com/milvus-io/milvus/pkg/util/contextutil"
"github.com/milvus-io/milvus/pkg/util/crypto"
"github.com/milvus-io/milvus/pkg/util/indexparamcheck"
"github.com/milvus-io/milvus/pkg/util/merr"
"github.com/milvus-io/milvus/pkg/util/metric"
"github.com/milvus-io/milvus/pkg/util/paramtable"
Expand Down
2 changes: 1 addition & 1 deletion internal/querynodev2/segments/index_attr_cache.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,11 @@ import (

"github.com/milvus-io/milvus/internal/proto/datapb"
"github.com/milvus-io/milvus/internal/proto/querypb"
"github.com/milvus-io/milvus/internal/util/indexparamcheck"
"github.com/milvus-io/milvus/internal/util/vecindexmgr"
"github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/util/conc"
"github.com/milvus-io/milvus/pkg/util/funcutil"
"github.com/milvus-io/milvus/pkg/util/indexparamcheck"
"github.com/milvus-io/milvus/pkg/util/typeutil"
)

Expand Down
2 changes: 1 addition & 1 deletion internal/querynodev2/segments/index_attr_cache_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@ import (
"github.com/milvus-io/milvus-proto/go-api/v2/commonpb"
"github.com/milvus-io/milvus/internal/proto/datapb"
"github.com/milvus-io/milvus/internal/proto/querypb"
"github.com/milvus-io/milvus/internal/util/indexparamcheck"
"github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/util/indexparamcheck"
"github.com/milvus-io/milvus/pkg/util/paramtable"
"github.com/milvus-io/milvus/pkg/util/typeutil"
)
Expand Down
2 changes: 1 addition & 1 deletion internal/querynodev2/segments/segment.go
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ import (
"github.com/milvus-io/milvus/internal/querynodev2/segments/state"
"github.com/milvus-io/milvus/internal/storage"
"github.com/milvus-io/milvus/internal/util/cgo"
"github.com/milvus-io/milvus/internal/util/indexparamcheck"
"github.com/milvus-io/milvus/internal/util/vecindexmgr"
"github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/log"
"github.com/milvus-io/milvus/pkg/metrics"
"github.com/milvus-io/milvus/pkg/util/funcutil"
"github.com/milvus-io/milvus/pkg/util/indexparamcheck"
"github.com/milvus-io/milvus/pkg/util/indexparams"
"github.com/milvus-io/milvus/pkg/util/merr"
"github.com/milvus-io/milvus/pkg/util/metautil"
Expand Down
2 changes: 1 addition & 1 deletion internal/querynodev2/segments/segment_loader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ import (
"github.com/milvus-io/milvus/internal/proto/datapb"
"github.com/milvus-io/milvus/internal/proto/querypb"
"github.com/milvus-io/milvus/internal/storage"
"github.com/milvus-io/milvus/internal/util/indexparamcheck"
"github.com/milvus-io/milvus/internal/util/initcore"
"github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/util/contextutil"
"github.com/milvus-io/milvus/pkg/util/funcutil"
"github.com/milvus-io/milvus/pkg/util/indexparamcheck"
"github.com/milvus-io/milvus/pkg/util/merr"
"github.com/milvus-io/milvus/pkg/util/metric"
"github.com/milvus-io/milvus/pkg/util/paramtable"
Expand Down
2 changes: 1 addition & 1 deletion internal/querynodev2/segments/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,12 +29,12 @@ import (
"github.com/milvus-io/milvus/internal/querycoordv2/params"
"github.com/milvus-io/milvus/internal/querynodev2/segments/metricsutil"
"github.com/milvus-io/milvus/internal/storage"
"github.com/milvus-io/milvus/internal/util/indexparamcheck"
"github.com/milvus-io/milvus/internal/util/vecindexmgr"
"github.com/milvus-io/milvus/pkg/common"
"github.com/milvus-io/milvus/pkg/log"
"github.com/milvus-io/milvus/pkg/mq/msgstream"
"github.com/milvus-io/milvus/pkg/util/contextutil"
"github.com/milvus-io/milvus/pkg/util/indexparamcheck"
"github.com/milvus-io/milvus/pkg/util/merr"
"github.com/milvus-io/milvus/pkg/util/paramtable"
"github.com/milvus-io/milvus/pkg/util/typeutil"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ type AUTOINDEXChecker struct {
baseChecker
}

func (c *AUTOINDEXChecker) CheckTrain(params map[string]string) error {
func (c *AUTOINDEXChecker) CheckTrain(dataType schemapb.DataType, params map[string]string) error {

Check warning on line 12 in internal/util/indexparamcheck/auto_index_checker.go

View check run for this annotation

Codecov / codecov/patch

internal/util/indexparamcheck/auto_index_checker.go#L12

Added line #L12 was not covered by tests
return nil
}

func (c *AUTOINDEXChecker) CheckValidDataType(field *schemapb.FieldSchema) error {
func (c *AUTOINDEXChecker) CheckValidDataType(indexType IndexType, field *schemapb.FieldSchema) error {

Check warning on line 16 in internal/util/indexparamcheck/auto_index_checker.go

View check run for this annotation

Codecov / codecov/patch

internal/util/indexparamcheck/auto_index_checker.go#L16

Added line #L16 was not covered by tests
return nil
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,29 +19,17 @@ package indexparamcheck
import (
"fmt"
"math"
"strings"

"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/util/typeutil"
)

type baseChecker struct{}

func (c baseChecker) CheckTrain(params map[string]string) error {
// vector dimension should be checked on collection creation. this is just some basic check
isSparse := false
if val, exist := params[common.IsSparseKey]; exist {
val = strings.ToLower(val)
if val != "true" && val != "false" {
return fmt.Errorf("invalid is_sparse value: %s, must be true or false", val)
}
if val == "true" {
isSparse = true
}
}
if isSparse {
func (c baseChecker) CheckTrain(dataType schemapb.DataType, params map[string]string) error {
if typeutil.IsSparseFloatVectorType(dataType) {
if !CheckStrByValues(params, Metric, SparseMetrics) {
return fmt.Errorf("metric type not found or not supported for sparse float vectors, supported: %v", SparseMetrics)
}
Expand All @@ -55,13 +43,13 @@ func (c baseChecker) CheckTrain(params map[string]string) error {
}

// CheckValidDataType check whether the field data type is supported for the index type
func (c baseChecker) CheckValidDataType(field *schemapb.FieldSchema) error {
func (c baseChecker) CheckValidDataType(indexType IndexType, field *schemapb.FieldSchema) error {
return nil
}

func (c baseChecker) SetDefaultMetricTypeIfNotExist(m map[string]string, dType schemapb.DataType) {}
func (c baseChecker) SetDefaultMetricTypeIfNotExist(dType schemapb.DataType, m map[string]string) {}

Check warning on line 50 in internal/util/indexparamcheck/base_checker.go

View check run for this annotation

Codecov / codecov/patch

internal/util/indexparamcheck/base_checker.go#L50

Added line #L50 was not covered by tests

func (c baseChecker) StaticCheck(params map[string]string) error {
func (c baseChecker) StaticCheck(dataType schemapb.DataType, params map[string]string) error {
return errors.New("unsupported index type")
}

Expand Down
Loading

0 comments on commit 49d96c4

Please sign in to comment.