diff --git a/internal/rootcoord/meta_table.go b/internal/rootcoord/meta_table.go index efe4f45e78c76916ba8316fcd9fdd44e94a6c447..868625f9d30bf691be975a1a38ece3ab980c2e34 100644 --- a/internal/rootcoord/meta_table.go +++ b/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) diff --git a/internal/rootcoord/meta_table_test.go b/internal/rootcoord/meta_table_test.go index 76b4f08d0b3e6060ad3edad09c9f39d5123ec296..20be0c69bc21386378ab017b84fdfeb28fcb1ae7 100644 --- a/internal/rootcoord/meta_table_test.go +++ b/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") } diff --git a/internal/rootcoord/root_coord_test.go b/internal/rootcoord/root_coord_test.go index 8f36ddb6a3613968be37272805b209798c33fb3d..d7a64b403a601089afeae1c1ffebdc5c2a93f87a 100644 --- a/internal/rootcoord/root_coord_test.go +++ b/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) diff --git a/tests/python_client/testcases/test_collection.py b/tests/python_client/testcases/test_collection.py index 97338a28808123bdbd85f9679bf45a38598cb49b..91b827a9e844a0f63226a82821e15b8b1287c050 100644 --- a/tests/python_client/testcases/test_collection.py +++ b/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 diff --git a/tests/python_client/testcases/test_index.py b/tests/python_client/testcases/test_index.py index 35ec82e5b8f57587d39dd05cab0ba7038ccb55d2..1b41238c6cb6a1f00666f5bf941008eea2d9f761 100644 --- a/tests/python_client/testcases/test_index.py +++ b/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