enhance: rewrite index params for compatibility (#35788)

#32900 Signed-off-by: luzhang <[email protected]> Co-authored-by: luzhang <[email protected]>
milvus-io · Sep 2, 2024 · 325f198 · 325f198
1 parent 3698c53
commit 325f198
Show file tree

Hide file tree

Showing 4 changed files with 160 additions and 3 deletions.
diff --git a/internal/datacoord/index_meta.go b/internal/datacoord/index_meta.go
@@ -210,6 +210,7 @@ func checkParams(fieldIndex *model.Index, req *indexpb.CreateIndexRequest) bool
 		for i, param2 := range req.GetUserIndexParams() {
 			if param2.Key == param1.Key && param2.Value == param1.Value {
 				exist = true
+				break
 			} else if param1.Key == common.MetricTypeKey && param2.Key == param1.Key && useAutoIndex && !req.GetUserAutoindexMetricTypeSpecified() {
 				// when users use autoindex, metric type is the only thing they can specify
 				// if they do not specify metric type, will use autoindex default metric type
@@ -225,14 +226,32 @@ func checkParams(fieldIndex *model.Index, req *indexpb.CreateIndexRequest) bool
 					}
 				}
 				exist = true
+				break
 			}
 		}
 		if !exist {
 			notEq = true
 			break
 		}
 	}
-
+	// Check whether new index type match old, if not, only
+	// allow autoindex config changed when upgraded to new config
+	// using store meta config to rewrite new config
+	if !notEq && req.GetIsAutoIndex() && useAutoIndex {
+		for _, param1 := range fieldIndex.IndexParams {
+			if param1.Key == common.IndexTypeKey &&
+				indexparamcheck.IsScalarIndexType(param1.Value) {
+				for _, param2 := range req.GetIndexParams() {
+					if param1.Key == param2.Key && param1.Value != param2.Value {
+						req.IndexParams = make([]*commonpb.KeyValuePair, len(fieldIndex.IndexParams))
+						copy(req.IndexParams, fieldIndex.IndexParams)
+						break
+					}
+				}
+			}
+		}
+	}
+	log.Info("final request", zap.Any("create index request", req.String()))
 	return !notEq
 }
 
@@ -254,8 +273,10 @@ func (m *indexMeta) CanCreateIndex(req *indexpb.CreateIndexRequest) (UniqueID, e
 			}
 			errMsg := "at most one distinct index is allowed per field"
 			log.Warn(errMsg,
-				zap.String("source index", fmt.Sprintf("{index_name: %s, field_id: %d, index_params: %v, type_params: %v}", index.IndexName, index.FieldID, index.IndexParams, index.TypeParams)),
-				zap.String("current index", fmt.Sprintf("{index_name: %s, field_id: %d, index_params: %v, type_params: %v}", req.GetIndexName(), req.GetFieldID(), req.GetIndexParams(), req.GetTypeParams())))
+				zap.String("source index", fmt.Sprintf("{index_name: %s, field_id: %d, index_params: %v, user_params: %v, type_params: %v}",
+					index.IndexName, index.FieldID, index.IndexParams, index.UserIndexParams, index.TypeParams)),
+				zap.String("current index", fmt.Sprintf("{index_name: %s, field_id: %d, index_params: %v, user_params: %v, type_params: %v}",
+					req.GetIndexName(), req.GetFieldID(), req.GetIndexParams(), req.GetUserIndexParams(), req.GetTypeParams())))
 			return 0, fmt.Errorf("CreateIndex failed: %s", errMsg)
 		}
 		if req.FieldID == index.FieldID {

diff --git a/internal/datacoord/index_meta_test.go b/internal/datacoord/index_meta_test.go
@@ -78,6 +78,136 @@ func TestReloadFromKV(t *testing.T) {
 	})
 }
 
