[GitHub] asifdxtreme closed pull request #446: SCB-929 Concurrent error in update resource APIs

[GitBox <gi...@apache.org>]

asifdxtreme closed pull request #446: SCB-929 Concurrent error in update resource APIs
URL: https://github.com/apache/incubator-servicecomb-service-center/pull/446
 
 
   

This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:

As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):

diff --git a/server/service/instance.go b/server/service/instance.go
index f33fe486..d147cf58 100644
--- a/server/service/instance.go
+++ b/server/service/instance.go
@@ -653,9 +653,10 @@ func (s *InstanceService) UpdateStatus(ctx context.Context, in *pb.UpdateInstanc
 		}, nil
 	}
 
-	instance.Status = in.Status
+	copyInstanceRef := *instance
+	copyInstanceRef.Status = in.Status
 
-	if err := serviceUtil.UpdateInstance(ctx, domainProject, instance); err != nil {
+	if err := serviceUtil.UpdateInstance(ctx, domainProject, &copyInstanceRef); err != nil {
 		log.Errorf(err, "update instance status failed, %s", updateStatusFlag)
 		resp := &pb.UpdateInstanceStatusResponse{
 			Response: pb.CreateResponseWithSCErr(err),
@@ -696,9 +697,10 @@ func (s *InstanceService) UpdateInstanceProperties(ctx context.Context, in *pb.U
 		}, nil
 	}
 
-	instance.Properties = in.Properties
+	copyInstanceRef := *instance
+	copyInstanceRef.Properties = in.Properties
 
-	if err := serviceUtil.UpdateInstance(ctx, domainProject, instance); err != nil {
+	if err := serviceUtil.UpdateInstance(ctx, domainProject, &copyInstanceRef); err != nil {
 		log.Errorf(err, "update instance properties failed, %s", instanceFlag)
 		resp := &pb.UpdateInstancePropsResponse{
 			Response: pb.CreateResponseWithSCErr(err),
diff --git a/server/service/microservice.go b/server/service/microservice.go
index d93c537b..cbde70fc 100644
--- a/server/service/microservice.go
+++ b/server/service/microservice.go
@@ -518,13 +518,12 @@ func (s *MicroServiceService) UpdateProperties(ctx context.Context, in *pb.Updat
 			Response: pb.CreateResponse(scerr.ErrServiceNotExists, "service does not exist."),
 		}, nil
 	}
-	service.Properties = make(map[string]string)
-	for propertyKey := range in.Properties {
-		service.Properties[propertyKey] = in.Properties[propertyKey]
-	}
-	service.ModTimestamp = strconv.FormatInt(time.Now().Unix(), 10)
 
-	data, err := json.Marshal(service)
+	copyServiceRef := *service
+	copyServiceRef.Properties = in.Properties
+	copyServiceRef.ModTimestamp = strconv.FormatInt(time.Now().Unix(), 10)
+
+	data, err := json.Marshal(copyServiceRef)
 	if err != nil {
 		log.Errorf(err, "update service properties failed, serviceId is %s: json marshal service failed.", in.ServiceId)
 		return &pb.UpdateServicePropsResponse{
diff --git a/server/service/rule.go b/server/service/rule.go
index 40ca4bdc..ad3efc70 100644
--- a/server/service/rule.go
+++ b/server/service/rule.go
@@ -199,23 +199,24 @@ func (s *MicroServiceService) UpdateRule(ctx context.Context, in *pb.UpdateServi
 		}, nil
 	}
 
-	oldRulePatten := rule.Pattern
-	oldRuleAttr := rule.Attribute
+	copyRuleRef := *rule
+	oldRulePatten := copyRuleRef.Pattern
+	oldRuleAttr := copyRuleRef.Attribute
 	isChangeIndex := false
-	if rule.Attribute != in.GetRule().Attribute {
+	if copyRuleRef.Attribute != in.GetRule().Attribute {
 		isChangeIndex = true
-		rule.Attribute = in.GetRule().Attribute
+		copyRuleRef.Attribute = in.GetRule().Attribute
 	}
-	if rule.Pattern != in.GetRule().Pattern {
+	if copyRuleRef.Pattern != in.GetRule().Pattern {
 		isChangeIndex = true
-		rule.Pattern = in.GetRule().Pattern
+		copyRuleRef.Pattern = in.GetRule().Pattern
 	}
-	rule.RuleType = in.GetRule().RuleType
-	rule.Description = in.GetRule().Description
-	rule.ModTimestamp = strconv.FormatInt(time.Now().Unix(), 10)
+	copyRuleRef.RuleType = in.GetRule().RuleType
+	copyRuleRef.Description = in.GetRule().Description
+	copyRuleRef.ModTimestamp = strconv.FormatInt(time.Now().Unix(), 10)
 
 	key := apt.GenerateServiceRuleKey(domainProject, in.ServiceId, in.RuleId)
-	data, err := json.Marshal(rule)
+	data, err := json.Marshal(copyRuleRef)
 	if err != nil {
 		log.Errorf(err, "update rule failed, serviceId is %s, ruleId is %s: marshal service rule failed.", in.ServiceId, in.RuleId)
 		return &pb.UpdateServiceRuleResponse{
@@ -225,8 +226,8 @@ func (s *MicroServiceService) UpdateRule(ctx context.Context, in *pb.UpdateServi
 	opts := []registry.PluginOp{}
 	if isChangeIndex {
 		//加入新的rule index
-		indexKey := apt.GenerateRuleIndexKey(domainProject, in.ServiceId, rule.Attribute, rule.Pattern)
-		opts = append(opts, registry.OpPut(registry.WithStrKey(indexKey), registry.WithStrValue(rule.RuleId)))
+		indexKey := apt.GenerateRuleIndexKey(domainProject, in.ServiceId, copyRuleRef.Attribute, copyRuleRef.Pattern)
+		opts = append(opts, registry.OpPut(registry.WithStrKey(indexKey), registry.WithStrValue(copyRuleRef.RuleId)))
 
 		//删除旧的rule index
 		oldIndexKey := apt.GenerateRuleIndexKey(domainProject, in.ServiceId, oldRuleAttr, oldRulePatten)
diff --git a/server/service/tag.go b/server/service/tag.go
index aafaf82b..d07a1dbd 100644
--- a/server/service/tag.go
+++ b/server/service/tag.go
@@ -71,13 +71,13 @@ func (s *MicroServiceService) AddTags(ctx context.Context, in *pb.AddServiceTags
 			Response: pb.CreateResponse(scerr.ErrInternal, err.Error()),
 		}, err
 	}
-	if len(dataTags) > 0 {
-		for key, value := range addTags {
-			dataTags[key] = value
+	for key, value := range dataTags {
+		if _, ok := addTags[key]; ok {
+			continue
 		}
-	} else {
-		dataTags = addTags
+		addTags[key] = value
 	}
+	dataTags = addTags
 
 	checkErr := serviceUtil.AddTagIntoETCD(ctx, domainProject, in.ServiceId, dataTags)
 	if checkErr != nil {
@@ -130,9 +130,14 @@ func (s *MicroServiceService) UpdateTag(ctx context.Context, in *pb.UpdateServic
 			Response: pb.CreateResponse(scerr.ErrTagNotExists, "Update tag for service failed for update tags not exist, please add first."),
 		}, nil
 	}
-	tags[in.Key] = in.Value
 
-	checkErr := serviceUtil.AddTagIntoETCD(ctx, domainProject, in.ServiceId, tags)
+	copyTags := make(map[string]string, len(tags))
+	for k, v := range tags {
+		copyTags[k] = v
+	}
+	copyTags[in.Key] = in.Value
+
+	checkErr := serviceUtil.AddTagIntoETCD(ctx, domainProject, in.ServiceId, copyTags)
 	if checkErr != nil {
 		log.Errorf(checkErr, "update service tag failed, serviceId %s, tag %s.", in.ServiceId, tagFlag)
 		resp := &pb.UpdateServiceTagResponse{
@@ -175,18 +180,23 @@ func (s *MicroServiceService) DeleteTags(ctx context.Context, in *pb.DeleteServi
 			Response: pb.CreateResponse(scerr.ErrInternal, err.Error()),
 		}, err
 	}
+
+	copyTags := make(map[string]string, len(tags))
+	for k, v := range tags {
+		copyTags[k] = v
+	}
 	for _, key := range in.Keys {
-		if _, ok := tags[key]; !ok {
+		if _, ok := copyTags[key]; !ok {
 			log.Errorf(nil, "delete service tags failed, serviceId %s, tags %v: tag %s not exist.", in.ServiceId, in.Keys, key)
 			return &pb.DeleteServiceTagsResponse{
 				Response: pb.CreateResponse(scerr.ErrTagNotExists, "Delete tags failed for this key "+key+" does not exist."),
 			}, nil
 		}
-		delete(tags, key)
+		delete(copyTags, key)
 	}
 
 	// tags 可能size == 0
-	data, err := json.Marshal(tags)
+	data, err := json.Marshal(copyTags)
 	if err != nil {
 		log.Errorf(err, "delete service tags failed, serviceId %s, tags %v: marshall service tag failed.", in.ServiceId, in.Keys)
 		return &pb.DeleteServiceTagsResponse{


 

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services