Skip to content

Commit 5713620

Browse files
authored
enhance: skip alter operation when no change are detected (milvus-io#36785)
issue: milvus-io#36784 Signed-off-by: jaime <yun.zhang@zilliz.com>
1 parent 5b26583 commit 5713620

6 files changed

Lines changed: 101 additions & 2 deletions

File tree

internal/proxy/meta_cache.go

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,6 @@ type collectionInfo struct {
113113
type databaseInfo struct {
114114
dbID typeutil.UniqueID
115115
createdTimestamp uint64
116-
properties map[string]string
117116
}
118117

119118
// schemaInfo is a helper function wraps *schemapb.CollectionSchema
@@ -1210,7 +1209,6 @@ func (m *MetaCache) GetDatabaseInfo(ctx context.Context, database string) (*data
12101209
dbInfo := &databaseInfo{
12111210
dbID: resp.GetDbID(),
12121211
createdTimestamp: resp.GetCreatedTimestamp(),
1213-
properties: funcutil.KeyValuePair2Map(resp.GetProperties()),
12141212
}
12151213
m.dbInfo[database] = dbInfo
12161214
return dbInfo, nil

internal/rootcoord/alter_collection_task.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ func (a *alterCollectionTask) Execute(ctx context.Context) error {
5959
return err
6060
}
6161

62+
if ContainsKeyPairArray(a.Req.GetProperties(), oldColl.Properties) {
63+
log.Info("skip to alter collection due to no changes were detected in the properties", zap.Int64("collectionID", oldColl.CollectionID))
64+
return nil
65+
}
66+
6267
newColl := oldColl.Clone()
6368
newColl.Properties = MergeProperties(oldColl.Properties, a.Req.GetProperties())
6469

internal/rootcoord/alter_collection_task_test.go

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,45 @@ func Test_alterCollectionTask_Execute(t *testing.T) {
178178
assert.Error(t, err)
179179
})
180180

181+
t.Run("alter successfully", func(t *testing.T) {
182+
meta := mockrootcoord.NewIMetaTable(t)
183+
meta.On("GetCollectionByName",
184+
mock.Anything,
185+
mock.Anything,
186+
mock.Anything,
187+
mock.Anything,
188+
).Return(&model.Collection{
189+
CollectionID: int64(1),
190+
Properties: []*commonpb.KeyValuePair{
191+
{
192+
Key: common.CollectionTTLConfigKey,
193+
Value: "1",
194+
},
195+
{
196+
Key: common.CollectionAutoCompactionKey,
197+
Value: "true",
198+
},
199+
},
200+
}, nil)
201+
core := newTestCore(withValidProxyManager(), withMeta(meta))
202+
task := &alterCollectionTask{
203+
baseTask: newBaseTask(context.Background(), core),
204+
Req: &milvuspb.AlterCollectionRequest{
205+
Base: &commonpb.MsgBase{MsgType: commonpb.MsgType_AlterCollection},
206+
CollectionName: "cn",
207+
Properties: []*commonpb.KeyValuePair{
208+
{
209+
Key: common.CollectionAutoCompactionKey,
210+
Value: "true",
211+
},
212+
},
213+
},
214+
}
215+
216+
err := task.Execute(context.Background())
217+
assert.NoError(t, err)
218+
})
219+
181220
t.Run("alter successfully", func(t *testing.T) {
182221
meta := mockrootcoord.NewIMetaTable(t)
183222
meta.On("GetCollectionByName",

internal/rootcoord/alter_database_task.go

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,11 @@ func (a *alterDatabaseTask) Execute(ctx context.Context) error {
5959
return err
6060
}
6161

62+
if ContainsKeyPairArray(a.Req.GetProperties(), oldDB.Properties) {
63+
log.Info("skip to alter database due to no changes were detected in the properties", zap.String("databaseName", a.Req.GetDbName()))
64+
return nil
65+
}
66+
6267
newDB := oldDB.Clone()
6368
ret := MergeProperties(oldDB.Properties, a.Req.GetProperties())
6469
newDB.Properties = ret

internal/rootcoord/alter_database_task_test.go

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,41 @@ func Test_alterDatabaseTask_Execute(t *testing.T) {
7676
assert.Error(t, err)
7777
})
7878

79+
meta := mockrootcoord.NewIMetaTable(t)
80+
properties = append(properties, &commonpb.KeyValuePair{Key: common.DatabaseForceDenyReadingKey, Value: "true"})
81+
82+
meta.On("GetDatabaseByName",
83+
mock.Anything,
84+
mock.Anything,
85+
mock.Anything,
86+
mock.Anything,
87+
).Return(&model.Database{ID: int64(1), Properties: properties}, nil).Maybe()
88+
meta.On("AlterDatabase",
89+
mock.Anything,
90+
mock.Anything,
91+
mock.Anything,
92+
mock.Anything,
93+
).Return(nil).Maybe()
94+
95+
t.Run("alter skip due to no change", func(t *testing.T) {
96+
core := newTestCore(withMeta(meta))
97+
task := &alterDatabaseTask{
98+
baseTask: newBaseTask(context.Background(), core),
99+
Req: &rootcoordpb.AlterDatabaseRequest{
100+
DbName: "cn",
101+
Properties: []*commonpb.KeyValuePair{
102+
{
103+
Key: common.DatabaseForceDenyReadingKey,
104+
Value: "true",
105+
},
106+
},
107+
},
108+
}
109+
110+
err := task.Execute(context.Background())
111+
assert.NoError(t, err)
112+
})
113+
79114
t.Run("alter step failed", func(t *testing.T) {
80115
meta := mockrootcoord.NewIMetaTable(t)
81116
meta.On("GetDatabaseByName",

internal/rootcoord/util.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,23 @@ func EqualKeyPairArray(p1 []*commonpb.KeyValuePair, p2 []*commonpb.KeyValuePair)
5656
return false
5757
}
5858
}
59+
return ContainsKeyPairArray(p1, p2)
60+
}
61+
62+
func ContainsKeyPairArray(src []*commonpb.KeyValuePair, target []*commonpb.KeyValuePair) bool {
63+
m1 := make(map[string]string)
64+
for _, p := range target {
65+
m1[p.Key] = p.Value
66+
}
67+
for _, p := range src {
68+
val, ok := m1[p.Key]
69+
if !ok {
70+
return false
71+
}
72+
if val != p.Value {
73+
return false
74+
}
75+
}
5976
return true
6077
}
6178

0 commit comments

Comments
 (0)