You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by GitBox <gi...@apache.org> on 2021/11/25 02:06:17 UTC

[GitHub] [servicecomb-kie] little-cui commented on a change in pull request #228: [Sync task] adding, deleting and modifying operations to add tasks when synchronization is turned on

little-cui commented on a change in pull request #228:
URL: https://github.com/apache/servicecomb-kie/pull/228#discussion_r756507891



##########
File path: pkg/model/db_schema.go
##########
@@ -48,6 +48,25 @@ type KVDoc struct {
 	Domain string            `json:"domain,omitempty" yaml:"domain,omitempty" validate:"min=1,max=256,commonName"`              //redundant
 }
 
+// Task is db struct to store sync task
+type Task struct {
+	TaskID    string      `json:"task_id" bson:"task_id"`
+	Action    string      `json:"action" bson:"action"`
+	DataType  string      `json:"data_type" bson:"data_type"`
+	Data      interface{} `json:"data" bson:"data"`
+	Timestamp int64       `json:"timestamp" bson:"timestamp"`
+	Status    string      `json:"status" bson:"status"`
+}
+
+// Tombstone is db struct to store the deleted resource information
+type Tombstone struct {

Review comment:
       Task和Tombstone都是公共类型,可以放到CARI仓库

##########
File path: .github/workflows/mongo_storage.yml
##########
@@ -13,11 +13,8 @@ jobs:
       uses: actions/checkout@v1
     - name: UT
       run: |
-        cd build
-        time bash build_docker.sh
-        cd ../
-        sudo docker-compose -f ./deployments/docker/docker-compose.yaml up -d
-        sleep 20
+        sudo docker-compose -f ./deployments/docker/mongo-compose.yaml up -d
+        sleep 15
         export TEST_DB_KIND=mongo
-        export TEST_DB_URI=mongodb://kie:123@127.0.0.1:27017/kie
+        export TEST_DB_URI=mongodb://kie:123@127.0.0.1:30001/kie

Review comment:
       修改端口的原因是什么?

##########
File path: server/datasource/etcd/kv/kv_dao.go
##########
@@ -59,6 +61,100 @@ func (s *Dao) Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
 	return kv, nil
 }
 
+// CreateWithTask is used to create a task when the configuration is created
+func (s *Dao) CreateWithTask(ctx context.Context, kv *model.KVDoc, task *model.Task) (*model.KVDoc, error) {
+	kvBytes, err := json.Marshal(kv)
+	if err != nil {
+		openlog.Error("fail to marshal kv")
+		return nil, err
+	}
+	taskBytes, err := json.Marshal(task)
+	if err != nil {
+		openlog.Error("fail to marshal task ")
+		return nil, err
+	}
+	kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.KV(kv.Domain, kv.Project, kv.ID)), etcdadpt.WithValue(kvBytes))
+	taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, strconv.FormatInt(task.Timestamp, 10), task.TaskID)), etcdadpt.WithValue(taskBytes))
+	resp, err := etcdadpt.Instance().TxnWithCmp(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut}, nil, nil)
+	if err != nil {
+		openlog.Error("create error", openlog.WithTags(openlog.Tags{
+			"err":  err.Error(),
+			"kv":   kv,
+			"task": task,
+		}))
+		return nil, err
+	}
+	if !resp.Succeeded {

Review comment:
       没有cmp语义,这个条件不存在,其他地方一并修改

##########
File path: server/datasource/etcd/kv/kv_dao.go
##########
@@ -59,6 +61,100 @@ func (s *Dao) Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
 	return kv, nil
 }
 
+// CreateWithTask is used to create a task when the configuration is created
+func (s *Dao) CreateWithTask(ctx context.Context, kv *model.KVDoc, task *model.Task) (*model.KVDoc, error) {
+	kvBytes, err := json.Marshal(kv)
+	if err != nil {
+		openlog.Error("fail to marshal kv")
+		return nil, err
+	}
+	taskBytes, err := json.Marshal(task)
+	if err != nil {
+		openlog.Error("fail to marshal task ")
+		return nil, err
+	}
+	kvOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.KV(kv.Domain, kv.Project, kv.ID)), etcdadpt.WithValue(kvBytes))
+	taskOpPut := etcdadpt.OpPut(etcdadpt.WithStrKey(key.TaskKey(kv.Domain, kv.Project, strconv.FormatInt(task.Timestamp, 10), task.TaskID)), etcdadpt.WithValue(taskBytes))
+	resp, err := etcdadpt.Instance().TxnWithCmp(ctx, []etcdadpt.OpOptions{kvOpPut, taskOpPut}, nil, nil)

