enhance: alterdatabase support delete property (#38380)

alterdatabase support delete property issue: https://github.com/milvus-io/milvus/issues/38379 --------- Signed-off-by: Xianhui.Lin <xianhui.lin@zilliz.com>
2024-12-12 17:04:43 +08:00 · 2024-12-12 17:04:43 +08:00 · d0a8110a7a
parent 59234a3350
commit d0a8110a7a
11 changed files with 96 additions and 22 deletions
--- a/go.mod
+++ b/go.mod
@ -18,12 +18,12 @@ require (
 	github.com/gin-gonic/gin v1.9.1
 	github.com/go-playground/validator/v10 v10.14.0
 	github.com/gofrs/flock v0.8.1
-	github.com/golang/protobuf v1.5.4
+	github.com/golang/protobuf v1.5.4 // indirect
 	github.com/google/btree v1.1.2
 	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.5.0-beta.0.20241204065646-180ce3a8d277
+	github.com/milvus-io/milvus-proto/go-api/v2 v2.5.0-beta.0.20241211060635-410431d7865b
 	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
--- a/go.sum
+++ b/go.sum
@ -630,10 +630,8 @@ github.com/milvus-io/cgosymbolizer v0.0.0-20240722103217-b7dee0e50119 h1:9VXijWu
 github.com/milvus-io/cgosymbolizer v0.0.0-20240722103217-b7dee0e50119/go.mod h1:DvXTE/K/RtHehxU8/GtDs4vFtfw64jJ3PaCnFri8CRg=
 github.com/milvus-io/gorocksdb v0.0.0-20220624081344-8c5f4212846b h1:TfeY0NxYxZzUfIfYe5qYDBzt4ZYRqzUjTR6CvUzjat8=
 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.5.0-beta.0.20241202112430-822be0295910 h1:cFRrdFZwhFHv33pue1z8beYSvrXDYFSFsCuvXGX3DHE=
-github.com/milvus-io/milvus-proto/go-api/v2 v2.5.0-beta.0.20241202112430-822be0295910/go.mod h1:/6UT4zZl6awVeXLeE7UGDWZvXj3IWkRsh3mqsn0DiAs=
-github.com/milvus-io/milvus-proto/go-api/v2 v2.5.0-beta.0.20241204065646-180ce3a8d277 h1:5/35+F32fs6ifVzI1e+VkUNpK0gWyXQSdZVnmNUFrrg=
-github.com/milvus-io/milvus-proto/go-api/v2 v2.5.0-beta.0.20241204065646-180ce3a8d277/go.mod h1:/6UT4zZl6awVeXLeE7UGDWZvXj3IWkRsh3mqsn0DiAs=
+github.com/milvus-io/milvus-proto/go-api/v2 v2.5.0-beta.0.20241211060635-410431d7865b h1:iPPhnFx+s7FF53UeWj7A4EYhPRMFPL6mHqyQw7qRjeQ=
+github.com/milvus-io/milvus-proto/go-api/v2 v2.5.0-beta.0.20241211060635-410431d7865b/go.mod h1:/6UT4zZl6awVeXLeE7UGDWZvXj3IWkRsh3mqsn0DiAs=
 github.com/milvus-io/pulsar-client-go v0.12.1 h1:O2JZp1tsYiO7C0MQ4hrUY/aJXnn2Gry6hpm7UodghmE=
 github.com/milvus-io/pulsar-client-go v0.12.1/go.mod h1:dkutuH4oS2pXiGm+Ti7fQZ4MRjrMPZ8IJeEGAWMeckk=
 github.com/minio/asm2plan9s v0.0.0-20200509001527-cdd76441f9d8 h1:AMFGa4R4MiIpspGNG7Z948v4n35fFGB3RR3G/ry4FWs=
--- a/internal/proto/root_coord.proto
+++ b/internal/proto/root_coord.proto
@ -225,6 +225,7 @@ message AlterDatabaseRequest {
  string db_name = 2;
  string db_id = 3;
  repeated common.KeyValuePair properties = 4;
+  repeated string delete_keys = 5;
 }

 message GetPChannelInfoRequest {
--- 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)
--- 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 TestDescribeDatabaseTask(t *testing.T) {
--- a/internal/rootcoord/alter_collection_task.go
+++ b/internal/rootcoord/alter_collection_task.go
@ -154,7 +154,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
 }

--- 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)
--- 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)
+	})
 }
