You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@servicecomb.apache.org by ti...@apache.org on 2020/02/07 01:48:29 UTC

[servicecomb-service-center] branch master updated: [SCB-1742] Use cache only when Indexer is not creditable (#629)

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 <hu...@163.com>
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{},
 	)
 }