Skip to content

Commit 30411d6

Browse files
authored
fix: [2.5] deny to set the mmap param for the alter index api (#39520)
- issue: #39517 - pr: #39518 Signed-off-by: SimFG <bang.fu@zilliz.com>
1 parent 4602e97 commit 30411d6

File tree

5 files changed

+63
-11
lines changed

5 files changed

+63
-11
lines changed

internal/datacoord/index_service.go

Lines changed: 27 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ import (
2828
"github.com/milvus-io/milvus/internal/metastore/model"
2929
"github.com/milvus-io/milvus/internal/util/indexparamcheck"
3030
"github.com/milvus-io/milvus/internal/util/vecindexmgr"
31+
pkgcommon "github.com/milvus-io/milvus/pkg/common"
3132
"github.com/milvus-io/milvus/pkg/log"
3233
"github.com/milvus-io/milvus/pkg/metrics"
3334
"github.com/milvus-io/milvus/pkg/proto/datapb"
@@ -385,8 +386,33 @@ func (s *Server) AlterIndex(ctx context.Context, req *indexpb.AlterIndexRequest)
385386
return merr.Status(merr.WrapErrParameterInvalidMsg("cannot provide both DeleteKeys and ExtraParams")), nil
386387
}
387388

389+
collInfo, err := s.handler.GetCollection(ctx, req.GetCollectionID())
390+
if err != nil {
391+
log.Warn("failed to get collection", zap.Error(err))
392+
return merr.Status(err), nil
393+
}
394+
schemaHelper, err := typeutil.CreateSchemaHelper(collInfo.Schema)
395+
if err != nil {
396+
log.Warn("failed to create schema helper", zap.Error(err))
397+
return merr.Status(err), nil
398+
}
399+
400+
reqIndexParamMap := funcutil.KeyValuePair2Map(req.GetParams())
401+
388402
for _, index := range indexes {
389403
if len(req.GetParams()) > 0 {
404+
fieldSchema, err := schemaHelper.GetFieldFromID(index.FieldID)
405+
if err != nil {
406+
log.Warn("failed to get field schema", zap.Error(err))
407+
return merr.Status(err), nil
408+
}
409+
isVecIndex := typeutil.IsVectorType(fieldSchema.DataType)
410+
err = pkgcommon.ValidateAutoIndexMmapConfig(Params.AutoIndexConfig.Enable.GetAsBool(), isVecIndex, reqIndexParamMap)
411+
if err != nil {
412+
log.Warn("failed to validate auto index mmap config", zap.Error(err))
413+
return merr.Status(err), nil
414+
}
415+
390416
// update user index params
391417
newUserIndexParams := UpdateParams(index, index.UserIndexParams, req.GetParams())
392418
log.Info("alter index user index params",
@@ -425,7 +451,7 @@ func (s *Server) AlterIndex(ctx context.Context, req *indexpb.AlterIndexRequest)
425451
}
426452
}
427453

428-
err := s.meta.indexMeta.AlterIndex(ctx, indexes...)
454+
err = s.meta.indexMeta.AlterIndex(ctx, indexes...)
429455
if err != nil {
430456
log.Warn("failed to alter index", zap.Error(err))
431457
return merr.Status(err), nil

internal/datacoord/index_service_test.go

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -551,6 +551,23 @@ func TestServer_AlterIndex(t *testing.T) {
551551
},
552552
}
553553

554+
mockHandler := NewNMockHandler(t)
555+
556+
mockGetCollectionInfo := func() {
557+
mockHandler.EXPECT().GetCollection(mock.Anything, collID).Return(&collectionInfo{
558+
ID: collID,
559+
Schema: &schemapb.CollectionSchema{
560+
Fields: []*schemapb.FieldSchema{
561+
{
562+
FieldID: fieldID,
563+
Name: "FieldFloatVector",
564+
DataType: schemapb.DataType_FloatVector,
565+
},
566+
},
567+
},
568+
}, nil).Once()
569+
}
570+
554571
s := &Server{
555572
meta: &meta{
556573
catalog: catalog,
@@ -608,6 +625,7 @@ func TestServer_AlterIndex(t *testing.T) {
608625
},
609626
allocator: mock0Allocator,
610627
notifyIndexChan: make(chan UniqueID, 1),
628+
handler: mockHandler,
611629
}
612630

613631
t.Run("server not available", func(t *testing.T) {
@@ -620,6 +638,7 @@ func TestServer_AlterIndex(t *testing.T) {
620638
s.stateCode.Store(commonpb.StateCode_Healthy)
621639

622640
t.Run("mmap_unsupported", func(t *testing.T) {
641+
mockGetCollectionInfo()
623642
indexParams[0].Value = "GPU_CAGRA"
624643

625644
resp, err := s.AlterIndex(ctx, req)
@@ -630,6 +649,7 @@ func TestServer_AlterIndex(t *testing.T) {
630649
})
631650

632651
t.Run("param_value_invalied", func(t *testing.T) {
652+
mockGetCollectionInfo()
633653
req.Params[0].Value = "abc"
634654
resp, err := s.AlterIndex(ctx, req)
635655
assert.ErrorIs(t, merr.CheckRPCCall(resp, err), merr.ErrParameterInvalid)
@@ -638,6 +658,7 @@ func TestServer_AlterIndex(t *testing.T) {
638658
})
639659

640660
t.Run("delete_params", func(t *testing.T) {
661+
mockGetCollectionInfo()
641662
deleteReq := &indexpb.AlterIndexRequest{
642663
CollectionID: collID,
643664
IndexName: indexName,
@@ -657,6 +678,7 @@ func TestServer_AlterIndex(t *testing.T) {
657678
}
658679
})
659680
t.Run("update_and_delete_params", func(t *testing.T) {
681+
mockGetCollectionInfo()
660682
updateAndDeleteReq := &indexpb.AlterIndexRequest{
661683
CollectionID: collID,
662684
IndexName: indexName,

internal/proxy/task_index.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -583,7 +583,7 @@ func (t *alterIndexTask) SetID(uid UniqueID) {
583583
}
584584

585585
func (t *alterIndexTask) Name() string {
586-
return CreateIndexTaskName
586+
return AlterIndexTaskName
587587
}
588588

589589
func (t *alterIndexTask) Type() commonpb.MsgType {

internal/proxy/validate_util.go

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -896,13 +896,5 @@ func newValidateUtil(opts ...validateOption) *validateUtil {
896896
}
897897

898898
func ValidateAutoIndexMmapConfig(isVectorField bool, indexParams map[string]string) error {
899-
if !Params.AutoIndexConfig.Enable.GetAsBool() {
900-
return nil
901-
}
902-
903-
_, ok := indexParams[common.MmapEnabledKey]
904-
if ok && isVectorField {
905-
return fmt.Errorf("mmap index is not supported to config for the collection in auto index mode")
906-
}
907-
return nil
899+
return common.ValidateAutoIndexMmapConfig(Params.AutoIndexConfig.Enable.GetAsBool(), isVectorField, indexParams)
908900
}

pkg/common/common.go

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -430,3 +430,15 @@ func GetReplicateEndTS(kvs []*commonpb.KeyValuePair) (uint64, bool) {
430430
}
431431
return 0, false
432432
}
433+
434+
func ValidateAutoIndexMmapConfig(autoIndexConfigEnable, isVectorField bool, indexParams map[string]string) error {
435+
if !autoIndexConfigEnable {
436+
return nil
437+
}
438+
439+
_, ok := indexParams[MmapEnabledKey]
440+
if ok && isVectorField {
441+
return fmt.Errorf("mmap index is not supported to config for the collection in auto index mode")
442+
}
443+
return nil
444+
}

0 commit comments

Comments
 (0)