mirror of
https://github.com/milvus-io/milvus.git
synced 2026-07-21 02:05:41 +00:00
fix: Return actual db name and db id in cached DescribeCollection queried by collection id (#51254)
issue: #51253 ### What this PR does When `proxy.enableCachedServiceProvider` is on and `DescribeCollection` is called with only a collection id (e.g. the HTTP management API on port 9091), the cached provider echoed the request db name — empty or default — instead of the database the collection actually belongs to, and never filled `db_id`. The remote path does not have this problem because the coordinator resolves the database by DBID and returns it in the top-level `DbName`/`DbId` fields. The proxy meta cache already stores the actual db name of each collection; this PR also stores the db id, and prefers the cached `dbName`/`dbID` when assembling the cached DescribeCollection response. ### Verification - New unit test `TestCachedProxyServiceProvider_DescribeCollection_ByIDReturnsActualDbName` (request carries only `collectionID`, asserts real `db_name`/`db_id` are returned); existing tests in the file still pass. - Verified end-to-end on a standalone build of this branch: created `db1`/`coll1`, queried `GET :9091/api/v1/collection` with only `collectionID` — response now returns `"db_name": "db1"` and the `db_id` matching `databases/describe`, where it previously returned neither. - The same logic was also verified on a 2.5-based deployment (real cluster) before porting to master. Happy to backport to 2.5/2.6 if needed. Signed-off-by: fudongying <fudongying@bytedance.com>
This commit is contained in:
@@ -105,6 +105,7 @@ type Cache interface {
|
||||
|
||||
type collectionInfo struct {
|
||||
collID typeutil.UniqueID
|
||||
dbID typeutil.UniqueID
|
||||
dbName string
|
||||
schema *schemaInfo
|
||||
createdTimestamp uint64
|
||||
@@ -501,6 +502,7 @@ func (m *MetaCache) update(ctx context.Context, database, collectionName string,
|
||||
mlog.Uint64("version", collection.GetRequestTime()), mlog.Uint64("cache version", curVersion))
|
||||
return &collectionInfo{
|
||||
collID: collection.CollectionID,
|
||||
dbID: collection.GetDbId(),
|
||||
dbName: collection.GetDbName(),
|
||||
schema: schemaInfo,
|
||||
createdTimestamp: collection.CreatedTimestamp,
|
||||
@@ -533,6 +535,7 @@ func (m *MetaCache) update(ctx context.Context, database, collectionName string,
|
||||
|
||||
m.collInfo[database][collectionName] = &collectionInfo{
|
||||
collID: collection.CollectionID,
|
||||
dbID: collection.GetDbId(),
|
||||
dbName: collection.GetDbName(),
|
||||
schema: schemaInfo,
|
||||
createdTimestamp: collection.CreatedTimestamp,
|
||||
|
||||
@@ -229,6 +229,12 @@ func (node *CachedProxyServiceProvider) DescribeCollection(ctx context.Context,
|
||||
return nil, err
|
||||
}
|
||||
|
||||
// prefer the actual database resolved by the coordinator and carried in the
|
||||
// cache, the request db name may be empty/default when querying by collection id
|
||||
if c.dbName != "" {
|
||||
resp.DbName = c.dbName
|
||||
}
|
||||
resp.DbId = c.dbID
|
||||
resp.CollectionID = c.collID
|
||||
resp.UpdateTimestamp = c.updateTimestamp
|
||||
resp.UpdateTimestampStr = fmt.Sprintf("%d", c.updateTimestamp)
|
||||
|
||||
@@ -142,3 +142,43 @@ func TestCachedProxyServiceProvider_DescribeCollection_FilterNamespaceField(t *t
|
||||
_, hasID := fieldNames["id"]
|
||||
assert.True(t, hasID)
|
||||
}
|
||||
|
||||
func TestCachedProxyServiceProvider_DescribeCollection_ByIDReturnsActualDbName(t *testing.T) {
|
||||
ctx := context.Background()
|
||||
|
||||
origCache := globalMetaCache
|
||||
defer func() { globalMetaCache = origCache }()
|
||||
|
||||
collectionID := int64(2000)
|
||||
schema := &schemapb.CollectionSchema{
|
||||
Name: "coll1",
|
||||
Fields: []*schemapb.FieldSchema{{
|
||||
FieldID: common.StartOfUserFieldID,
|
||||
Name: "id",
|
||||
IsPrimaryKey: true,
|
||||
DataType: schemapb.DataType_Int64,
|
||||
}},
|
||||
}
|
||||
|
||||
mockCache := &MockCache{}
|
||||
mockCache.EXPECT().GetCollectionName(mock.Anything, "", collectionID).Return("coll1", nil)
|
||||
mockCache.EXPECT().GetCollectionID(mock.Anything, "", "coll1").Return(collectionID, nil)
|
||||
mockCache.EXPECT().GetCollectionInfo(mock.Anything, "", "coll1", collectionID).Return(&collectionInfo{
|
||||
collID: collectionID,
|
||||
// resolved by the coordinator and carried in the cache entry
|
||||
dbID: 7,
|
||||
dbName: "db1",
|
||||
schema: newSchemaInfo(schema),
|
||||
shardsNum: common.DefaultShardsNum,
|
||||
}, nil)
|
||||
globalMetaCache = mockCache
|
||||
|
||||
provider := &CachedProxyServiceProvider{Proxy: &Proxy{}}
|
||||
// the monitoring http path passes only the collection id, both db name and
|
||||
// collection name are empty
|
||||
resp, err := provider.DescribeCollection(ctx, &milvuspb.DescribeCollectionRequest{CollectionID: collectionID})
|
||||
assert.NoError(t, err)
|
||||
assert.Equal(t, commonpb.ErrorCode_Success, resp.GetStatus().GetErrorCode())
|
||||
assert.Equal(t, "db1", resp.GetDbName())
|
||||
assert.Equal(t, int64(7), resp.GetDbId())
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user