--- a/internal/rootcoord/quota_center.go
+++ b/internal/rootcoord/quota_center.go
@ -711,8 +711,14 @@ func (q *QuotaCenter) getDenyReadingDBs() map[int64]struct{} {
 	for _, dbID := range lo.Uniq(q.collectionIDToDBID.Values()) {
 		if db, err := q.meta.GetDatabaseByID(q.ctx, dbID, typeutil.MaxTimestamp); err == nil {
 			if v := db.GetProperty(common.DatabaseForceDenyReadingKey); v != "" {
-				if dbForceDenyReadingEnabled, _ := strconv.ParseBool(v); dbForceDenyReadingEnabled {
-					dbIDs[dbID] = struct{}{}
+				if dbForceDenyReadingEnabled, err := strconv.ParseBool(v); err == nil {
+					if dbForceDenyReadingEnabled {
+						dbIDs[dbID] = struct{}{}
+					}
+				} else {
+					log.Warn("invalid configuration for database force deny reading",
+						zap.String("config item", common.DatabaseForceDenyReadingKey),
+						zap.String("config value", v))
 				}
 			}
 		}
@ -740,8 +746,14 @@ func (q *QuotaCenter) getDenyWritingDBs() map[int64]struct{} {
 	for _, dbID := range lo.Uniq(q.collectionIDToDBID.Values()) {
 		if db, err := q.meta.GetDatabaseByID(q.ctx, dbID, typeutil.MaxTimestamp); err == nil {
 			if v := db.GetProperty(common.DatabaseForceDenyWritingKey); v != "" {
-				if dbForceDenyWritingEnabled, _ := strconv.ParseBool(v); dbForceDenyWritingEnabled {
-					dbIDs[dbID] = struct{}{}
+				if dbForceDenyWritingEnabled, err := strconv.ParseBool(v); err == nil {
+					if dbForceDenyWritingEnabled {
+						dbIDs[dbID] = struct{}{}
+					}
+				} else {
+					log.Warn("invalid configuration for database force deny writing",
+						zap.String("config item", common.DatabaseForceDenyWritingKey),
+						zap.String("config value", v))
 				}
 			}
 		}
@ -1341,6 +1353,10 @@ func (q *QuotaCenter) checkDBDiskQuota(dbSizeInfo map[int64]int64) []int64 {
 					dbDiskQuotaMB := dbDiskQuotaBytes * 1024 * 1024
 					checkDiskQuota(dbID, binlogSize, dbDiskQuotaMB)
 					continue
+				} else {
+					log.Warn("invalid configuration for diskQuota.mb",
+						zap.String("config item", common.DatabaseDiskQuotaKey),
+						zap.String("config value", dbDiskQuotaStr))
 				}
 			}
 		}
--- a/pkg/go.mod
+++ b/pkg/go.mod
@ -14,7 +14,7 @@ require (
 	github.com/grpc-ecosystem/go-grpc-middleware v1.3.0
 	github.com/json-iterator/go v1.1.12
 	github.com/klauspost/compress v1.17.7
-	github.com/milvus-io/milvus-proto/go-api/v2 v2.5.0-beta.0.20241204065646-180ce3a8d277
+	github.com/milvus-io/milvus-proto/go-api/v2 v2.5.0-beta.0.20241211060635-410431d7865b
 	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
--- a/pkg/go.sum
+++ b/pkg/go.sum
@ -490,6 +490,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.5.0-beta.0.20241204065646-180ce3a8d277 h1:5/35+F32fs6ifVzI1e+VkUNpK0gWyXQSdZVnmNUFrrg=
 github.com/milvus-io/milvus-proto/go-api/v2 v2.5.0-beta.0.20241204065646-180ce3a8d277/go.mod h1:/6UT4zZl6awVeXLeE7UGDWZvXj3IWkRsh3mqsn0DiAs=
+github.com/milvus-io/milvus-proto/go-api/v2 v2.5.0-beta.0.20241211060635-410431d7865b h1:iPPhnFx+s7FF53UeWj7A4EYhPRMFPL6mHqyQw7qRjeQ=
+github.com/milvus-io/milvus-proto/go-api/v2 v2.5.0-beta.0.20241211060635-410431d7865b/go.mod h1:/6UT4zZl6awVeXLeE7UGDWZvXj3IWkRsh3mqsn0DiAs=
 github.com/milvus-io/pulsar-client-go v0.12.1 h1:O2JZp1tsYiO7C0MQ4hrUY/aJXnn2Gry6hpm7UodghmE=
 github.com/milvus-io/pulsar-client-go v0.12.1/go.mod h1:dkutuH4oS2pXiGm+Ti7fQZ4MRjrMPZ8IJeEGAWMeckk=
 github.com/minio/highwayhash v1.0.2 h1:Aak5U0nElisjDCfPSG79Tgzkn2gl66NxOMspRrKnA/g=