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{},
)
}