diff --git a/internal/proxy/meta_cache.go b/internal/proxy/meta_cache.go index d794e47b81..8c3b100880 100644 --- a/internal/proxy/meta_cache.go +++ b/internal/proxy/meta_cache.go @@ -113,7 +113,6 @@ type collectionInfo struct { type databaseInfo struct { dbID typeutil.UniqueID createdTimestamp uint64 - properties map[string]string } // schemaInfo is a helper function wraps *schemapb.CollectionSchema @@ -1210,7 +1209,6 @@ func (m *MetaCache) GetDatabaseInfo(ctx context.Context, database string) (*data dbInfo := &databaseInfo{ dbID: resp.GetDbID(), createdTimestamp: resp.GetCreatedTimestamp(), - properties: funcutil.KeyValuePair2Map(resp.GetProperties()), } m.dbInfo[database] = dbInfo return dbInfo, nil diff --git a/internal/rootcoord/alter_collection_task.go b/internal/rootcoord/alter_collection_task.go index 7d5f9fa693..8fd05e11cb 100644 --- a/internal/rootcoord/alter_collection_task.go +++ b/internal/rootcoord/alter_collection_task.go @@ -59,6 +59,11 @@ func (a *alterCollectionTask) Execute(ctx context.Context) error { return err } + if ContainsKeyPairArray(a.Req.GetProperties(), oldColl.Properties) { + log.Info("skip to alter collection due to no changes were detected in the properties", zap.Int64("collectionID", oldColl.CollectionID)) + return nil + } + newColl := oldColl.Clone() newColl.Properties = MergeProperties(oldColl.Properties, a.Req.GetProperties()) diff --git a/internal/rootcoord/alter_collection_task_test.go b/internal/rootcoord/alter_collection_task_test.go index 7993fd5cce..7e75195f29 100644 --- a/internal/rootcoord/alter_collection_task_test.go +++ b/internal/rootcoord/alter_collection_task_test.go @@ -178,6 +178,45 @@ func Test_alterCollectionTask_Execute(t *testing.T) { assert.Error(t, err) }) + t.Run("alter successfully", func(t *testing.T) { + meta := mockrootcoord.NewIMetaTable(t) + meta.On("GetCollectionByName", + mock.Anything, + mock.Anything, + mock.Anything, + mock.Anything, + ).Return(&model.Collection{ + CollectionID: int64(1), + Properties: []*commonpb.KeyValuePair{ + { + Key: common.CollectionTTLConfigKey, + Value: "1", + }, + { + Key: common.CollectionAutoCompactionKey, + Value: "true", + }, + }, + }, nil) + core := newTestCore(withValidProxyManager(), withMeta(meta)) + task := &alterCollectionTask{ + baseTask: newBaseTask(context.Background(), core), + Req: &milvuspb.AlterCollectionRequest{ + Base: &commonpb.MsgBase{MsgType: commonpb.MsgType_AlterCollection}, + CollectionName: "cn", + Properties: []*commonpb.KeyValuePair{ + { + Key: common.CollectionAutoCompactionKey, + Value: "true", + }, + }, + }, + } + + err := task.Execute(context.Background()) + assert.NoError(t, err) + }) + t.Run("alter successfully", func(t *testing.T) { meta := mockrootcoord.NewIMetaTable(t) meta.On("GetCollectionByName", diff --git a/internal/rootcoord/alter_database_task.go b/internal/rootcoord/alter_database_task.go index 3fef13e504..292ba8bf3c 100644 --- a/internal/rootcoord/alter_database_task.go +++ b/internal/rootcoord/alter_database_task.go @@ -59,6 +59,11 @@ func (a *alterDatabaseTask) Execute(ctx context.Context) error { return err } + if ContainsKeyPairArray(a.Req.GetProperties(), oldDB.Properties) { + log.Info("skip to alter database due to no changes were detected in the properties", zap.String("databaseName", a.Req.GetDbName())) + return nil + } + newDB := oldDB.Clone() ret := MergeProperties(oldDB.Properties, a.Req.GetProperties()) newDB.Properties = ret diff --git a/internal/rootcoord/alter_database_task_test.go b/internal/rootcoord/alter_database_task_test.go index aa90efc9a2..99855d0d6e 100644 --- a/internal/rootcoord/alter_database_task_test.go +++ b/internal/rootcoord/alter_database_task_test.go @@ -76,6 +76,41 @@ func Test_alterDatabaseTask_Execute(t *testing.T) { assert.Error(t, err) }) + meta := mockrootcoord.NewIMetaTable(t) + properties = append(properties, &commonpb.KeyValuePair{Key: common.DatabaseForceDenyReadingKey, Value: "true"}) + + meta.On("GetDatabaseByName", + mock.Anything, + mock.Anything, + mock.Anything, + mock.Anything, + ).Return(&model.Database{ID: int64(1), Properties: properties}, nil).Maybe() + meta.On("AlterDatabase", + mock.Anything, + mock.Anything, + mock.Anything, + mock.Anything, + ).Return(nil).Maybe() + + t.Run("alter skip due to no change", func(t *testing.T) { + core := newTestCore(withMeta(meta)) + task := &alterDatabaseTask{ + baseTask: newBaseTask(context.Background(), core), + Req: &rootcoordpb.AlterDatabaseRequest{ + DbName: "cn", + Properties: []*commonpb.KeyValuePair{ + { + Key: common.DatabaseForceDenyReadingKey, + Value: "true", + }, + }, + }, + } + + err := task.Execute(context.Background()) + assert.NoError(t, err) + }) + t.Run("alter step failed", func(t *testing.T) { meta := mockrootcoord.NewIMetaTable(t) meta.On("GetDatabaseByName", diff --git a/internal/rootcoord/util.go b/internal/rootcoord/util.go index 51d7bcc0f6..88a27e758d 100644 --- a/internal/rootcoord/util.go +++ b/internal/rootcoord/util.go @@ -56,6 +56,23 @@ func EqualKeyPairArray(p1 []*commonpb.KeyValuePair, p2 []*commonpb.KeyValuePair) return false } } + return ContainsKeyPairArray(p1, p2) +} + +func ContainsKeyPairArray(src []*commonpb.KeyValuePair, target []*commonpb.KeyValuePair) bool { + m1 := make(map[string]string) + for _, p := range target { + m1[p.Key] = p.Value + } + for _, p := range src { + val, ok := m1[p.Key] + if !ok { + return false + } + if val != p.Value { + return false + } + } return true }