fix: lost dbname when only passing collection id to describeCollection (

milvus-io#31167) issue: milvus-io#30931 Signed-off-by: chyezh <[email protected]>
yellow-shine · Mar 11, 2024 · 6a9418d · 6a9418d
1 parent 58d7b9f
commit 6a9418d
Show file tree

Hide file tree

Showing 6 changed files with 38 additions and 7 deletions.
diff --git a/internal/metastore/kv/rootcoord/kv_catalog.go b/internal/metastore/kv/rootcoord/kv_catalog.go
@@ -210,7 +210,12 @@ func (kc *Catalog) loadCollectionFromDefaultDb(ctx context.Context, collectionID
 
 func (kc *Catalog) loadCollection(ctx context.Context, dbID int64, collectionID typeutil.UniqueID, ts typeutil.Timestamp) (*pb.CollectionInfo, error) {
 	if isDefaultDB(dbID) {
-		return kc.loadCollectionFromDefaultDb(ctx, collectionID, ts)
+		info, err := kc.loadCollectionFromDefaultDb(ctx, collectionID, ts)
+		if err != nil {
+			return nil, err
+		}
+		fixDefaultDBIDConsistency(info)
+		return info, nil
 	}
 	return kc.loadCollectionFromDb(ctx, dbID, collectionID, ts)
 }
@@ -625,6 +630,7 @@ func (kc *Catalog) ListCollections(ctx context.Context, dbID int64, ts typeutil.
 			log.Warn("unmarshal collection info failed", zap.Error(err))
 			continue
 		}
+		fixDefaultDBIDConsistency(&collMeta)
 		collection, err := kc.appendPartitionAndFieldsInfo(ctx, &collMeta, ts)
 		if err != nil {
 			return nil, err
@@ -1188,3 +1194,12 @@ func isDefaultDB(dbID int64) bool {
 	}
 	return false
 }
+
+// fixDefaultDBIDConsistency fix dbID consistency for collectionInfo.
+// We have two versions of default databaseID (0 at legacy path, 1 at new path), we should keep consistent view when user use default database.
+// all collections in default database should be marked with dbID 1.
+func fixDefaultDBIDConsistency(coll *pb.CollectionInfo) {
+	if isDefaultDB(coll.DbId) {
+		coll.DbId = util.DefaultDBID
+	}
+}
diff --git a/internal/metastore/kv/rootcoord/kv_catalog_test.go b/internal/metastore/kv/rootcoord/kv_catalog_test.go
@@ -72,7 +72,8 @@ func TestCatalog_ListCollections(t *testing.T) {
 	}
 
 	coll2 := &pb.CollectionInfo{
-		ID: 2,
+		ID:   2,
+		DbId: testDb,
 		Schema: &schemapb.CollectionSchema{
 			Name: "c1",
 			Fields: []*schemapb.FieldSchema{
@@ -172,6 +173,7 @@ func TestCatalog_ListCollections(t *testing.T) {
 		assert.NoError(t, err)
 		assert.Equal(t, 1, len(ret))
 		assert.Equal(t, coll1.ID, ret[0].CollectionID)
+		assert.Equal(t, util.DefaultDBID, ret[0].DBID)
 	})
 
 	t.Run("list collection with db", func(t *testing.T) {
@@ -208,6 +210,7 @@ func TestCatalog_ListCollections(t *testing.T) {
 		assert.NoError(t, err)
 		assert.NotNil(t, ret)
 		assert.Equal(t, 1, len(ret))
+		assert.Equal(t, int64(testDb), ret[0].DBID)
 	})
 
 	t.Run("list collection ok for the newest version", func(t *testing.T) {
@@ -376,6 +379,7 @@ func TestCatalog_GetCollectionByID(t *testing.T) {
 	coll, err = c.GetCollectionByID(ctx, 0, 10000, 1)
 	assert.NoError(t, err)
 	assert.NotNil(t, coll)
+	assert.Equal(t, util.DefaultDBID, coll.DBID)
 }
 
 func TestCatalog_CreatePartitionV2(t *testing.T) {

diff --git a/internal/rootcoord/describe_collection_task.go b/internal/rootcoord/describe_collection_task.go
@@ -45,7 +45,10 @@ func (t *describeCollectionTask) Execute(ctx context.Context) (err error) {
 		return err
 	}
 	aliases := t.core.meta.ListAliasesByID(coll.CollectionID)
-	t.Rsp = convertModelToDesc(coll, aliases)
-	t.Rsp.DbName = t.Req.GetDbName()
+	db, err := t.core.meta.GetDatabaseByID(ctx, coll.DBID, t.GetTs())
+	if err != nil {
+		return err
+	}
+	t.Rsp = convertModelToDesc(coll, aliases, db.Name)
 	return nil
 }
diff --git a/internal/rootcoord/describe_collection_task_test.go b/internal/rootcoord/describe_collection_task_test.go
@@ -101,10 +101,15 @@ func Test_describeCollectionTask_Execute(t *testing.T) {
 		).Return(&model.Collection{
 			CollectionID: 1,
 			Name:         "test coll",
+			DBID:         1,
 		}, nil)
 		meta.On("ListAliasesByID",
 			mock.Anything,
 		).Return([]string{alias1, alias2})
+		meta.EXPECT().GetDatabaseByID(mock.Anything, mock.Anything, mock.Anything).Return(&model.Database{
+			ID:   1,
+			Name: "test db",
+		}, nil)
 
 		core := newTestCore(withMeta(meta))
 		task := &describeCollectionTask{
@@ -120,6 +125,7 @@ func Test_describeCollectionTask_Execute(t *testing.T) {
 		err := task.Execute(context.Background())
 		assert.NoError(t, err)
 		assert.Equal(t, task.Rsp.GetStatus().GetErrorCode(), commonpb.ErrorCode_Success)
+		assert.Equal(t, "test db", task.Rsp.GetDbName())
 		assert.ElementsMatch(t, []string{alias1, alias2}, task.Rsp.GetAliases())
 	})
 }
diff --git a/internal/rootcoord/meta_table.go b/internal/rootcoord/meta_table.go
@@ -338,7 +338,7 @@ func (mt *MetaTable) GetDatabaseByName(ctx context.Context, dbName string, ts Ti
 	return mt.getDatabaseByNameInternal(ctx, dbName, ts)
 }
 
-func (mt *MetaTable) getDatabaseByNameInternal(ctx context.Context, dbName string, ts Timestamp) (*model.Database, error) {
+func (mt *MetaTable) getDatabaseByNameInternal(_ context.Context, dbName string, _ Timestamp) (*model.Database, error) {
 	// backward compatibility for rolling  upgrade
 	if dbName == "" {
 		log.Warn("db name is empty")

diff --git a/internal/rootcoord/root_coord.go b/internal/rootcoord/root_coord.go
@@ -1122,8 +1122,11 @@ func (c *Core) describeCollection(ctx context.Context, in *milvuspb.DescribeColl
 	return c.meta.GetCollectionByID(ctx, in.GetDbName(), in.GetCollectionID(), ts, allowUnavailable)
 }
 
-func convertModelToDesc(collInfo *model.Collection, aliases []string) *milvuspb.DescribeCollectionResponse {
-	resp := &milvuspb.DescribeCollectionResponse{Status: merr.Success()}
+func convertModelToDesc(collInfo *model.Collection, aliases []string, dbName string) *milvuspb.DescribeCollectionResponse {
+	resp := &milvuspb.DescribeCollectionResponse{
+		Status: merr.Success(),
+		DbName: dbName,
+	}
 
 	resp.Schema = &schemapb.CollectionSchema{
 		Name:               collInfo.Name,