+func TestMeta_ScalarAutoIndex(t *testing.T) {
+	var (
+		collID      = UniqueID(1)
+		indexID     = UniqueID(10)
+		fieldID     = UniqueID(100)
+		indexName   = "_default_idx"
+		typeParams  = []*commonpb.KeyValuePair{}
+		indexParams = []*commonpb.KeyValuePair{
+			{
+				Key:   common.IndexTypeKey,
+				Value: "HYBRID",
+			},
+		}
+		userIndexParams = []*commonpb.KeyValuePair{
+			{
+				Key:   common.IndexTypeKey,
+				Value: common.AutoIndexName,
+			},
+		}
+	)
+
+	catalog := catalogmocks.NewDataCoordCatalog(t)
+	m := newSegmentIndexMeta(catalog)
+
+	req := &indexpb.CreateIndexRequest{
+		CollectionID:    collID,
+		FieldID:         fieldID,
+		IndexName:       indexName,
+		TypeParams:      typeParams,
+		IndexParams:     indexParams,
+		Timestamp:       0,
+		IsAutoIndex:     true,
+		UserIndexParams: userIndexParams,
+	}
+
+	t.Run("user index params consistent", func(t *testing.T) {
+		m.indexes[collID] = map[UniqueID]*model.Index{
+			indexID: {
+				TenantID:        "",
+				CollectionID:    collID,
+				FieldID:         fieldID,
+				IndexID:         indexID,
+				IndexName:       indexName,
+				IsDeleted:       false,
+				CreateTime:      10,
+				TypeParams:      typeParams,
+				IndexParams:     indexParams,
+				IsAutoIndex:     false,
+				UserIndexParams: userIndexParams,
+			},
+		}
+		tmpIndexID, err := m.CanCreateIndex(req)
+		assert.NoError(t, err)
+		assert.Equal(t, int64(indexID), tmpIndexID)
+	})
+
+	t.Run("user index params not consistent", func(t *testing.T) {
+		m.indexes[collID] = map[UniqueID]*model.Index{
+			indexID: {
+				TenantID:        "",
+				CollectionID:    collID,
+				FieldID:         fieldID,
+				IndexID:         indexID,
+				IndexName:       indexName,
+				IsDeleted:       false,
+				CreateTime:      10,
+				TypeParams:      typeParams,
+				IndexParams:     indexParams,
+				IsAutoIndex:     false,
+				UserIndexParams: userIndexParams,
+			},
+		}
+		req.UserIndexParams = append(req.UserIndexParams, &commonpb.KeyValuePair{Key: "bitmap_cardinality_limit", Value: "1000"})
+		tmpIndexID, err := m.CanCreateIndex(req)
+		assert.Error(t, err)
+		assert.Equal(t, int64(0), tmpIndexID)
+
+		req.UserIndexParams = append(req.UserIndexParams, &commonpb.KeyValuePair{Key: "bitmap_cardinality_limit", Value: "500"})
+		tmpIndexID, err = m.CanCreateIndex(req)
+		assert.Error(t, err)
+		assert.Equal(t, int64(0), tmpIndexID)
+	})
+
+	req = &indexpb.CreateIndexRequest{
+		CollectionID: collID,
+		FieldID:      fieldID,
+		IndexName:    indexName,
+		TypeParams:   typeParams,
+		IndexParams: []*commonpb.KeyValuePair{
+			{
+				Key:   common.IndexTypeKey,
+				Value: "HYBRID",
+			}},
+		Timestamp:       0,
+		IsAutoIndex:     true,
+		UserIndexParams: userIndexParams,
+	}
+
+	t.Run("index param rewrite", func(t *testing.T) {
+		m.indexes[collID] = map[UniqueID]*model.Index{
+			indexID: {
+				TenantID:     "",
+				CollectionID: collID,
+				FieldID:      fieldID,
+				IndexID:      indexID,
+				IndexName:    indexName,
+				IsDeleted:    false,
+				CreateTime:   10,
+				TypeParams:   typeParams,
+				IndexParams: []*commonpb.KeyValuePair{
+					{
+						Key:   common.IndexTypeKey,
+						Value: "INVERTED",
+					},
+				},
+				IsAutoIndex:     false,
+				UserIndexParams: userIndexParams,
+			},
+		}
+		tmpIndexID, err := m.CanCreateIndex(req)
+		assert.NoError(t, err)
+		assert.Equal(t, int64(indexID), tmpIndexID)
+		newIndexParams := req.GetIndexParams()
+		assert.Equal(t, len(newIndexParams), 1)
+		assert.Equal(t, newIndexParams[0].Key, common.IndexTypeKey)
+		assert.Equal(t, newIndexParams[0].Value, "INVERTED")
+	})
+
+}
+
 func TestMeta_CanCreateIndex(t *testing.T) {
 	var (
 		collID = UniqueID(1)

diff --git a/internal/proxy/task_index.go b/internal/proxy/task_index.go
@@ -197,6 +197,7 @@ func (cit *createIndexTask) parseIndexParams() error {
 			}
 
 			indexParamsMap[common.IndexTypeKey] = indexType
+			cit.isAutoIndex = true
 		}
 	} else {
 		specifyIndexType, exist := indexParamsMap[common.IndexTypeKey]

diff --git a/pkg/util/indexparamcheck/index_type.go b/pkg/util/indexparamcheck/index_type.go
@@ -52,6 +52,11 @@ const (
 	AutoIndex IndexType = "AUTOINDEX"
 )
 
+func IsScalarIndexType(indexType IndexType) bool {
+	return indexType == IndexSTLSORT || indexType == IndexTRIE || indexType == IndexTrie ||
+		indexType == IndexBitmap || indexType == IndexHybrid || indexType == IndexINVERTED
+}
+
 func IsGpuIndex(indexType IndexType) bool {
 	return indexType == IndexGpuBF ||
 		indexType == IndexRaftIvfFlat ||