enhance: [2.4]alterdatabase support delete property (#38450)

alterdatabase support delete property issue: #38379 --------- Signed-off-by: Xianhui.Lin <[email protected]>
milvus-io · Dec 15, 2024 · 352e51a · 352e51a
1 parent 4c896c6
commit 352e51a
Show file tree

Hide file tree

Showing 12 changed files with 106 additions and 16 deletions.
diff --git a/go.mod b/go.mod
@@ -26,7 +26,7 @@ require (
 	github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
 	github.com/klauspost/compress v1.17.9
 	github.com/mgutz/ansi v0.0.0-20200706080929-d51e80ef957d
-	github.com/milvus-io/milvus-proto/go-api/v2 v2.4.18-0.20241204065031-4466aae60fc2
+	github.com/milvus-io/milvus-proto/go-api/v2 v2.4.18-0.20241212110236-7f744ed2ad11
 	github.com/minio/minio-go/v7 v7.0.73
 	github.com/pingcap/log v1.1.1-0.20221015072633-39906604fb81
 	github.com/prometheus/client_golang v1.14.0

diff --git a/go.sum b/go.sum
@@ -616,6 +616,8 @@ github.com/milvus-io/gorocksdb v0.0.0-20220624081344-8c5f4212846b h1:TfeY0NxYxZz
 github.com/milvus-io/gorocksdb v0.0.0-20220624081344-8c5f4212846b/go.mod h1:iwW+9cWfIzzDseEBCCeDSN5SD16Tidvy8cwQ7ZY8Qj4=
 github.com/milvus-io/milvus-proto/go-api/v2 v2.4.18-0.20241204065031-4466aae60fc2 h1:I3lqktgaXm87Lbsj1UmjSttq+APd+b6IdB2GamPB9cg=
 github.com/milvus-io/milvus-proto/go-api/v2 v2.4.18-0.20241204065031-4466aae60fc2/go.mod h1:/6UT4zZl6awVeXLeE7UGDWZvXj3IWkRsh3mqsn0DiAs=
+github.com/milvus-io/milvus-proto/go-api/v2 v2.4.18-0.20241212110236-7f744ed2ad11 h1:Mq+jtk/gSU6oMP5DPjMJHuRVBVDwxx61MKLxSCk1osg=
+github.com/milvus-io/milvus-proto/go-api/v2 v2.4.18-0.20241212110236-7f744ed2ad11/go.mod h1:/6UT4zZl6awVeXLeE7UGDWZvXj3IWkRsh3mqsn0DiAs=
 github.com/milvus-io/milvus-storage/go v0.0.0-20231227072638-ebd0b8e56d70 h1:Z+sp64fmAOxAG7mU0dfVOXvAXlwRB0c8a96rIM5HevI=
 github.com/milvus-io/milvus-storage/go v0.0.0-20231227072638-ebd0b8e56d70/go.mod h1:GPETMcTZq1gLY1WA6Na5kiNAKnq8SEMMiVKUZrM3sho=
 github.com/milvus-io/pulsar-client-go v0.6.10 h1:eqpJjU+/QX0iIhEo3nhOqMNXL+TyInAs1IAHZCrCM/A=

diff --git a/internal/proto/root_coord.proto b/internal/proto/root_coord.proto
@@ -233,4 +233,5 @@ message AlterDatabaseRequest {
   string db_name = 2;
   string db_id = 3;
   repeated common.KeyValuePair properties = 4;
+  repeated string delete_keys = 5;
 }
diff --git a/internal/proxy/task.go b/internal/proxy/task.go
@@ -1109,6 +1109,10 @@ func (t *alterCollectionFieldTask) OnEnqueue() error {
 	return nil
 }
 
+const (
+	MmapEnabledKey = "mmap_enabled"
+)
+
 var allowedProps = []string{
 	common.MaxLengthKey,
 	common.MmapEnabledKey,
@@ -1123,12 +1127,35 @@ func IsKeyAllowed(key string) bool {
 	return false
 }
 
+func updatePropertiesKeys(oldProps []*commonpb.KeyValuePair) []*commonpb.KeyValuePair {
+	props := make(map[string]string)
+	for _, prop := range oldProps {
+		var updatedKey string
+		if prop.Key == MmapEnabledKey {
+			updatedKey = common.MmapEnabledKey
+		} else {
+			updatedKey = prop.Key
+		}
+		props[updatedKey] = prop.Value
+	}
+
+	propKV := make([]*commonpb.KeyValuePair, 0)
+	for key, value := range props {
+		propKV = append(propKV, &commonpb.KeyValuePair{
+			Key:   key,
+			Value: value,
+		})
+	}
+
+	return propKV
+}
+
 func (t *alterCollectionFieldTask) PreExecute(ctx context.Context) error {
 	collSchema, err := globalMetaCache.GetCollectionSchema(ctx, t.GetDbName(), t.CollectionName)
 	if err != nil {
 		return err
 	}
-
+	t.Properties = updatePropertiesKeys(t.Properties)
 	for _, prop := range t.Properties {
 		if !IsKeyAllowed(prop.Key) {
 			return merr.WrapErrParameterInvalidMsg("%s does not allow update in collection field param", prop.Key)
@@ -1147,6 +1174,7 @@ func (t *alterCollectionFieldTask) PreExecute(ctx context.Context) error {
 			if loaded {
 				return merr.WrapErrCollectionLoaded(t.CollectionName, "can not alter collection field properties if collection loaded")
 			}
+
 		case common.MaxLengthKey:
 			IsStringType := false
 			fieldName := ""

diff --git a/internal/proxy/task_database.go b/internal/proxy/task_database.go
@@ -285,6 +285,7 @@ func (t *alterDatabaseTask) Execute(ctx context.Context) error {
 		DbName:     t.AlterDatabaseRequest.GetDbName(),
 		DbId:       t.AlterDatabaseRequest.GetDbId(),
 		Properties: t.AlterDatabaseRequest.GetProperties(),
+		DeleteKeys: t.AlterDatabaseRequest.GetDeleteKeys(),
 	}
 
 	ret, err := t.rootCoord.AlterDatabase(ctx, req)

diff --git a/internal/proxy/task_database_test.go b/internal/proxy/task_database_test.go
@@ -185,6 +185,20 @@ func TestAlterDatabase(t *testing.T) {
 
 	err = task.Execute(context.Background())
 	assert.Nil(t, err)
+
+	task1 := &alterDatabaseTask{
+		AlterDatabaseRequest: &milvuspb.AlterDatabaseRequest{
+			Base:       &commonpb.MsgBase{},
+			DbName:     "test_alter_database",
+			DeleteKeys: []string{common.MmapEnabledKey},
+		},
+		rootCoord: rc,
+	}
+	err1 := task1.PreExecute(context.Background())
+	assert.Nil(t, err1)
+
+	err1 = task1.Execute(context.Background())
+	assert.Nil(t, err1)
 }
 
 func TestDescribeDatabase(t *testing.T) {

diff --git a/internal/rootcoord/alter_collection_task.go b/internal/rootcoord/alter_collection_task.go
@@ -145,7 +145,6 @@ func DeleteProperties(oldProps []*commonpb.KeyValuePair, deleteKeys []string) []
 	for key, value := range propsMap {
 		propKV = append(propKV, &commonpb.KeyValuePair{Key: key, Value: value})
 	}
-	log.Info("Alter Collection Drop Properties", zap.Any("newProperties", propKV))
 	return propKV
 }
 

diff --git a/internal/rootcoord/alter_database_task.go b/internal/rootcoord/alter_database_task.go
@@ -47,9 +47,13 @@ func (a *alterDatabaseTask) Prepare(ctx context.Context) error {
 }
 
 func (a *alterDatabaseTask) Execute(ctx context.Context) error {
-	// Now we only support alter properties of database
-	if a.Req.GetProperties() == nil {
-		return errors.New("only support alter database properties, but database properties is empty")
+	// Now we support alter and delete properties of database
+	if a.Req.GetProperties() == nil && a.Req.GetDeleteKeys() == nil {
+		return errors.New("alter database requires either properties or deletekeys to modify or delete keys, both cannot be empty")
+	}
+
+	if len(a.Req.GetProperties()) > 0 && len(a.Req.GetDeleteKeys()) > 0 {
+		return errors.New("alter database operation cannot modify properties and delete keys at the same time")
 	}
 
 	oldDB, err := a.core.meta.GetDatabaseByName(ctx, a.Req.GetDbName(), a.ts)
@@ -59,14 +63,18 @@ 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
+	if (len(a.Req.GetProperties())) > 0 {
+		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
+		}
+		ret := MergeProperties(oldDB.Properties, a.Req.GetProperties())
+		newDB.Properties = ret
+	} else if (len(a.Req.GetDeleteKeys())) > 0 {
+		ret := DeleteProperties(oldDB.Properties, a.Req.GetDeleteKeys())
+		newDB.Properties = ret
+	}
 
 	ts := a.GetTs()
 	redoTask := newBaseRedoTask(a.core.stepExecutor)

diff --git a/internal/rootcoord/alter_database_task_test.go b/internal/rootcoord/alter_database_task_test.go
@@ -212,4 +212,39 @@ func Test_alterDatabaseTask_Execute(t *testing.T) {
 			Value: "true",
 		})
 	})
+
+	t.Run("test delete collection props", func(t *testing.T) {
+		oldProps := []*commonpb.KeyValuePair{
+			{
+				Key:   common.CollectionTTLConfigKey,
+				Value: "1",
+			},
+		}
+
+		deleteKeys := []string{
+			common.CollectionAutoCompactionKey,
+		}
+
+		ret := DeleteProperties(oldProps, deleteKeys)
+
+		assert.Contains(t, ret, &commonpb.KeyValuePair{
+			Key:   common.CollectionTTLConfigKey,
+			Value: "1",
+		})
+
+		oldProps2 := []*commonpb.KeyValuePair{
+			{
+				Key:   common.CollectionTTLConfigKey,
+				Value: "1",
+			},
+		}
+
+		deleteKeys2 := []string{
+			common.CollectionTTLConfigKey,
+		}
+
+		ret2 := DeleteProperties(oldProps2, deleteKeys2)
+
+		assert.Empty(t, ret2)
+	})
 }
diff --git a/pkg/go.mod b/pkg/go.mod
@@ -12,7 +12,7 @@ require (
 	github.com/expr-lang/expr v1.15.7
 	github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
 	github.com/klauspost/compress v1.17.7
-	github.com/milvus-io/milvus-proto/go-api/v2 v2.4.18-0.20241204065031-4466aae60fc2
+	github.com/milvus-io/milvus-proto/go-api/v2 v2.4.18-0.20241212110236-7f744ed2ad11
 	github.com/nats-io/nats-server/v2 v2.10.12
 	github.com/nats-io/nats.go v1.34.1
 	github.com/panjf2000/ants/v2 v2.7.2

diff --git a/pkg/go.sum b/pkg/go.sum
@@ -505,6 +505,8 @@ github.com/milvus-io/gorocksdb v0.0.0-20220624081344-8c5f4212846b h1:TfeY0NxYxZz
 github.com/milvus-io/gorocksdb v0.0.0-20220624081344-8c5f4212846b/go.mod h1:iwW+9cWfIzzDseEBCCeDSN5SD16Tidvy8cwQ7ZY8Qj4=
 github.com/milvus-io/milvus-proto/go-api/v2 v2.4.18-0.20241204065031-4466aae60fc2 h1:I3lqktgaXm87Lbsj1UmjSttq+APd+b6IdB2GamPB9cg=
 github.com/milvus-io/milvus-proto/go-api/v2 v2.4.18-0.20241204065031-4466aae60fc2/go.mod h1:/6UT4zZl6awVeXLeE7UGDWZvXj3IWkRsh3mqsn0DiAs=
+github.com/milvus-io/milvus-proto/go-api/v2 v2.4.18-0.20241212110236-7f744ed2ad11 h1:Mq+jtk/gSU6oMP5DPjMJHuRVBVDwxx61MKLxSCk1osg=
+github.com/milvus-io/milvus-proto/go-api/v2 v2.4.18-0.20241212110236-7f744ed2ad11/go.mod h1:/6UT4zZl6awVeXLeE7UGDWZvXj3IWkRsh3mqsn0DiAs=
 github.com/milvus-io/pulsar-client-go v0.6.10 h1:eqpJjU+/QX0iIhEo3nhOqMNXL+TyInAs1IAHZCrCM/A=
 github.com/milvus-io/pulsar-client-go v0.6.10/go.mod h1:lQqCkgwDF8YFYjKA+zOheTk1tev2B+bKj5j7+nm8M1w=
 github.com/minio/highwayhash v1.0.2 h1:Aak5U0nElisjDCfPSG79Tgzkn2gl66NxOMspRrKnA/g=

diff --git a/tests/python_client/requirements.txt b/tests/python_client/requirements.txt
@@ -12,8 +12,8 @@ allure-pytest==2.7.0
 pytest-print==0.2.1
 pytest-level==0.1.1
 pytest-xdist==2.5.0
-pymilvus==2.4.11rc8
-pymilvus[bulk_writer]==2.4.11rc8
+pymilvus==2.4.11rc11
+pymilvus[bulk_writer]==2.4.11rc11
 pytest-rerunfailures==9.1.1
 git+https://github.com/Projectplace/pytest-tags
 ndg-httpsclient