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/06/25 13:57:12 UTC
[GitHub] little-cui closed pull request #380: SCB-680 Fix the wrong rev in
find instance api
little-cui closed pull request #380: SCB-680 Fix the wrong rev in find instance api
URL: https://github.com/apache/incubator-servicecomb-service-center/pull/380
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/handler/cache/cache.go b/server/handler/cache/cache.go
index 947ac5a8..6fdb69c5 100644
--- a/server/handler/cache/cache.go
+++ b/server/handler/cache/cache.go
@@ -21,7 +21,6 @@ import (
"github.com/apache/incubator-servicecomb-service-center/pkg/rest"
serviceUtil "github.com/apache/incubator-servicecomb-service-center/server/service/util"
"net/http"
- "strconv"
)
type CacheResponse struct {
@@ -34,7 +33,7 @@ func (l *CacheResponse) Handle(i *chain.Invocation) {
noCache := r.URL.Query().Get(serviceUtil.CTX_NOCACHE) == "1"
cacheOnly := r.URL.Query().Get(serviceUtil.CTX_CACHEONLY) == "1"
- rev, _ := strconv.ParseInt(r.URL.Query().Get("rev"), 10, 64)
+ rev := r.URL.Query().Get("rev")
if noCache {
i.WithContext(serviceUtil.CTX_NOCACHE, "1")
@@ -46,7 +45,7 @@ func (l *CacheResponse) Handle(i *chain.Invocation) {
return
}
- if rev > 0 {
+ if len(rev) > 0 {
i.WithContext(serviceUtil.CTX_REQUEST_REVISION, rev)
return
}
diff --git a/server/rest/controller/v4/instance_controller.go b/server/rest/controller/v4/instance_controller.go
index f105a68d..87cfe30c 100644
--- a/server/rest/controller/v4/instance_controller.go
+++ b/server/rest/controller/v4/instance_controller.go
@@ -139,10 +139,10 @@ func (this *MicroServiceInstanceService) FindInstances(w http.ResponseWriter, r
respInternal := resp.Response
resp.Response = nil
- iv, _ := r.Context().Value(serviceUtil.CTX_REQUEST_REVISION).(int64)
- ov, _ := r.Context().Value(serviceUtil.CTX_RESPONSE_REVISION).(int64)
+ iv, _ := r.Context().Value(serviceUtil.CTX_REQUEST_REVISION).(string)
+ ov, _ := r.Context().Value(serviceUtil.CTX_RESPONSE_REVISION).(string)
w.Header().Set(serviceUtil.HEADER_REV, fmt.Sprint(ov))
- if iv > 0 && iv == ov {
+ if len(iv) > 0 && iv == ov {
w.WriteHeader(http.StatusNotModified)
return
}
diff --git a/server/service/instance.go b/server/service/instance.go
index 1618886f..4b909681 100644
--- a/server/service/instance.go
+++ b/server/service/instance.go
@@ -540,8 +540,10 @@ func (s *InstanceService) Find(ctx context.Context, in *pb.FindInstancesRequest)
// cache
if item := serviceUtil.FindInstancesCache.Get(provider.Tenant, in.ConsumerServiceId, provider); item != nil {
noCache, cacheOnly := ctx.Value(serviceUtil.CTX_NOCACHE) == "1", ctx.Value(serviceUtil.CTX_CACHEONLY) == "1"
- rev, _ := ctx.Value(serviceUtil.CTX_REQUEST_REVISION).(int64)
- if !noCache && (cacheOnly || rev <= item.Rev) {
+ rev, _ := ctx.Value(serviceUtil.CTX_REQUEST_REVISION).(string)
+ reqRev, _ := serviceUtil.ParseRevision(rev)
+ cacheRev, _ := serviceUtil.ParseRevision(item.Rev)
+ if !noCache && (cacheOnly || reqRev <= cacheRev) {
instances := item.Instances
if rev == item.Rev {
instances = instances[:0]
diff --git a/server/service/instance_test.go b/server/service/instance_test.go
index a4bc595f..83f1ecb6 100644
--- a/server/service/instance_test.go
+++ b/server/service/instance_test.go
@@ -1182,12 +1182,13 @@ var _ = Describe("'Instance' service", func() {
})
Expect(err).To(BeNil())
Expect(respFind.Response.Code).To(Equal(pb.Response_SUCCESS))
- Expect(len(respFind.Instances)).To(Equal(1))
+ rev, _ := ctx.Value(serviceUtil.CTX_RESPONSE_REVISION).(string)
+ reqRev, reqCount := serviceUtil.ParseRevision(rev)
+ Expect(int64(len(respFind.Instances))).To(Equal(reqCount))
Expect(respFind.Instances[0].InstanceId).To(Equal(instanceId8))
- rev, _ := ctx.Value(serviceUtil.CTX_RESPONSE_REVISION).(int64)
- Expect(rev).NotTo(Equal(0))
+ Expect(reqRev).NotTo(Equal(0))
- util.SetContext(ctx, serviceUtil.CTX_REQUEST_REVISION, rev-1)
+ util.SetContext(ctx, serviceUtil.CTX_REQUEST_REVISION, reqRev-1)
respFind, err = instanceResource.Find(ctx, &pb.FindInstancesRequest{
ConsumerServiceId: serviceId8,
AppId: "query_instance",
@@ -1196,11 +1197,11 @@ var _ = Describe("'Instance' service", func() {
})
Expect(err).To(BeNil())
Expect(respFind.Response.Code).To(Equal(pb.Response_SUCCESS))
- Expect(len(respFind.Instances)).To(Equal(1))
+ Expect(int64(len(respFind.Instances))).To(Equal(reqCount))
Expect(respFind.Instances[0].InstanceId).To(Equal(instanceId8))
Expect(ctx.Value(serviceUtil.CTX_RESPONSE_REVISION)).To(Equal(rev))
- util.SetContext(ctx, serviceUtil.CTX_REQUEST_REVISION, rev+1)
+ util.SetContext(ctx, serviceUtil.CTX_REQUEST_REVISION, reqRev+1)
respFind, err = instanceResource.Find(ctx, &pb.FindInstancesRequest{
ConsumerServiceId: serviceId8,
AppId: "query_instance",
@@ -1209,7 +1210,7 @@ var _ = Describe("'Instance' service", func() {
})
Expect(err).To(BeNil())
Expect(respFind.Response.Code).To(Equal(pb.Response_SUCCESS))
- Expect(len(respFind.Instances)).To(Equal(1))
+ Expect(int64(len(respFind.Instances))).To(Equal(reqCount))
Expect(respFind.Instances[0].InstanceId).To(Equal(instanceId8))
Expect(ctx.Value(serviceUtil.CTX_RESPONSE_REVISION)).To(Equal(rev))
diff --git a/server/service/util/find_cache.go b/server/service/util/find_cache.go
index d5c2ab73..7a4c1701 100644
--- a/server/service/util/find_cache.go
+++ b/server/service/util/find_cache.go
@@ -29,7 +29,7 @@ var FindInstancesCache = &VersionRuleCache{
type VersionRuleCacheItem struct {
Instances []*pb.MicroServiceInstance
- Rev int64
+ Rev string
}
type VersionRuleCache struct {
diff --git a/server/service/util/find_cache_test.go b/server/service/util/find_cache_test.go
index 92751015..ff161bb0 100644
--- a/server/service/util/find_cache_test.go
+++ b/server/service/util/find_cache_test.go
@@ -28,13 +28,13 @@ func TestVersionRuleCache_Get(t *testing.T) {
t.Fatalf("TestVersionRuleCache_Get failed, %v", c)
}
- r := &VersionRuleCacheItem{Rev: 1}
+ r := &VersionRuleCacheItem{Rev: "1"}
FindInstancesCache.Set("d", "c", p, r)
c = FindInstancesCache.Get("d", "c", p)
if c == nil {
t.Fatalf("TestVersionRuleCache_Get failed, %v", c)
}
- if c.Rev != 1 {
+ if c.Rev != "1" {
t.Fatalf("TestVersionRuleCache_Get failed, rev %d != 1", c.Rev)
}
c = FindInstancesCache.Get("d", "c2", p)
@@ -54,7 +54,7 @@ func TestVersionRuleCache_Get(t *testing.T) {
if c == nil {
t.Fatalf("TestVersionRuleCache_Get failed, %v", c)
}
- if c.Rev != 1 {
+ if c.Rev != "1" {
t.Fatalf("TestVersionRuleCache_Get failed, rev %d != 1", c.Rev)
}
}
diff --git a/server/service/util/instance_util.go b/server/service/util/instance_util.go
index bf91107c..55932480 100644
--- a/server/service/util/instance_util.go
+++ b/server/service/util/instance_util.go
@@ -66,20 +66,36 @@ func GetInstance(ctx context.Context, domainProject string, serviceId string, in
return instance, nil
}
+func ParseRevision(rev string) (int64, int64) {
+ arrRev := strings.Split(rev, ".")
+ reqRev, _ := strconv.ParseInt(arrRev[0], 10, 64)
+ reqCount := int64(0)
+ if len(arrRev) > 1 {
+ reqCount, _ = strconv.ParseInt(arrRev[1], 10, 64)
+ }
+ return reqRev, reqCount
+}
+
+func FormatRevision(rev, count int64) string {
+ return fmt.Sprintf("%d.%d", rev, count)
+}
+
func GetAllInstancesOfServices(ctx context.Context, domainProject string, ids []string) (
- instances []*pb.MicroServiceInstance, rev int64, err error) {
+ instances []*pb.MicroServiceInstance, rev string, err error) {
cloneCtx := util.CloneContext(ctx)
noCache, cacheOnly := ctx.Value(CTX_NOCACHE) == "1", ctx.Value(CTX_CACHEONLY) == "1"
- rev, _ = cloneCtx.Value(CTX_REQUEST_REVISION).(int64)
- if !noCache && !cacheOnly && rev > 0 {
- // force to find in cache at first time when rev > 0
+ rawRev, _ := cloneCtx.Value(CTX_REQUEST_REVISION).(string)
+ reqRev, reqCount := ParseRevision(rawRev)
+ if !noCache && !cacheOnly && reqRev > 0 {
+ // force to find in cache at first time when rev is not empty
util.SetContext(cloneCtx, CTX_CACHEONLY, "1")
}
var (
- max int64
- kvs []*mvccpb.KeyValue
+ maxRev int64
+ instCount int64
+ kvs []*mvccpb.KeyValue
)
for i := 0; i < 2; i++ {
for _, serviceId := range ids {
@@ -87,28 +103,31 @@ func GetAllInstancesOfServices(ctx context.Context, domainProject string, ids []
opts := append(FromContext(cloneCtx), registry.WithStrKey(key), registry.WithPrefix())
resp, err := backend.Store().Instance().Search(cloneCtx, opts...)
if err != nil {
- return nil, 0, err
+ return nil, "", err
}
- if len(resp.Kvs) > 0 {
- kvs = append(kvs, resp.Kvs...)
+ if len(resp.Kvs) == 0 {
+ continue
}
- if cmax := resp.MaxModRevision(); max < cmax {
- max = cmax
+
+ if cmax := resp.MaxModRevision(); maxRev < cmax {
+ maxRev = cmax
}
+ instCount += int64(len(resp.Kvs))
+ kvs = append(kvs, resp.Kvs...)
}
- if noCache || cacheOnly || rev == 0 {
+ if noCache || cacheOnly || len(rev) == 0 {
break
}
- if rev == max {
+ if reqRev == maxRev && reqCount == instCount {
// return not modified
kvs = kvs[:0]
break
}
- if rev < max || i != 0 {
+ if reqRev < maxRev || i != 0 {
break
}
@@ -123,13 +142,13 @@ func GetAllInstancesOfServices(ctx context.Context, domainProject string, ids []
instance := &pb.MicroServiceInstance{}
err := json.Unmarshal(kv.Value, instance)
if err != nil {
- return nil, 0, fmt.Errorf("unmarshal %s faild, %s",
+ return nil, "", fmt.Errorf("unmarshal %s faild, %s",
util.BytesToStringWithNoCopy(kv.Key), err.Error())
}
instances = append(instances, instance)
}
- rev = max
+ rev = FormatRevision(maxRev, instCount)
return
}
----------------------------------------------------------------
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