Skip to content

Commit

Permalink
Call GetDomainName once in task_list_mngr:NewManager() (cadence-workf…
Browse files Browse the repository at this point in the history
…low#6107)

This makes newTaskListConfig error-less as it should be (since it just
produces config)
  • Loading branch information
dkrotx authored Jun 5, 2024
1 parent 100c5ba commit 294a120
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 17 deletions.
11 changes: 6 additions & 5 deletions service/matching/tasklist/matcher_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,8 @@ import (
"github.com/uber/cadence/service/matching/config"
)

const testDomainName = "test-domain"

type MatcherTestSuite struct {
suite.Suite
controller *gomock.Controller
Expand All @@ -64,8 +66,8 @@ func (t *MatcherTestSuite) SetupTest() {
t.client = matching.NewMockClient(t.controller)
cfg := config.NewConfig(dynamicconfig.NewNopCollection(), "some random hostname")
t.taskList = NewTestTaskListID(t.T(), uuid.New(), common.ReservedTaskListPrefix+"tl0/1", persistence.TaskListTypeDecision)
tlCfg, err := newTaskListConfig(t.taskList, cfg, t.newDomainCache())
t.NoError(err)
tlCfg := newTaskListConfig(t.taskList, cfg, testDomainName)

tlCfg.ForwarderConfig = config.ForwarderConfig{
ForwarderMaxOutstandingPolls: func() int { return 1 },
ForwarderMaxOutstandingTasks: func() int { return 1 },
Expand All @@ -78,8 +80,7 @@ func (t *MatcherTestSuite) SetupTest() {
t.matcher = newTaskMatcher(tlCfg, t.fwdr, metrics.NoopScope(metrics.Matching), []string{"dca1", "dca2"}, loggerimpl.NewNopLogger())

rootTaskList := NewTestTaskListID(t.T(), t.taskList.GetDomainID(), t.taskList.Parent(20), persistence.TaskListTypeDecision)
rootTasklistCfg, err := newTaskListConfig(rootTaskList, cfg, t.newDomainCache())
t.NoError(err)
rootTasklistCfg := newTaskListConfig(rootTaskList, cfg, testDomainName)
t.rootMatcher = newTaskMatcher(rootTasklistCfg, nil, metrics.NoopScope(metrics.Matching), []string{"dca1", "dca2"}, loggerimpl.NewNopLogger())
}

Expand Down Expand Up @@ -593,7 +594,7 @@ func (t *MatcherTestSuite) TestIsolationPollFailure() {
}

func (t *MatcherTestSuite) newDomainCache() cache.DomainCache {
domainName := "test-domain"
domainName := testDomainName
dc := cache.NewMockDomainCache(t.controller)
dc.EXPECT().GetDomainName(gomock.Any()).Return(domainName, nil).AnyTimes()
return dc
Expand Down
19 changes: 7 additions & 12 deletions service/matching/tasklist/task_list_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -142,19 +142,18 @@ func NewManager(
timeSource clock.TimeSource,
createTime time.Time,
) (Manager, error) {
taskListConfig, err := newTaskListConfig(taskList, config, domainCache)
domainName, err := domainCache.GetDomainName(taskList.GetDomainID())
if err != nil {
return nil, err
}

taskListConfig := newTaskListConfig(taskList, config, domainName)

if taskListKind == nil {
normalTaskListKind := types.TaskListKindNormal
taskListKind = &normalTaskListKind
}
domainName, err := domainCache.GetDomainName(taskList.GetDomainID())
if err != nil {
return nil, err
}

scope := NewPerTaskListScope(domainName, taskList.GetName(), *taskListKind, metricsClient, metrics.MatchingTaskListMgrScope)
db := newTaskListDB(taskManager, taskList.GetDomainID(), domainName, taskList.GetName(), taskList.GetType(), int(*taskListKind), logger)

Expand Down Expand Up @@ -253,6 +252,7 @@ func (c *taskListManagerImpl) handleErr(err error) error {
// be written to database and later asynchronously matched with a poller
func (c *taskListManagerImpl) AddTask(ctx context.Context, params AddTaskParams) (bool, error) {
c.startWG.Wait()

if c.shouldReload() {
c.Stop()
return false, errShutdown
Expand Down Expand Up @@ -662,12 +662,7 @@ func rangeIDToTaskIDBlock(rangeID, rangeSize int64) taskIDBlock {
}
}

func newTaskListConfig(id *Identifier, cfg *config.Config, domainCache cache.DomainCache) (*config.TaskListConfig, error) {
domainName, err := domainCache.GetDomainName(id.GetDomainID())
if err != nil {
return nil, err
}

func newTaskListConfig(id *Identifier, cfg *config.Config, domainName string) *config.TaskListConfig {
taskListName := id.GetName()
taskType := id.GetType()
return &config.TaskListConfig{
Expand Down Expand Up @@ -734,7 +729,7 @@ func newTaskListConfig(id *Identifier, cfg *config.Config, domainCache cache.Dom
TaskDispatchRPS: cfg.TaskDispatchRPS,
TaskDispatchRPSTTL: cfg.TaskDispatchRPSTTL,
MaxTimeBetweenTaskDeletes: cfg.MaxTimeBetweenTaskDeletes,
}, nil
}
}

func NewPerTaskListScope(
Expand Down

0 comments on commit 294a120

Please sign in to comment.