This is an automated email from the ASF dual-hosted git repository.

tianxiaoliang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/servicecomb-service-center.git


The following commit(s) were added to refs/heads/master by this push:
     new b63555b  [SCB-1742] Use cache only when Indexer is not creditable 
(#629)
b63555b is described below

commit b63555bb32c3325bc69455b5fc3d816228adddec
Author: humingcheng <[email protected]>
AuthorDate: Fri Feb 7 09:48:21 2020 +0800

    [SCB-1742] Use cache only when Indexer is not creditable (#629)
    
    * [SCB-1742] Use cache only and not call the backend when Indexer is not 
creditable
    
    * [SCB-1742] Add comments
---
 pkg/rest/client.go                                 |  6 +++---
 server/plugin/pkg/discovery/aggregate/indexer.go   | 22 +++++++++++++++++++
 server/plugin/pkg/discovery/etcd/indexer_cache.go  |  7 ++++++
 server/plugin/pkg/discovery/etcd/indexer_etcd.go   |  9 +++++++-
 server/plugin/pkg/discovery/indexer.go             |  5 +++++
 server/plugin/pkg/discovery/indexer_cache.go       |  9 +++++++-
 .../plugin/pkg/discovery/servicecenter/indexer.go  |  8 +++++++
 .../plugin/pkg/discovery/servicecenter/syncer.go   | 25 +++++++++++-----------
 .../cache/{filter_rev.go => filter_consistency.go} | 20 +++++++++++++----
 server/service/cache/instance.go                   |  2 +-
 10 files changed, 90 insertions(+), 23 deletions(-)

diff --git a/pkg/rest/client.go b/pkg/rest/client.go
index 0bb9948..c2aa631 100644
--- a/pkg/rest/client.go
+++ b/pkg/rest/client.go
@@ -41,9 +41,9 @@ var defaultURLClientOption = URLClientOption{
        Compressed:            true,
        VerifyPeer:            true,
        SSLVersion:            tls.VersionTLS12,
-       HandshakeTimeout:      30 * time.Second,
-       ResponseHeaderTimeout: 180 * time.Second,
-       RequestTimeout:        300 * time.Second,
+       HandshakeTimeout:      10 * time.Second,
+       ResponseHeaderTimeout: 30 * time.Second,
+       RequestTimeout:        60 * time.Second,
        ConnsPerHost:          DEFAULT_CONN_POOL_PER_HOST_SIZE,
 }
 
diff --git a/server/plugin/pkg/discovery/aggregate/indexer.go 
b/server/plugin/pkg/discovery/aggregate/indexer.go
index d425b0d..6117d52 100644
--- a/server/plugin/pkg/discovery/aggregate/indexer.go
+++ b/server/plugin/pkg/discovery/aggregate/indexer.go
@@ -19,6 +19,7 @@ import (
        "github.com/apache/servicecomb-service-center/pkg/util"
        
"github.com/apache/servicecomb-service-center/server/plugin/pkg/discovery"
        
"github.com/apache/servicecomb-service-center/server/plugin/pkg/registry"
+
        "golang.org/x/net/context"
 )
 
@@ -29,6 +30,11 @@ type AdaptorsIndexer struct {
        Adaptors []discovery.Adaptor
 }
 
+// Search implements discovery.Indexer.Search.
+// AdaptorsIndexer ignores the errors during search to ensure availability, so
+// it always searches successfully, no matter how many Adaptors are abnormal.
+// But at the cost of that, AdaptorsIndexer doesn't guarantee the correctness
+// of the search results.
 func (i *AdaptorsIndexer) Search(ctx context.Context, opts 
...registry.PluginOpOption) (*discovery.Response, error) {
        var (
                response discovery.Response
@@ -51,6 +57,14 @@ func (i *AdaptorsIndexer) Search(ctx context.Context, opts 
...registry.PluginOpO
        return &response, nil
 }
 
+// Creditable implements discovery.Indexer.Creditable.
+// AdaptorsIndexer's search result's are not creditable as it ignores the
+// errors. In other words, AdaptorsIndexer makes the best efforts to search
+// data, but it does not ensure the correctness.
+func (i *AdaptorsIndexer) Creditable() bool {
+       return false
+}
+
 func NewAdaptorsIndexer(as []discovery.Adaptor) *AdaptorsIndexer {
        return &AdaptorsIndexer{Adaptors: as}
 }
@@ -67,6 +81,7 @@ type AggregatorIndexer struct {
        LocalIndexer discovery.Indexer
 }
 
+// Search implements discovery.Indexer.Search.
 func (i *AggregatorIndexer) Search(ctx context.Context, opts 
...registry.PluginOpOption) (resp *discovery.Response, err error) {
        op := registry.OpGet(opts...)
 
@@ -95,6 +110,13 @@ func (i *AggregatorIndexer) search(ctx context.Context, 
opts ...registry.PluginO
        return i.AdaptorsIndexer.Search(ctx, opts...)
 }
 
+// Creditable implements discovery.Indexer.Creditable.
+func (i *AggregatorIndexer) Creditable() bool {
+       return i.AdaptorsIndexer.Creditable() &&
+               i.LocalIndexer.Creditable() &&
+               i.CacheIndexer.Creditable()
+}
+
 func NewAggregatorIndexer(as *Aggregator) *AggregatorIndexer {
        indexer := NewAdaptorsIndexer(as.Adaptors)
        ai := &AggregatorIndexer{
diff --git a/server/plugin/pkg/discovery/etcd/indexer_cache.go 
b/server/plugin/pkg/discovery/etcd/indexer_cache.go
index 6b9463f..66ea6e9 100644
--- a/server/plugin/pkg/discovery/etcd/indexer_cache.go
+++ b/server/plugin/pkg/discovery/etcd/indexer_cache.go
@@ -18,9 +18,11 @@ package etcd
 
 import (
        "fmt"
+
        "github.com/apache/servicecomb-service-center/pkg/util"
        
"github.com/apache/servicecomb-service-center/server/plugin/pkg/discovery"
        
"github.com/apache/servicecomb-service-center/server/plugin/pkg/registry"
+
        "golang.org/x/net/context"
 )
 
@@ -55,6 +57,11 @@ func (i *CacheIndexer) Search(ctx context.Context, opts 
...registry.PluginOpOpti
        return i.EtcdIndexer.Search(ctx, opts...)
 }
 
+// Creditable implements discovery.Indexer.Creditable.
+func (i *CacheIndexer) Creditable() bool {
+       return i.EtcdIndexer.Creditable()
+}
+
 func NewCacheIndexer(cfg *discovery.Config, cache discovery.Cache) 
*CacheIndexer {
        return &CacheIndexer{
                EtcdIndexer:  NewEtcdIndexer(cfg.Key, cfg.Parser),
diff --git a/server/plugin/pkg/discovery/etcd/indexer_etcd.go 
b/server/plugin/pkg/discovery/etcd/indexer_etcd.go
index e5d201b..5b06782 100644
--- a/server/plugin/pkg/discovery/etcd/indexer_etcd.go
+++ b/server/plugin/pkg/discovery/etcd/indexer_etcd.go
@@ -18,14 +18,16 @@ package etcd
 
 import (
        "fmt"
+       "strings"
+
        "github.com/apache/servicecomb-service-center/pkg/log"
        "github.com/apache/servicecomb-service-center/pkg/util"
        "github.com/apache/servicecomb-service-center/server/core/backend"
        pb "github.com/apache/servicecomb-service-center/server/core/proto"
        
"github.com/apache/servicecomb-service-center/server/plugin/pkg/discovery"
        
"github.com/apache/servicecomb-service-center/server/plugin/pkg/registry"
+
        "golang.org/x/net/context"
-       "strings"
 )
 
 // EtcdIndexer implements discovery.Indexer.
@@ -81,6 +83,11 @@ func (i *EtcdIndexer) Search(ctx context.Context, opts 
...registry.PluginOpOptio
        return
 }
 
+// Creditable implements discovery.Indexer.Creditable.
+func (i *EtcdIndexer) Creditable() bool {
+       return true
+}
+
 func NewEtcdIndexer(root string, p pb.Parser) (indexer *EtcdIndexer) {
        return &EtcdIndexer{Client: backend.Registry(), Parser: p, Root: root}
 }
diff --git a/server/plugin/pkg/discovery/indexer.go 
b/server/plugin/pkg/discovery/indexer.go
index b7e42b5..b21c65b 100644
--- a/server/plugin/pkg/discovery/indexer.go
+++ b/server/plugin/pkg/discovery/indexer.go
@@ -18,6 +18,7 @@ package discovery
 
 import (
        
"github.com/apache/servicecomb-service-center/server/plugin/pkg/registry"
+
        "golang.org/x/net/context"
 )
 
@@ -27,4 +28,8 @@ import (
 type Indexer interface {
        // Search searches k-v data based on the input options
        Search(ctx context.Context, opts ...registry.PluginOpOption) 
(*Response, error)
+       // Creditable judges whether Indexer's search results are creditable
+       // It is recommended to use cache only and not to call the backend
+       // directly, If Indexer is not creditable.
+       Creditable() bool
 }
diff --git a/server/plugin/pkg/discovery/indexer_cache.go 
b/server/plugin/pkg/discovery/indexer_cache.go
index 0f3c675..7641a7b 100644
--- a/server/plugin/pkg/discovery/indexer_cache.go
+++ b/server/plugin/pkg/discovery/indexer_cache.go
@@ -17,11 +17,13 @@
 package discovery
 
 import (
+       "time"
+
        "github.com/apache/servicecomb-service-center/pkg/log"
        "github.com/apache/servicecomb-service-center/pkg/util"
        
"github.com/apache/servicecomb-service-center/server/plugin/pkg/registry"
+
        "golang.org/x/net/context"
-       "time"
 )
 
 // CacheIndexer implements discovery.Indexer.
@@ -76,6 +78,11 @@ func (i *CacheIndexer) searchByPrefix(op registry.PluginOp) 
*Response {
        return resp
 }
 
+// Creditable implements discovery.Indexer.Creditable.
+func (i *CacheIndexer) Creditable() bool {
+       return true
+}
+
 func NewCacheIndexer(cache CacheReader) *CacheIndexer {
        return &CacheIndexer{
                Cache: cache,
diff --git a/server/plugin/pkg/discovery/servicecenter/indexer.go 
b/server/plugin/pkg/discovery/servicecenter/indexer.go
index 00ab11d..a37fb6d 100644
--- a/server/plugin/pkg/discovery/servicecenter/indexer.go
+++ b/server/plugin/pkg/discovery/servicecenter/indexer.go
@@ -24,6 +24,7 @@ import (
        scerr "github.com/apache/servicecomb-service-center/server/error"
        
"github.com/apache/servicecomb-service-center/server/plugin/pkg/discovery"
        
"github.com/apache/servicecomb-service-center/server/plugin/pkg/registry"
+
        "golang.org/x/net/context"
 )
 
@@ -106,6 +107,13 @@ func (i *ClusterIndexer) searchInstances(ctx 
context.Context, op registry.Plugin
        return resp, nil
 }
 
+// Creditable implements discovery.Indexer.Creditable.
+// ClusterIndexer's search result's are not creditable as SCClientAggregate
+// ignores sc clients' errors.
+func (i *ClusterIndexer) Creditable() bool {
+       return false
+}
+
 func NewClusterIndexer(t discovery.Type, cache discovery.Cache) 
*ClusterIndexer {
        return &ClusterIndexer{
                CacheIndexer: discovery.NewCacheIndexer(cache),
diff --git a/server/plugin/pkg/discovery/servicecenter/syncer.go 
b/server/plugin/pkg/discovery/servicecenter/syncer.go
index ea24fff..6cedcf2 100644
--- a/server/plugin/pkg/discovery/servicecenter/syncer.go
+++ b/server/plugin/pkg/discovery/servicecenter/syncer.go
@@ -17,6 +17,9 @@ package servicecenter
 
 import (
        "fmt"
+       "sync"
+       "time"
+
        "github.com/apache/servicecomb-service-center/pkg/gopool"
        "github.com/apache/servicecomb-service-center/pkg/log"
        "github.com/apache/servicecomb-service-center/pkg/util"
@@ -27,9 +30,8 @@ import (
        pb "github.com/apache/servicecomb-service-center/server/core/proto"
        
"github.com/apache/servicecomb-service-center/server/plugin/pkg/discovery"
        
"github.com/apache/servicecomb-service-center/server/plugin/pkg/registry"
+
        "golang.org/x/net/context"
-       "sync"
-       "time"
 )
 
 var (
@@ -48,20 +50,18 @@ func (c *Syncer) Initialize() {
        c.Client = GetOrCreateSCClient()
 }
 
-func (c *Syncer) Sync(ctx context.Context) error {
+func (c *Syncer) Sync(ctx context.Context) {
        cache, errs := c.Client.GetScCache(ctx)
-       if cache == nil && len(errs) > 0 {
+       if len(errs) > 0 {
                err := fmt.Errorf("%v", errs)
-               log.Errorf(err, "sync failed")
-               return err
-       }
-
-       if len(errs) == 0 {
-               alarm.Clear(alarm.IdBackendConnectionRefuse)
-       } else {
+               log.Errorf(err, "Sync catches errors")
                alarm.Raise(alarm.IdBackendConnectionRefuse,
-                       alarm.AdditionalContext("%v", errs))
+                       alarm.AdditionalContext(err.Error()))
+               if cache == nil {
+                       return
+               }
        }
+       alarm.Clear(alarm.IdBackendConnectionRefuse)
 
        // microservice
        serviceCacher, ok := c.cachers[backend.SERVICE]
@@ -102,7 +102,6 @@ func (c *Syncer) Sync(ctx context.Context) error {
        if ok {
                c.check(instCacher, &cache.Instances, errs)
        }
-       return nil
 }
 
 func (c *Syncer) check(local *ServiceCenterCacher, remote model.Getter, 
skipClusters map[string]error) {
diff --git a/server/service/cache/filter_rev.go 
b/server/service/cache/filter_consistency.go
similarity index 68%
rename from server/service/cache/filter_rev.go
rename to server/service/cache/filter_consistency.go
index 381b033..feb18bc 100644
--- a/server/service/cache/filter_rev.go
+++ b/server/service/cache/filter_consistency.go
@@ -20,15 +20,19 @@ import (
        "github.com/apache/servicecomb-service-center/pkg/cache"
        "github.com/apache/servicecomb-service-center/pkg/log"
        "github.com/apache/servicecomb-service-center/pkg/util"
+       "github.com/apache/servicecomb-service-center/server/core/backend"
        serviceUtil 
"github.com/apache/servicecomb-service-center/server/service/util"
+
        "golang.org/x/net/context"
 )
 
-type RevisionFilter struct {
+// ConsistencyFilter improves consistency.
+// Scenario: cache maybe different between several service-centers.
+type ConsistencyFilter struct {
        InstancesFilter
 }
 
-func (f *RevisionFilter) Name(ctx context.Context, parent *cache.Node) string {
+func (f *ConsistencyFilter) Name(ctx context.Context, parent *cache.Node) 
string {
        item := parent.Cache.Get(CACHE_FIND).(*VersionRuleCacheItem)
        requestRev := ctx.Value(CTX_FIND_REQUEST_REV).(string)
        if len(requestRev) == 0 || requestRev == item.Rev {
@@ -37,10 +41,18 @@ func (f *RevisionFilter) Name(ctx context.Context, parent 
*cache.Node) string {
        return requestRev
 }
 
-func (f *RevisionFilter) Init(ctx context.Context, parent *cache.Node) (node 
*cache.Node, err error) {
+// Init generates cache.
+// We think cache inconsistency happens and correction is needed only when the
+// revision in the request is not empty and different from the revision of
+// parent cache. To correct inconsistency, RevisionFilter skips cache and get
+// data from the backend directly to response.
+// It's impossible to guarantee consistency if the backend is not creditable,
+// thus in this condition RevisionFilter uses cache only.
+func (f *ConsistencyFilter) Init(ctx context.Context, parent *cache.Node) 
(node *cache.Node, err error) {
        pCache := parent.Cache.Get(CACHE_FIND).(*VersionRuleCacheItem)
        requestRev := ctx.Value(CTX_FIND_REQUEST_REV).(string)
-       if len(requestRev) == 0 || requestRev == pCache.Rev {
+       if len(requestRev) == 0 || requestRev == pCache.Rev ||
+               !(backend.Store().Instance().Creditable()) {
                node = cache.NewNode()
                node.Cache.Set(CACHE_FIND, pCache)
                return
diff --git a/server/service/cache/instance.go b/server/service/cache/instance.go
index b6ec609..72b2593 100644
--- a/server/service/cache/instance.go
+++ b/server/service/cache/instance.go
@@ -37,7 +37,7 @@ func init() {
                &TagsFilter{},
                &AccessibleFilter{},
                &InstancesFilter{},
-               &RevisionFilter{},
+               &ConsistencyFilter{},
        )
 }
 

Reply via email to