From 8a89e911b460371aa7df85ee5356daef5187a0db Mon Sep 17 00:00:00 2001 From: wxing1292 Date: Tue, 2 Jan 2018 14:26:35 -0800 Subject: [PATCH] bugfix: metrics client is initialized 2 times per shard (#484) * bugfix: metrics client is initialized 2 times per shard, now is initialized 1 per host --- common/mocks/ExecutionManagerFactory.go | 6 ++++-- .../persistence/cassandraPersistenceClientFactory.go | 10 +++------- service/history/historyEngine.go | 1 - service/history/service.go | 3 ++- service/history/shardContext.go | 6 ++---- service/history/shardController.go | 4 ++-- 6 files changed, 13 insertions(+), 17 deletions(-) diff --git a/common/mocks/ExecutionManagerFactory.go b/common/mocks/ExecutionManagerFactory.go index 4e4ae19059b..5822294a460 100644 --- a/common/mocks/ExecutionManagerFactory.go +++ b/common/mocks/ExecutionManagerFactory.go @@ -20,8 +20,10 @@ package mocks -import "github.com/uber/cadence/common/persistence" -import "github.com/stretchr/testify/mock" +import ( + "github.com/stretchr/testify/mock" + "github.com/uber/cadence/common/persistence" +) // ExecutionManagerFactory is an autogenerated mock type for the ExecutionManagerFactory type type ExecutionManagerFactory struct { diff --git a/common/persistence/cassandraPersistenceClientFactory.go b/common/persistence/cassandraPersistenceClientFactory.go index cc2012e1a99..a3f67fadb2b 100644 --- a/common/persistence/cassandraPersistenceClientFactory.go +++ b/common/persistence/cassandraPersistenceClientFactory.go @@ -37,7 +37,7 @@ type ( // NewCassandraPersistenceClientFactory is used to create an instance of ExecutionManagerFactory implementation func NewCassandraPersistenceClientFactory(hosts string, port int, user, password, dc string, keyspace string, - numConns int, logger bark.Logger, mClient metrics.Client) (ExecutionManagerFactory, error) { + numConns int, logger bark.Logger, metricsClient metrics.Client) (ExecutionManagerFactory, error) { cluster := common.NewCassandraCluster(hosts, port, user, password, dc) cluster.Keyspace = keyspace cluster.ProtoVersion = cassandraProtoVersion @@ -51,7 +51,7 @@ func NewCassandraPersistenceClientFactory(hosts string, port int, user, password return nil, err } - return &cassandraPersistenceClientFactory{session: session, metricsClient: mClient, logger: logger}, nil + return &cassandraPersistenceClientFactory{session: session, logger: logger, metricsClient: metricsClient}, nil } // CreateExecutionManager implements ExecutionManagerFactory interface @@ -66,11 +66,7 @@ func (f *cassandraPersistenceClientFactory) CreateExecutionManager(shardID int) return mgr, nil } - tags := map[string]string{ - metrics.ShardTagName: metrics.AllShardsTagValue, - } - return NewWorkflowExecutionPersistenceClient( - mgr, f.metricsClient.Tagged(tags)), nil + return NewWorkflowExecutionPersistenceClient(mgr, f.metricsClient), nil } // Close releases the underlying resources held by this object diff --git a/service/history/historyEngine.go b/service/history/historyEngine.go index 0f192e1ea82..7858ce15183 100644 --- a/service/history/historyEngine.go +++ b/service/history/historyEngine.go @@ -58,7 +58,6 @@ type ( historyEventNotifier historyEventNotifier tokenSerializer common.TaskTokenSerializer hSerializerFactory persistence.HistorySerializerFactory - metricsReporter metrics.Client historyCache *historyCache domainCache cache.DomainCache metricsClient metrics.Client diff --git a/service/history/service.go b/service/history/service.go index 9aaaf790cc1..208f2e3b47e 100644 --- a/service/history/service.go +++ b/service/history/service.go @@ -214,7 +214,8 @@ func (s *Service) Start() { p.CassandraConfig.Keyspace, s.config.ExecutionMgrNumConns, p.Logger, - base.GetMetricsClient()) + s.metricsClient, + ) if err != nil { log.Fatalf("Creating Cassandra execution manager persistence factory failed: %v", err) } diff --git a/service/history/shardContext.go b/service/history/shardContext.go index 7b00d9d89ce..1d1aa4bb240 100644 --- a/service/history/shardContext.go +++ b/service/history/shardContext.go @@ -463,9 +463,7 @@ func acquireShard(shardID int, shardManager persistence.ShardManager, historyMgr shardInfo := response.ShardInfo updatedShardInfo := copyShardInfo(shardInfo) updatedShardInfo.Owner = owner - tags := map[string]string{ - metrics.ShardTagName: metrics.AllShardsTagValue, - } + context := &shardContextImpl{ shardID: shardID, shardManager: shardManager, @@ -474,7 +472,7 @@ func acquireShard(shardID int, shardManager persistence.ShardManager, historyMgr domainCache: domainCache, shardInfo: updatedShardInfo, closeCh: closeCh, - metricsClient: metricsClient.Tagged(tags), + metricsClient: metricsClient, config: config, } context.logger = logger.WithFields(bark.Fields{ diff --git a/service/history/shardController.go b/service/history/shardController.go index f4b403eba2d..d8c41f1425a 100644 --- a/service/history/shardController.go +++ b/service/history/shardController.go @@ -107,7 +107,7 @@ func newShardController(host *membership.HostInfo, resolver membership.ServiceRe func newHistoryShardsItem(shardID int, shardMgr persistence.ShardManager, historyMgr persistence.HistoryManager, metadataMgr persistence.MetadataManager, executionMgrFactory persistence.ExecutionManagerFactory, factory EngineFactory, - host *membership.HostInfo, config *Config, logger bark.Logger, reporter metrics.Client) (*historyShardsItem, error) { + host *membership.HostInfo, config *Config, logger bark.Logger, metricsClient metrics.Client) (*historyShardsItem, error) { executionMgr, err := executionMgrFactory.CreateExecutionManager(shardID) if err != nil { @@ -128,7 +128,7 @@ func newHistoryShardsItem(shardID int, shardMgr persistence.ShardManager, histor logger: logger.WithFields(bark.Fields{ logging.TagHistoryShardID: shardID, }), - metricsClient: reporter, + metricsClient: metricsClient, }, nil }