bugfix: metrics client is initialized 2 times per shard #484
Conversation
|
Where is the 2 times initialization in old implementation? |
|
@vancexu cassandraPersistenceClientFactory.go and shardContext.go |
|
@yiminc A2 & A3: since the tagged metrics client is per shard (from the original code), i assume that we need to continue to do that. |
| @@ -94,7 +94,7 @@ func (s *shardControllerSuite) TestAcquireShardSuccess() { | |||
| if hostID == 0 { | |||
| myShards = append(myShards, shardID) | |||
| mockExecutionMgr := &mmocks.ExecutionManager{} | |||
| s.mockExecutionMgrFactory.On("CreateExecutionManager", mock.Anything).Return(mockExecutionMgr, nil).Once() | |||
| s.mockExecutionMgrFactory.On("CreateExecutionManager", mock.Anything, mock.Anything).Return(mockExecutionMgr, nil).Once() | |||
| logger bark.Logger | ||
| metricsClient metrics.Client |
There was a problem hiding this comment.
we need this field, so the factory can use the metrics client if provided
| @@ -214,7 +214,8 @@ func (s *Service) Start() { | |||
| p.CassandraConfig.Keyspace, | |||
| s.config.ExecutionMgrNumConns, | |||
| p.Logger, | |||
| base.GetMetricsClient()) | |||
There was a problem hiding this comment.
What does this method do? Should it be removed if no longer used?
There was a problem hiding this comment.
@madhuravi
this is actually referenced at line 138:
https://github.com/uber/cadence/pull/484/files/b41eff2eeeca590eba88c5cb7ef67de7977ac034#diff-48531afaf74492193b35d730980b6ecdR138
| @@ -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) { | |||
| @@ -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) { | |||
solves #477