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 2018/12/18 08:04:15 UTC
[GitHub] little-cui closed pull request #514: SCB-1059 Re-register instance
does not keep alive the lease
little-cui closed pull request #514: SCB-1059 Re-register instance does not keep alive the lease
URL: https://github.com/apache/servicecomb-service-center/pull/514
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/core/backend/discovery.go b/server/core/backend/discovery.go
index 9b480ff8..b58557b1 100644
--- a/server/core/backend/discovery.go
+++ b/server/core/backend/discovery.go
@@ -175,6 +175,8 @@ func (s *KvStore) DependencyQueue() discovery.Adaptor { return s.Adapt
func (s *KvStore) Domain() discovery.Adaptor { return s.Adaptors(DOMAIN) }
func (s *KvStore) Project() discovery.Adaptor { return s.Adaptors(PROJECT) }
+// KeepAlive will always return ok when registry is unavailable
+// unless the registry response is LeaseNotFound
func (s *KvStore) KeepAlive(ctx context.Context, opts ...registry.PluginOpOption) (int64, error) {
op := registry.OpPut(opts...)
diff --git a/server/service/instance.go b/server/service/instance.go
index a630958f..6f939bea 100644
--- a/server/service/instance.go
+++ b/server/service/instance.go
@@ -20,6 +20,7 @@ import (
"encoding/json"
"errors"
"fmt"
+ errorsEx "github.com/apache/servicecomb-service-center/pkg/errors"
"github.com/apache/servicecomb-service-center/pkg/gopool"
"github.com/apache/servicecomb-service-center/pkg/log"
"github.com/apache/servicecomb-service-center/pkg/util"
@@ -99,24 +100,26 @@ func (s *InstanceService) Register(ctx context.Context, in *pb.RegisterInstanceR
instance := in.GetInstance()
//允许自定义id
- //如果没填写 并且endpoints沒重復，則产生新的全局instance id
- oldInstanceId, checkErr := serviceUtil.InstanceExist(ctx, in.Instance)
- if checkErr != nil {
- log.Errorf(checkErr, "service[%s]'s instance existence check failed, endpoints %v, host '%s', operator %s",
- instance.ServiceId, instance.Endpoints, instance.HostName, remoteIP)
- resp := pb.CreateResponseWithSCErr(checkErr)
- if checkErr.InternalError() {
- return &pb.RegisterInstanceResponse{Response: resp}, checkErr
+ if len(instance.InstanceId) > 0 {
+ // keep alive the lease ttl
+ resp, err := s.Heartbeat(ctx, &pb.HeartbeatRequest{ServiceId: instance.ServiceId, InstanceId: instance.InstanceId})
+ switch resp.Response.Code {
+ case pb.Response_SUCCESS:
+ log.Infof("register instance successful, reuse instance[%s/%s], operator %s",
+ instance.ServiceId, instance.InstanceId, remoteIP)
+ return &pb.RegisterInstanceResponse{
+ Response: resp.GetResponse(),
+ InstanceId: instance.InstanceId,
+ }, nil
+ case scerr.ErrInstanceNotExists:
+ // register a new one
+ default:
+ log.Errorf(err, "register instance failed, reuse instance[%s/%s], operator %s",
+ instance.ServiceId, instance.InstanceId, remoteIP)
+ return &pb.RegisterInstanceResponse{
+ Response: resp.GetResponse(),
+ }, err
}
- return &pb.RegisterInstanceResponse{Response: resp}, nil
- }
- if len(oldInstanceId) > 0 {
- log.Infof("register instance successful, reuse instance[%s/%s], operator %s",
- instance.ServiceId, oldInstanceId, remoteIP)
- return &pb.RegisterInstanceResponse{
- Response: pb.CreateResponse(pb.Response_SUCCESS, "instance already exists"),
- InstanceId: oldInstanceId,
- }, nil
}
if err := s.preProcessRegisterInstance(ctx, instance); err != nil {
@@ -236,31 +239,16 @@ func (s *InstanceService) Unregister(ctx context.Context, in *pb.UnregisterInsta
instanceFlag := util.StringJoin([]string{serviceId, instanceId}, "/")
- isExist, err := serviceUtil.InstanceExistById(ctx, domainProject, serviceId, instanceId)
- if err != nil {
- log.Errorf(err, "unregister instance failed, instance[%s], operator %s: query instance failed", instanceFlag, remoteIP)
- return &pb.UnregisterInstanceResponse{
- Response: pb.CreateResponse(scerr.ErrInternal, err.Error()),
- }, err
- }
- if !isExist {
- log.Errorf(nil, "unregister instance failed, instance[%s], operator %s: instance not exist", instanceFlag, remoteIP)
- return &pb.UnregisterInstanceResponse{
- Response: pb.CreateResponse(scerr.ErrInstanceNotExists, "Service instance does not exist."),
- }, nil
- }
-
- err, isInnerErr := revokeInstance(ctx, domainProject, serviceId, instanceId)
+ err := revokeInstance(ctx, domainProject, serviceId, instanceId)
if err != nil {
- log.Errorf(nil, "unregister instance failed, instance[%s], operator %s: revoke instance failed", instanceFlag, remoteIP)
- if isInnerErr {
- return &pb.UnregisterInstanceResponse{
- Response: pb.CreateResponse(scerr.ErrUnavailableBackend, err.Error()),
- }, err
+ log.Errorf(err, "unregister instance failed, instance[%s], operator %s: revoke instance failed", instanceFlag, remoteIP)
+ resp := &pb.UnregisterInstanceResponse{
+ Response: pb.CreateResponseWithSCErr(err),
}
- return &pb.UnregisterInstanceResponse{
- Response: pb.CreateResponse(scerr.ErrInstanceNotExists, err.Error()),
- }, nil
+ if err.InternalError() {
+ return resp, err
+ }
+ return resp, nil
}
log.Infof("unregister instance[%s], operator %s", instanceFlag, remoteIP)
@@ -269,20 +257,23 @@ func (s *InstanceService) Unregister(ctx context.Context, in *pb.UnregisterInsta
}, nil
}
-func revokeInstance(ctx context.Context, domainProject string, serviceId string, instanceId string) (error, bool) {
+func revokeInstance(ctx context.Context, domainProject string, serviceId string, instanceId string) *scerr.Error {
leaseID, err := serviceUtil.GetLeaseId(ctx, domainProject, serviceId, instanceId)
if err != nil {
- return err, true
+ return scerr.NewError(scerr.ErrUnavailableBackend, err.Error())
}
if leaseID == -1 {
- return errors.New("instance's leaseId not exist."), false
+ return scerr.NewError(scerr.ErrInstanceNotExists, "Instance's leaseId not exist.")
}
err = backend.Registry().LeaseRevoke(ctx, leaseID)
if err != nil {
- return err, true
+ if _, ok := err.(errorsEx.InternalError); !ok {
+ return scerr.NewError(scerr.ErrInstanceNotExists, err.Error())
+ }
+ return scerr.NewError(scerr.ErrUnavailableBackend, err.Error())
}
- return nil, false
+ return nil
}
func (s *InstanceService) Heartbeat(ctx context.Context, in *pb.HeartbeatRequest) (*pb.HeartbeatResponse, error) {
@@ -298,18 +289,17 @@ func (s *InstanceService) Heartbeat(ctx context.Context, in *pb.HeartbeatRequest
domainProject := util.ParseDomainProject(ctx)
instanceFlag := util.StringJoin([]string{in.ServiceId, in.InstanceId}, "/")
- _, ttl, err, isInnerErr := serviceUtil.HeartbeatUtil(ctx, domainProject, in.ServiceId, in.InstanceId)
+ _, ttl, err := serviceUtil.HeartbeatUtil(ctx, domainProject, in.ServiceId, in.InstanceId)
if err != nil {
- log.Errorf(err, "heartbeat failed, instance[%s], internal error '%v'. operator %s",
- instanceFlag, isInnerErr, remoteIP)
- if isInnerErr {
- return &pb.HeartbeatResponse{
- Response: pb.CreateResponse(scerr.ErrInternal, err.Error()),
- }, err
+ log.Errorf(err, "heartbeat failed, instance[%s]. operator %s",
+ instanceFlag, remoteIP)
+ resp := &pb.HeartbeatResponse{
+ Response: pb.CreateResponseWithSCErr(err),
}
- return &pb.HeartbeatResponse{
- Response: pb.CreateResponse(scerr.ErrInstanceNotExists, "Service instance does not exist."),
- }, nil
+ if err.InternalError() {
+ return resp, err
+ }
+ return resp, nil
}
if ttl == 0 {
@@ -384,7 +374,7 @@ func getHeartbeatFunc(ctx context.Context, domainProject string, instancesHbRst
InstanceId: element.InstanceId,
ErrMessage: "",
}
- _, _, err, _ := serviceUtil.HeartbeatUtil(ctx, domainProject, element.ServiceId, element.InstanceId)
+ _, _, err := serviceUtil.HeartbeatUtil(ctx, domainProject, element.ServiceId, element.InstanceId)
if err != nil {
hbRst.ErrMessage = err.Error()
log.Errorf(err, "heartbeat set failed, %s/%s", element.ServiceId, element.InstanceId)
diff --git a/server/service/instance_test.go b/server/service/instance_test.go
index 4359c36b..df493b68 100644
--- a/server/service/instance_test.go
+++ b/server/service/instance_test.go
@@ -86,6 +86,21 @@ var _ = Describe("'Instance' service", func() {
Expect(resp.Response.Code).To(Equal(pb.Response_SUCCESS))
Expect(resp.InstanceId).To(Not(Equal("")))
+ resp, err = instanceResource.Register(getContext(), &pb.RegisterInstanceRequest{
+ Instance: &pb.MicroServiceInstance{
+ InstanceId: "customId",
+ ServiceId: serviceId1,
+ Endpoints: []string{
+ "createInstance:127.0.0.1:8080",
+ },
+ HostName: "UT-HOST",
+ Status: pb.MSI_UP,
+ },
+ })
+ Expect(err).To(BeNil())
+ Expect(resp.Response.Code).To(Equal(pb.Response_SUCCESS))
+ Expect(resp.InstanceId).To(Equal("customId"))
+
By("status is nil")
resp, err = instanceResource.Register(getContext(), &pb.RegisterInstanceRequest{
Instance: &pb.MicroServiceInstance{
diff --git a/server/service/util/heartbeat_util.go b/server/service/util/heartbeat_util.go
index 42432fe2..6b12ea69 100644
--- a/server/service/util/heartbeat_util.go
+++ b/server/service/util/heartbeat_util.go
@@ -20,17 +20,21 @@ import (
"errors"
apt "github.com/apache/servicecomb-service-center/server/core"
"github.com/apache/servicecomb-service-center/server/core/backend"
+ scerr "github.com/apache/servicecomb-service-center/server/error"
"github.com/apache/servicecomb-service-center/server/plugin/pkg/registry"
"golang.org/x/net/context"
)
-func HeartbeatUtil(ctx context.Context, domainProject string, serviceId string, instanceId string) (leaseID int64, ttl int64, err error, isInnerErr bool) {
- leaseID, err = GetLeaseId(ctx, domainProject, serviceId, instanceId)
+func HeartbeatUtil(ctx context.Context, domainProject string, serviceId string, instanceId string) (leaseID int64, ttl int64, _ *scerr.Error) {
+ leaseID, err := GetLeaseId(ctx, domainProject, serviceId, instanceId)
if err != nil {
- return leaseID, ttl, err, true
+ return leaseID, ttl, scerr.NewError(scerr.ErrUnavailableBackend, err.Error())
}
ttl, err = KeepAliveLease(ctx, domainProject, serviceId, instanceId, leaseID)
- return leaseID, ttl, err, false
+ if err != nil {
+ return leaseID, ttl, scerr.NewError(scerr.ErrInstanceNotExists, err.Error())
+ }
+ return leaseID, ttl, nil
}
func KeepAliveLease(ctx context.Context, domainProject, serviceId, instanceId string, leaseID int64) (ttl int64, err error) {
diff --git a/server/service/util/instance_util.go b/server/service/util/instance_util.go
index fea3692a..7c068c07 100644
--- a/server/service/util/instance_util.go
+++ b/server/service/util/instance_util.go
@@ -100,35 +100,6 @@ func GetInstanceCountOfOneService(ctx context.Context, domainProject string, ser
return resp.Count, nil
}
-func InstanceExistById(ctx context.Context, domainProject string, serviceId string, instanceId string) (bool, error) {
- opts := append(FromContext(ctx),
- registry.WithStrKey(apt.GenerateInstanceKey(domainProject, serviceId, instanceId)),
- registry.WithCountOnly())
- resp, err := backend.Store().Instance().Search(ctx, opts...)
- if err != nil {
- return false, err
- }
- if resp.Count <= 0 {
- return false, nil
- }
- return true, nil
-}
-
-func InstanceExist(ctx context.Context, instance *pb.MicroServiceInstance) (string, *scerr.Error) {
- domainProject := util.ParseDomainProject(ctx)
- // check id index
- if len(instance.InstanceId) > 0 {
- exist, err := InstanceExistById(ctx, domainProject, instance.ServiceId, instance.InstanceId)
- if err != nil {
- return "", scerr.NewError(scerr.ErrInternal, err.Error())
- }
- if exist {
- return instance.InstanceId, nil
- }
- }
- return "", nil
-}
-
type EndpointIndexValue struct {
serviceId string
instanceId string
diff --git a/server/service/util/instance_util_test.go b/server/service/util/instance_util_test.go
index 9e9f72d6..4ba0ffcb 100644
--- a/server/service/util/instance_util_test.go
+++ b/server/service/util/instance_util_test.go
@@ -18,7 +18,6 @@ package util
import (
"github.com/apache/servicecomb-service-center/pkg/util"
- "github.com/apache/servicecomb-service-center/server/core/proto"
pb "github.com/apache/servicecomb-service-center/server/core/proto"
scerr "github.com/apache/servicecomb-service-center/server/error"
"golang.org/x/net/context"
@@ -66,29 +65,6 @@ func TestGetInstance(t *testing.T) {
}
}
-func TestInstanceExistById(t *testing.T) {
- _, err := InstanceExistById(context.Background(), "", "", "")
- if err != nil {
- t.Fatalf(`InstanceExistById failed`)
- }
-}
-
-func TestInstanceExist(t *testing.T) {
- _, err := InstanceExist(context.Background(), &proto.MicroServiceInstance{
- ServiceId: "a",
- })
- if err != nil {
- t.Fatalf(`InstanceExist endpoint failed`)
- }
- _, err = InstanceExist(context.Background(), &proto.MicroServiceInstance{
- ServiceId: "a",
- InstanceId: "a",
- })
- if err != nil {
- t.Fatalf(`InstanceExist instanceId failed`)
- }
-}
-
func TestDeleteServiceAllInstances(t *testing.T) {
err := DeleteServiceAllInstances(context.Background(), "")
if err != nil {
----------------------------------------------------------------
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