Review comment:
       使用etcdadpt.Txn()代替,其他地方一并修改

##########
File path: server/datasource/etcd/key/key.go
##########
@@ -28,8 +28,27 @@ const (
 	keyCounter = "counter"
 	keyHistory = "kv-history"
 	keyTrack   = "track"
+	syncer     = "syncer"
+	task       = "task"
+	tombstone  = "tombstone"
 )
 
+func getSyncRootKey() string {
+	return split + syncer + split + task
+}
+
+func getTombstoneRootKey() string {
+	return split + tombstone
+}
+
+func TaskKey(domain, project, timestamp, taskID string) string {

Review comment:
       timestamp定义为int64,使用更方便

##########
File path: server/datasource/dao.go
##########
@@ -73,10 +74,20 @@ type KVDao interface {
 	Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
 	Update(ctx context.Context, kv *model.KVDoc) error
 	List(ctx context.Context, project, domain string, options ...FindOption) (*model.KVResponse, error)
+	// CreateWithTask will create the task when the configuration is created
+	CreateWithTask(ctx context.Context, kv *model.KVDoc, task *model.Task) (*model.KVDoc, error)
+	// UpdateWithTask will create the task when the configuration is updated
+	UpdateWithTask(ctx context.Context, kv *model.KVDoc, task *model.Task) error
+
 	//FindOneAndDelete deletes one kv by id and return the deleted kv as these appeared before deletion
 	FindOneAndDelete(ctx context.Context, kvID string, project, domain string) (*model.KVDoc, error)
 	//FindManyAndDelete deletes multiple kvs and return the deleted kv list as these appeared before deletion
 	FindManyAndDelete(ctx context.Context, kvIDs []string, project, domain string) ([]*model.KVDoc, int64, error)
+	// FindOneAndDeleteWithTask will create the task when the configuration is deleted
+	FindOneAndDeleteWithTask(ctx context.Context, kvID string, project, domain string, task *model.Task, tombstone *model.Tombstone) (*model.KVDoc, error)
+	// FindManyAndDeleteWithTask will create tasks when many configurations are deleted
+	FindManyAndDeleteWithTask(ctx context.Context, kvIDs []string, project, domain string, tasks []*model.Task, tombstones []*model.Tombstone) ([]*model.KVDoc, int64, error)

Review comment:
       task, tombstone不需要外部传递,只要是事务写逻辑,都应该在DAO层内生成task, tombstone

##########
File path: server/datasource/dao.go
##########
@@ -73,10 +74,20 @@ type KVDao interface {
 	Create(ctx context.Context, kv *model.KVDoc) (*model.KVDoc, error)
 	Update(ctx context.Context, kv *model.KVDoc) error
 	List(ctx context.Context, project, domain string, options ...FindOption) (*model.KVResponse, error)
+	// CreateWithTask will create the task when the configuration is created
+	CreateWithTask(ctx context.Context, kv *model.KVDoc, task *model.Task) (*model.KVDoc, error)
+	// UpdateWithTask will create the task when the configuration is updated
+	UpdateWithTask(ctx context.Context, kv *model.KVDoc, task *model.Task) error

Review comment:
       使用 options pattern扩展原有接口,如:WithTransation(True),不建议增加新接口

##########
File path: server/service/kv/kv_svc.go
##########
@@ -250,9 +312,26 @@ func Update(ctx context.Context, kv *model.UpdateKVRequest) (*model.KVDoc, error
 }
 
 func FindOneAndDelete(ctx context.Context, kvID string, project, domain string) (*model.KVDoc, error) {
-	kv, err := datasource.GetBroker().GetKVDao().FindOneAndDelete(ctx, kvID, project, domain)
-	if err != nil {
-		return nil, err
+	var kv *model.KVDoc
+	var err error
+	if cfg.GetSync().Enabled {

Review comment:
       用了options pattern之后就消除了if




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@servicecomb.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org