fix: deny to set the mmap param for the alter index api when enable auto index (#39518)

- issue: #39517

Signed-off-by: SimFG <bang.fu@zilliz.com>

internal/datacoord/index_service.go

@@ -28,6 +28,7 @@ import (
 	"github.com/milvus-io/milvus/internal/metastore/model"
 	"github.com/milvus-io/milvus/internal/util/indexparamcheck"
 	"github.com/milvus-io/milvus/internal/util/vecindexmgr"
+	pkgcommon "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/proto/datapb"
@@ -386,8 +387,33 @@ func (s *Server) AlterIndex(ctx context.Context, req *indexpb.AlterIndexRequest)
 		return merr.Status(merr.WrapErrParameterInvalidMsg("cannot provide both DeleteKeys and ExtraParams")), nil
 	}
 
+	collInfo, err := s.handler.GetCollection(ctx, req.GetCollectionID())
+	if err != nil {
+		log.Warn("failed to get collection", zap.Error(err))
+		return merr.Status(err), nil
+	}
+	schemaHelper, err := typeutil.CreateSchemaHelper(collInfo.Schema)
+	if err != nil {
+		log.Warn("failed to create schema helper", zap.Error(err))
+		return merr.Status(err), nil
+	}
+
+	reqIndexParamMap := funcutil.KeyValuePair2Map(req.GetParams())
+
 	for _, index := range indexes {
 		if len(req.GetParams()) > 0 {
+			fieldSchema, err := schemaHelper.GetFieldFromID(index.FieldID)
+			if err != nil {
+				log.Warn("failed to get field schema", zap.Error(err))
+				return merr.Status(err), nil
+			}
+			isVecIndex := typeutil.IsVectorType(fieldSchema.DataType)
+			err = pkgcommon.ValidateAutoIndexMmapConfig(Params.AutoIndexConfig.Enable.GetAsBool(), isVecIndex, reqIndexParamMap)
+			if err != nil {
+				log.Warn("failed to validate auto index mmap config", zap.Error(err))
+				return merr.Status(err), nil
+			}
+
 			// update user index params
 			newUserIndexParams := UpdateParams(index, index.UserIndexParams, req.GetParams())
 			log.Info("alter index user index params",
@@ -426,7 +452,7 @@ func (s *Server) AlterIndex(ctx context.Context, req *indexpb.AlterIndexRequest)
 		}
 	}
 
-	err := s.meta.indexMeta.AlterIndex(ctx, indexes...)
+	err = s.meta.indexMeta.AlterIndex(ctx, indexes...)
 	if err != nil {
 		log.Warn("failed to alter index", zap.Error(err))
 		return merr.Status(err), nil

internal/datacoord/index_service_test.go

@@ -551,6 +551,23 @@ func TestServer_AlterIndex(t *testing.T) {
 		},
 	}
 
+	mockHandler := NewNMockHandler(t)
+
+	mockGetCollectionInfo := func() {
+		mockHandler.EXPECT().GetCollection(mock.Anything, collID).Return(&collectionInfo{
+			ID: collID,
+			Schema: &schemapb.CollectionSchema{
+				Fields: []*schemapb.FieldSchema{
+					{
+						FieldID:  fieldID,
+						Name:     "FieldFloatVector",
+						DataType: schemapb.DataType_FloatVector,
+					},
+				},
+			},
+		}, nil).Once()
+	}
+
 	s := &Server{
 		meta: &meta{
 			catalog:   catalog,
@@ -608,6 +625,7 @@ func TestServer_AlterIndex(t *testing.T) {
 		},
 		allocator:       mock0Allocator,
 		notifyIndexChan: make(chan UniqueID, 1),
+		handler:         mockHandler,
 	}
 
 	t.Run("server not available", func(t *testing.T) {
@@ -620,6 +638,7 @@ func TestServer_AlterIndex(t *testing.T) {
 	s.stateCode.Store(commonpb.StateCode_Healthy)
 
 	t.Run("mmap_unsupported", func(t *testing.T) {
+		mockGetCollectionInfo()
 		indexParams[0].Value = "GPU_CAGRA"
 
 		resp, err := s.AlterIndex(ctx, req)
@@ -630,6 +649,7 @@ func TestServer_AlterIndex(t *testing.T) {
 	})
 
 	t.Run("param_value_invalied", func(t *testing.T) {
+		mockGetCollectionInfo()
 		req.Params[0].Value = "abc"
 		resp, err := s.AlterIndex(ctx, req)
 		assert.ErrorIs(t, merr.CheckRPCCall(resp, err), merr.ErrParameterInvalid)
@@ -638,6 +658,7 @@ func TestServer_AlterIndex(t *testing.T) {
 	})
 
 	t.Run("delete_params", func(t *testing.T) {
+		mockGetCollectionInfo()
 		deleteReq := &indexpb.AlterIndexRequest{
 			CollectionID: collID,
 			IndexName:    indexName,
@@ -657,6 +678,7 @@ func TestServer_AlterIndex(t *testing.T) {
 		}
 	})
 	t.Run("update_and_delete_params", func(t *testing.T) {
+		mockGetCollectionInfo()
 		updateAndDeleteReq := &indexpb.AlterIndexRequest{
 			CollectionID: collID,
 			IndexName:    indexName,

internal/proxy/task_index.go

@@ -594,7 +594,7 @@ func (t *alterIndexTask) SetID(uid UniqueID) {
 }
 
 func (t *alterIndexTask) Name() string {
-	return CreateIndexTaskName
+	return AlterIndexTaskName
 }
 
 func (t *alterIndexTask) Type() commonpb.MsgType {

internal/proxy/validate_util.go

@@ -932,13 +932,5 @@ func newValidateUtil(opts ...validateOption) *validateUtil {
 }
 
 func ValidateAutoIndexMmapConfig(isVectorField bool, indexParams map[string]string) error {
-	if !Params.AutoIndexConfig.Enable.GetAsBool() {
+	return common.ValidateAutoIndexMmapConfig(Params.AutoIndexConfig.Enable.GetAsBool(), isVectorField, indexParams)
-		return nil
-	}
-
-	_, ok := indexParams[common.MmapEnabledKey]
-	if ok && isVectorField {
-		return fmt.Errorf("mmap index is not supported to config for the collection in auto index mode")
-	}
-	return nil
 }

pkg/common/common.go

@@ -430,3 +430,15 @@ func GetReplicateEndTS(kvs []*commonpb.KeyValuePair) (uint64, bool) {
 	}
 	return 0, false
 }
+
+func ValidateAutoIndexMmapConfig(autoIndexConfigEnable, isVectorField bool, indexParams map[string]string) error {
+	if !autoIndexConfigEnable {
+		return nil
+	}
+
+	_, ok := indexParams[MmapEnabledKey]
+	if ok && isVectorField {
+		return fmt.Errorf("mmap index is not supported to config for the collection in auto index mode")
+	}
+	return nil
+}

tests/restful_client_v2/api/milvus.py

@@ -525,7 +525,7 @@ class CollectionClient(Requests):
         url = f"{self.endpoint}/v2/vectordb/collections/drop_properties"
         payload = {
             "collectionName": collection_name,
-            "deleteKeys": delete_keys
+            "propertyKeys": delete_keys
         }
         if self.db_name is not None:
             payload["dbName"] = self.db_name
@@ -570,7 +570,7 @@ class CollectionClient(Requests):
         payload = {
             "collectionName": collection_name,
             "indexName": index_name,
-            "deleteKeys": delete_keys
+            "propertyKeys": delete_keys
         }
         if self.db_name is not None:
             payload["dbName"] = self.db_name