[
https://issues.apache.org/jira/browse/SCB-901?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=16608507#comment-16608507
]
ASF GitHub Bot commented on SCB-901:
------------------------------------
asifdxtreme closed pull request #439: SCB-901 Making service registration api
idempotent
URL: https://github.com/apache/incubator-servicecomb-service-center/pull/439
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/plugin/infra/registry/embededetcd/embededetcd.go
b/server/plugin/infra/registry/embededetcd/embededetcd.go
index 0f408397..c0f19499 100644
--- a/server/plugin/infra/registry/embededetcd/embededetcd.go
+++ b/server/plugin/infra/registry/embededetcd/embededetcd.go
@@ -342,9 +342,21 @@ func (s *EtcdEmbed) TxnWithCmp(ctx context.Context,
success []registry.PluginOp,
if err != nil {
return nil, err
}
+
+ var rangeResponse etcdserverpb.RangeResponse
+ for _, itf := range resp.Responses {
+ if rr, ok :=
itf.Response.(*etcdserverpb.ResponseOp_ResponseRange); ok {
+ // plz request the same type range kv in txn
success/fail options
+ rangeResponse.Kvs = append(rangeResponse.Kvs,
rr.ResponseRange.Kvs...)
+ rangeResponse.Count += rr.ResponseRange.Count
+ }
+ }
+
return ®istry.PluginResponse{
Succeeded: resp.Succeeded,
Revision: resp.Header.Revision,
+ Kvs: rangeResponse.Kvs,
+ Count: rangeResponse.Count,
}, nil
}
diff --git a/server/plugin/infra/registry/etcd/etcd.go
b/server/plugin/infra/registry/etcd/etcd.go
index b5674619..dbbfa12d 100644
--- a/server/plugin/infra/registry/etcd/etcd.go
+++ b/server/plugin/infra/registry/etcd/etcd.go
@@ -28,6 +28,7 @@ import (
mgr
"github.com/apache/incubator-servicecomb-service-center/server/plugin"
"github.com/coreos/etcd/clientv3"
"github.com/coreos/etcd/etcdserver/api/v3rpc/rpctypes"
+ "github.com/coreos/etcd/etcdserver/etcdserverpb"
"github.com/coreos/etcd/mvcc/mvccpb"
"golang.org/x/net/context"
"google.golang.org/grpc"
@@ -566,9 +567,21 @@ func (c *EtcdClient) TxnWithCmp(ctx context.Context,
success []registry.PluginOp
}
log.LogNilOrWarnf(start, "registry client txn {if(%v): %s, then: %d,
else: %d}, rev: %d",
resp.Succeeded, cmps, len(success), len(fail),
resp.Header.Revision)
+
+ var rangeResponse etcdserverpb.RangeResponse
+ for _, itf := range resp.Responses {
+ if rr, ok :=
itf.Response.(*etcdserverpb.ResponseOp_ResponseRange); ok {
+ // plz request the same type range kv in txn
success/fail options
+ rangeResponse.Kvs = append(rangeResponse.Kvs,
rr.ResponseRange.Kvs...)
+ rangeResponse.Count += rr.ResponseRange.Count
+ }
+ }
+
return ®istry.PluginResponse{
Succeeded: resp.Succeeded,
Revision: resp.Header.Revision,
+ Kvs: rangeResponse.Kvs,
+ Count: rangeResponse.Count,
}, nil
}
diff --git a/server/plugin/infra/registry/etcd/etcd_test.go
b/server/plugin/infra/registry/etcd/etcd_test.go
index 067b713a..a6c7c6a1 100644
--- a/server/plugin/infra/registry/etcd/etcd_test.go
+++ b/server/plugin/infra/registry/etcd/etcd_test.go
@@ -398,6 +398,18 @@ func TestEtcdClient_Txn(t *testing.T) {
t.Fatalf("TestEtcdClient failed, %#v", err)
}
+ // case: range request
+ resp, err = etcd.TxnWithCmp(context.Background(), nil,
[]registry.CompareOp{
+ {[]byte("/test_txn/c"), registry.CMP_VALUE, registry.CMP_EQUAL,
"c"},
+ }, []registry.PluginOp{
+ {Action: registry.Get, Key: []byte("/test_txn/a")},
+ {Action: registry.Get, Key: []byte("/test_txn/"), Prefix: true},
+ })
+ if err != nil || resp == nil || resp.Succeeded || resp.Count != 3 { //
a + [a,b]
+ t.Fatalf("TestEtcdClient failed, %#v", err)
+ }
+
+ // case: test key not exist
resp, err = etcd.TxnWithCmp(context.Background(), []registry.PluginOp{
{Action: registry.Put, Key: []byte("/test_txn/a"), Value:
[]byte("a")},
{Action: registry.Put, Key: []byte("/test_txn/b"), Value:
[]byte("b")},
diff --git a/server/service/microservice.go b/server/service/microservice.go
index 389b5b9e..d93c537b 100644
--- a/server/service/microservice.go
+++ b/server/service/microservice.go
@@ -111,11 +111,10 @@ func (s *MicroServiceService) CreateServicePri(ctx
context.Context, in *pb.Creat
}
// 产生全局service id
- serviceId := in.Service.ServiceId
- if len(serviceId) == 0 {
- serviceId = plugin.Plugins().UUID().GetServiceId()
+ requestServiceId := service.ServiceId
+ if len(requestServiceId) == 0 {
+ service.ServiceId = plugin.Plugins().UUID().GetServiceId()
}
- service.ServiceId = serviceId
service.Timestamp = strconv.FormatInt(time.Now().Unix(), 10)
service.ModTimestamp = service.Timestamp
@@ -127,7 +126,7 @@ func (s *MicroServiceService) CreateServicePri(ctx
context.Context, in *pb.Creat
Response: pb.CreateResponse(scerr.ErrInternal,
err.Error()),
}, err
}
- key := apt.GenerateServiceKey(domainProject, serviceId)
+ key := apt.GenerateServiceKey(domainProject, service.ServiceId)
keyBytes := util.StringToBytesWithNoCopy(key)
index := apt.GenerateServiceIndexKey(serviceKey)
indexBytes := util.StringToBytesWithNoCopy(index)
@@ -135,20 +134,24 @@ func (s *MicroServiceService) CreateServicePri(ctx
context.Context, in *pb.Creat
opts := []registry.PluginOp{
registry.OpPut(registry.WithKey(keyBytes),
registry.WithValue(data)),
- registry.OpPut(registry.WithKey(indexBytes),
registry.WithStrValue(serviceId)),
+ registry.OpPut(registry.WithKey(indexBytes),
registry.WithStrValue(service.ServiceId)),
}
uniqueCmpOpts := []registry.CompareOp{
registry.OpCmp(registry.CmpVer(indexBytes), registry.CMP_EQUAL,
0),
registry.OpCmp(registry.CmpVer(keyBytes), registry.CMP_EQUAL,
0),
}
+ failOpts := []registry.PluginOp{
+ registry.OpGet(registry.WithKey(indexBytes)),
+ }
if len(serviceKey.Alias) > 0 {
- opts = append(opts,
registry.OpPut(registry.WithKey(aliasBytes), registry.WithStrValue(serviceId)))
+ opts = append(opts,
registry.OpPut(registry.WithKey(aliasBytes),
registry.WithStrValue(service.ServiceId)))
uniqueCmpOpts = append(uniqueCmpOpts,
registry.OpCmp(registry.CmpVer(aliasBytes),
registry.CMP_EQUAL, 0))
+ failOpts = append(failOpts,
registry.OpGet(registry.WithKey(aliasBytes)))
}
- resp, err := backend.Registry().TxnWithCmp(ctx, opts, uniqueCmpOpts,
nil)
+ resp, err := backend.Registry().TxnWithCmp(ctx, opts, uniqueCmpOpts,
failOpts)
if err != nil {
log.Errorf(err, "create micro-service failed, %s: commit data
into etcd failed. operator: %s",
serviceFlag, remoteIP)
@@ -157,22 +160,33 @@ func (s *MicroServiceService) CreateServicePri(ctx
context.Context, in *pb.Creat
}, err
}
if !resp.Succeeded {
- if s.isCreateServiceEx(in) == true {
- serviceIdInner, _ := serviceUtil.GetServiceId(ctx,
serviceKey)
- log.Warnf("create micro-service failed, serviceId = %s
, flag = %s: service already exists. operator: %s",
- serviceIdInner, serviceFlag, remoteIP)
+ if len(requestServiceId) != 0 {
+ if len(resp.Kvs) == 0 ||
+ requestServiceId !=
util.BytesToStringWithNoCopy(resp.Kvs[0].Value) {
+ log.Warnf("create micro-service failed, %s:
service already exists. operator: %s",
+ serviceFlag, remoteIP)
+ return &pb.CreateServiceResponse{
+ Response:
pb.CreateResponse(scerr.ErrServiceAlreadyExists,
+ "ServiceId conflict or found
the same service with different id."),
+ }, nil
+ }
+ }
+ if len(resp.Kvs) == 0 {
+ // internal error?
+ log.Errorf(nil, "create micro-service failed, %s:
unexpected txn response. operator: %s",
+ serviceFlag, remoteIP)
return &pb.CreateServiceResponse{
- Response:
pb.CreateResponse(pb.Response_SUCCESS, "register service successfully"),
- ServiceId: serviceIdInner,
+ Response: pb.CreateResponse(scerr.ErrInternal,
"Unexpected txn response."),
}, nil
}
- log.Warnf("create micro-service failed, %s: service already
exists. operator: %s",
- serviceFlag, remoteIP)
-
+ serviceIdInner :=
util.BytesToStringWithNoCopy(resp.Kvs[0].Value)
+ log.Warnf("create micro-service failed, serviceId = %s , flag =
%s: service already exists. operator: %s",
+ serviceIdInner, serviceFlag, remoteIP)
return &pb.CreateServiceResponse{
- Response:
pb.CreateResponse(scerr.ErrServiceAlreadyExists, "Service already exists."),
+ Response: pb.CreateResponse(pb.Response_SUCCESS,
"register service successfully"),
+ ServiceId: serviceIdInner,
}, nil
}
@@ -184,7 +198,7 @@ func (s *MicroServiceService) CreateServicePri(ctx
context.Context, in *pb.Creat
serviceFlag, service.ServiceId, remoteIP)
return &pb.CreateServiceResponse{
Response: pb.CreateResponse(pb.Response_SUCCESS, "Register
service successfully."),
- ServiceId: serviceId,
+ ServiceId: service.ServiceId,
}, nil
}
diff --git a/server/service/microservice_test.go
b/server/service/microservice_test.go
index edbfbdf8..e9de3259 100644
--- a/server/service/microservice_test.go
+++ b/server/service/microservice_test.go
@@ -213,7 +213,8 @@ var _ = Describe("'Micro-service' service", func() {
},
})
Expect(err).To(BeNil())
-
Expect(resp.Response.Code).To(Equal(scerr.ErrServiceAlreadyExists))
+
Expect(resp.Response.Code).To(Equal(pb.Response_SUCCESS))
+ Expect(resp.ServiceId).To(Equal(sameId))
By("the same alias")
resp, err =
serviceResource.Create(getContext(), &pb.CreateServiceRequest{
@@ -230,9 +231,64 @@ var _ = Describe("'Micro-service' service", func() {
},
})
Expect(err).To(BeNil())
-
Expect(resp.Response.Code).To(Equal(scerr.ErrServiceAlreadyExists))
+
Expect(resp.Response.Code).To(Equal(pb.Response_SUCCESS))
+ Expect(resp.ServiceId).To(Equal(sameId))
+
+ By("the same serviceId and the same
serviceName")
+ resp, err =
serviceResource.Create(getContext(), &pb.CreateServiceRequest{
+ Service: &pb.MicroService{
+ ServiceId: sameId,
+ ServiceName: "some-relay",
+ Alias: "sr1",
+ AppId: "default",
+ Version: "1.0.0",
+ Level: "FRONT",
+ Schemas: []string{
+ "xxxxxxxx",
+ },
+ Status: "UP",
+ },
+ })
+ Expect(err).To(BeNil())
+
Expect(resp.Response.Code).To(Equal(pb.Response_SUCCESS))
+ Expect(resp.ServiceId).To(Equal(sameId))
+
+ By("the same serviceId and the same alias")
+ resp, err =
serviceResource.Create(getContext(), &pb.CreateServiceRequest{
+ Service: &pb.MicroService{
+ ServiceId: sameId,
+ ServiceName: "some-relay1",
+ Alias: "sr",
+ AppId: "default",
+ Version: "1.0.0",
+ Level: "FRONT",
+ Schemas: []string{
+ "xxxxxxxx",
+ },
+ Status: "UP",
+ },
+ })
+ Expect(err).To(BeNil())
+
Expect(resp.Response.Code).To(Equal(pb.Response_SUCCESS))
+ Expect(resp.ServiceId).To(Equal(sameId))
By("the same service key but with diff
serviceId")
+ resp, err =
serviceResource.Create(getContext(), &pb.CreateServiceRequest{
+ Service: &pb.MicroService{
+ ServiceId: "customId",
+ ServiceName: "some-relay",
+ Alias: "sr1",
+ AppId: "default",
+ Version: "1.0.0",
+ Level: "FRONT",
+ Schemas: []string{
+ "xxxxxxxx",
+ },
+ Status: "UP",
+ },
+ })
+ Expect(err).To(BeNil())
+
Expect(resp.Response.Code).To(Equal(scerr.ErrServiceAlreadyExists))
resp, err =
serviceResource.Create(getContext(), &pb.CreateServiceRequest{
Service: &pb.MicroService{
ServiceId: "customId",
@@ -250,12 +306,12 @@ var _ = Describe("'Micro-service' service", func() {
Expect(err).To(BeNil())
Expect(resp.Response.Code).To(Equal(scerr.ErrServiceAlreadyExists))
- By("the same serviceId")
+ By("the same service id but with diff key")
resp, err =
serviceResource.Create(getContext(), &pb.CreateServiceRequest{
Service: &pb.MicroService{
ServiceId: sameId,
- ServiceName: "some-relay1",
- Alias: "sr",
+ ServiceName: "some-relay2",
+ Alias: "sr2",
AppId: "default",
Version: "1.0.0",
Level: "FRONT",
----------------------------------------------------------------
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:
[email protected]
> Making service registration api idempotent
> ------------------------------------------
>
> Key: SCB-901
> URL: https://issues.apache.org/jira/browse/SCB-901
> Project: Apache ServiceComb
> Issue Type: Improvement
> Components: Service-Center
> Reporter: little-cui
> Assignee: little-cui
> Priority: Major
> Fix For: service-center-1.1.0
>
>
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
