Prohibit creating diff indexes with same index_name on a collection (#16774)

Signed-off-by: Nxige-16 <xi.ge@zilliz.com>

internal/rootcoord/meta_table.go

@@ -1056,37 +1056,28 @@ func (mt *MetaTable) GetNotIndexedSegments(collName string, fieldName string, id
 		return nil, schemapb.FieldSchema{}, fmt.Errorf("collection %s not found", collName)
 	}
 
-	var dupIdx typeutil.UniqueID
+	dupIdx := false
 	for _, f := range collMeta.FieldIndexes {
 		if info, ok := mt.indexID2Meta[f.IndexID]; ok {
 			if info.IndexName == idxInfo.IndexName {
-				dupIdx = info.IndexID
-				break
-			}
-		}
-	}
+				// the index name must be different for different indexes
+				if !EqualKeyPairArray(info.IndexParams, idxInfo.IndexParams) || f.FiledID != fieldSchema.FieldID {
+					return nil, schemapb.FieldSchema{}, fmt.Errorf("index name(%s) has been exist in collectio(%s), field(%s)", info.IndexName, collName, fieldName)
+				}
 
-	exist := false
-	var existInfo pb.IndexInfo
-	for _, f := range collMeta.FieldIndexes {
-		if f.FiledID == fieldSchema.FieldID {
-			existInfo, ok = mt.indexID2Meta[f.IndexID]
-			if !ok {
-				return nil, schemapb.FieldSchema{}, fmt.Errorf("index id = %d not found", f.IndexID)
-			}
-			if EqualKeyPairArray(existInfo.IndexParams, idxInfo.IndexParams) {
-				exist = true
-				break
+				// same index name, index params, and fieldId
+				dupIdx = true
 			}
 		}
 	}
-	if !exist {
+
+	// if no same index exist, save new index info to etcd
+	if !dupIdx {
 		idx := &pb.FieldIndexInfo{
 			FiledID: fieldSchema.FieldID,
 			IndexID: idxInfo.IndexID,
 		}
 		collMeta.FieldIndexes = append(collMeta.FieldIndexes, idx)
-		mt.collID2Meta[collMeta.ID] = collMeta
 		k1 := path.Join(CollectionMetaPrefix, strconv.FormatInt(collMeta.ID, 10))
 		v1, err := proto.Marshal(&collMeta)
 		if err != nil {
@@ -1095,7 +1086,6 @@ func (mt *MetaTable) GetNotIndexedSegments(collName string, fieldName string, id
 			return nil, schemapb.FieldSchema{}, fmt.Errorf("metaTable GetNotIndexedSegments Marshal collMeta fail key:%s, err:%w", k1, err)
 		}
 
-		mt.indexID2Meta[idx.IndexID] = *idxInfo
 		k2 := path.Join(IndexMetaPrefix, strconv.FormatInt(idx.IndexID, 10))
 		v2, err := proto.Marshal(idxInfo)
 		if err != nil {
@@ -1105,57 +1095,14 @@ func (mt *MetaTable) GetNotIndexedSegments(collName string, fieldName string, id
 		}
 		meta := map[string]string{k1: string(v1), k2: string(v2)}
 
-		if dupIdx != 0 {
-			dupInfo := mt.indexID2Meta[dupIdx]
-			dupInfo.IndexName = dupInfo.IndexName + "_bak"
-			mt.indexID2Meta[dupIdx] = dupInfo
-			k := path.Join(IndexMetaPrefix, strconv.FormatInt(dupInfo.IndexID, 10))
-			v, err := proto.Marshal(&dupInfo)
-			if err != nil {
-				log.Error("MetaTable GetNotIndexedSegments Marshal dupInfo fail",
-					zap.String("key", k), zap.Error(err))
-				return nil, schemapb.FieldSchema{}, fmt.Errorf("metaTable GetNotIndexedSegments Marshal dupInfo fail key:%s, err:%w", k, err)
-			}
-			meta[k] = string(v)
-		}
 		err = mt.txn.MultiSave(meta)
 		if err != nil {
 			log.Error("TxnKV MultiSave fail", zap.Error(err))
 			panic("TxnKV MultiSave fail")
 		}
-	} else {
-		idxInfo.IndexID = existInfo.IndexID
-		if existInfo.IndexName != idxInfo.IndexName { //replace index name
-			existInfo.IndexName = idxInfo.IndexName
-			mt.indexID2Meta[existInfo.IndexID] = existInfo
-			k := path.Join(IndexMetaPrefix, strconv.FormatInt(existInfo.IndexID, 10))
-			v, err := proto.Marshal(&existInfo)
-			if err != nil {
-				log.Error("MetaTable GetNotIndexedSegments Marshal existInfo fail",
-					zap.String("key", k), zap.Error(err))
-				return nil, schemapb.FieldSchema{}, fmt.Errorf("metaTable GetNotIndexedSegments Marshal existInfo fail key:%s, err:%w", k, err)
-			}
-			meta := map[string]string{k: string(v)}
-			if dupIdx != 0 {
-				dupInfo := mt.indexID2Meta[dupIdx]
-				dupInfo.IndexName = dupInfo.IndexName + "_bak"
-				mt.indexID2Meta[dupIdx] = dupInfo
-				k := path.Join(IndexMetaPrefix, strconv.FormatInt(dupInfo.IndexID, 10))
-				v, err := proto.Marshal(&dupInfo)
-				if err != nil {
-					log.Error("MetaTable GetNotIndexedSegments Marshal dupInfo fail",
-						zap.String("key", k), zap.Error(err))
-					return nil, schemapb.FieldSchema{}, fmt.Errorf("metaTable GetNotIndexedSegments Marshal dupInfo fail key:%s, err:%w", k, err)
-				}
-				meta[k] = string(v)
-			}
 
-			err = mt.txn.MultiSave(meta)
-			if err != nil {
-				log.Error("SnapShotKV MultiSave fail", zap.Error(err))
-				panic("SnapShotKV MultiSave fail")
-			}
-		}
+		mt.collID2Meta[collMeta.ID] = collMeta
+		mt.indexID2Meta[idx.IndexID] = *idxInfo
 	}
 
 	rstID := make([]typeutil.UniqueID, 0, 16)

internal/rootcoord/meta_table_test.go

@@ -387,6 +387,29 @@ func TestMetaTable(t *testing.T) {
 		assert.EqualError(t, err, fmt.Sprintf("index id = %d exist", segIdxInfo.IndexID))
 	})
 
+	wg.Add(1)
+	t.Run("add diff index with same index name", func(t *testing.T) {
+		defer wg.Done()
+		params := []*commonpb.KeyValuePair{
+			{
+				Key:   "field110-k1",
+				Value: "field110-v1",
+			},
+			{
+				Key:   "field110-k2",
+				Value: "field110-v2",
+			},
+		}
+		idxInfo := &pb.IndexInfo{
+			IndexName:   "testColl_index_110",
+			IndexID:     indexID,
+			IndexParams: params,
+		}
+
+		_, _, err := mt.GetNotIndexedSegments("collTest", "field110", idxInfo, nil)
+		assert.NotNil(t, err)
+	})
+
 	wg.Add(1)
 	t.Run("get not indexed segments", func(t *testing.T) {
 		defer wg.Done()
@@ -441,7 +464,6 @@ func TestMetaTable(t *testing.T) {
 		assert.Equal(t, segID, seg[0])
 		assert.Equal(t, segID2, seg[1])
 		assert.True(t, EqualKeyPairArray(field.TypeParams, tparams))
-
 	})
 
 	wg.Add(1)
@@ -450,7 +472,7 @@ func TestMetaTable(t *testing.T) {
 		_, idx, err := mt.GetIndexByName(collName, "field110")
 		assert.Nil(t, err)
 		assert.Equal(t, 1, len(idx))
-		assert.Equal(t, indexID, idx[0].IndexID)
+		assert.Equal(t, int64(2000), idx[0].IndexID)
 		params := []*commonpb.KeyValuePair{
 			{
 				Key:   "field110-i1",
@@ -487,7 +509,7 @@ func TestMetaTable(t *testing.T) {
 		idx, ok, err := mt.DropIndex(collName, "field110", "field110")
 		assert.Nil(t, err)
 		assert.True(t, ok)
-		assert.Equal(t, indexID, idx)
+		assert.Equal(t, int64(2000), idx)
 
 		_, ok, err = mt.DropIndex(collName, "field110", "field110-error")
 		assert.Nil(t, err)
@@ -1024,8 +1046,7 @@ func TestMetaTable(t *testing.T) {
 		bakMeta := mt.indexID2Meta
 		mt.indexID2Meta = make(map[int64]pb.IndexInfo)
 		_, _, err = mt.GetNotIndexedSegments(collInfo.Schema.Name, collInfo.Schema.Fields[0].Name, idx, nil)
-		assert.NotNil(t, err)
-		assert.EqualError(t, err, fmt.Sprintf("index id = %d not found", idxInfo[0].IndexID))
+		assert.Nil(t, err)
 		mt.indexID2Meta = bakMeta
 
 		mockTxnKV.multiSave = func(kvs map[string]string) error {
@@ -1049,19 +1070,7 @@ func TestMetaTable(t *testing.T) {
 		coll.FieldIndexes = append(coll.FieldIndexes, &pb.FieldIndexInfo{FiledID: coll.FieldIndexes[0].FiledID, IndexID: coll.FieldIndexes[0].IndexID + 1})
 
 		mt.collID2Meta[coll.ID] = coll
-		anotherIdx := pb.IndexInfo{
-			IndexName: "no-index",
-			IndexID:   coll.FieldIndexes[1].IndexID,
-			IndexParams: []*commonpb.KeyValuePair{
-				{
-					Key:   "no-idx-k1",
-					Value: "no-idx-v1",
-				},
-			},
-		}
-		mt.indexID2Meta[anotherIdx.IndexID] = anotherIdx
-
-		idx.IndexName = idxInfo[0].IndexName
+		idx.IndexName = "no-index-2"
 		mockTxnKV.multiSave = func(kvs map[string]string) error {
 			return fmt.Errorf("multi save error")
 		}

internal/rootcoord/root_coord_test.go

@@ -1561,26 +1561,10 @@ func TestRootCoord_Base(t *testing.T) {
 		collMeta, err := core.MetaTable.GetCollectionByName(collName, 0)
 		assert.NoError(t, err)
 		assert.Equal(t, 1, len(collMeta.FieldIndexes))
-		oldIdx := collMeta.FieldIndexes[0].IndexID
 
 		rsp, err := core.CreateIndex(ctx, req)
 		assert.NoError(t, err)
-		assert.Equal(t, commonpb.ErrorCode_Success, rsp.ErrorCode)
-		time.Sleep(100 * time.Millisecond)
-
-		collMeta, err = core.MetaTable.GetCollectionByName(collName, 0)
-		assert.NoError(t, err)
-		assert.Equal(t, 2, len(collMeta.FieldIndexes))
-		assert.Equal(t, oldIdx, collMeta.FieldIndexes[0].IndexID)
-
-		idxMeta, err := core.MetaTable.GetIndexByID(collMeta.FieldIndexes[1].IndexID)
-		assert.NoError(t, err)
-		assert.Equal(t, Params.CommonCfg.DefaultIndexName, idxMeta.IndexName)
-
-		idxMeta, err = core.MetaTable.GetIndexByID(collMeta.FieldIndexes[0].IndexID)
-		assert.NoError(t, err)
-		assert.Equal(t, Params.CommonCfg.DefaultIndexName+"_bak", idxMeta.IndexName)
-
+		assert.Equal(t, commonpb.ErrorCode_UnexpectedError, rsp.ErrorCode)
 	})
 
 	wg.Add(1)

tests/python_client/testcases/test_collection.py

@@ -2547,6 +2547,7 @@ class TestLoadPartition(TestcaseBase):
             pytest.skip("Skip index Temporary")
 
     @pytest.mark.tags(CaseLabel.L0)
+    @pytest.mark.skip("https://github.com/milvus-io/milvus/issues/16741")
     def test_load_partition_after_index_binary(self, get_binary_index):
         """
         target: test load binary_collection, after index created

tests/python_client/testcases/test_index.py

@@ -145,6 +145,7 @@ class TestIndexOperation(TestcaseBase):
     """ Test case of index interface """
 
     @pytest.mark.tags(CaseLabel.L1)
+    @pytest.mark.skip("https://github.com/milvus-io/milvus/issues/16741")
     def test_index_create_with_different_indexes(self):
         """
         target: test create index on one field, with two different type of index