Fix check idle tasklist to include active task writers #916
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
samarabbas
reviewed
Jul 3, 2018
service/matching/taskListManager.go
Outdated
| @@ -338,6 +343,7 @@ func (c *taskListManagerImpl) Start() error { | |||
| c.taskWriter.Start() | |||
| c.signalNewTask() | |||
| go c.getTasksPump() | |||
| go c.checkIdle() | |||
There was a problem hiding this comment.
Why do we need a separate goroutine for idle check? why can't be done in the same pump like we do for zero pollers check?
There was a problem hiding this comment.
Changed, previously because I try to avoid this check potentially block getTaskPump
samarabbas
reviewed
Jul 3, 2018
service/matching/taskListManager.go
Outdated
| @@ -949,3 +969,16 @@ func (c *taskContext) completeTask(err error) { | |||
| func createServiceBusyError(msg string) *s.ServiceBusyError { | |||
| return &s.ServiceBusyError{Message: msg} | |||
| } | |||
|
|
|||
| func (c *taskListManagerImpl) updateLastAddTaskTime() { | |||
| c.lastAddTaskTimeLock.Lock() | |||
There was a problem hiding this comment.
I think we should do this without adding another lock.
There was a problem hiding this comment.
Update with better solution in getTaskPump
samarabbas
reviewed
Jul 3, 2018
service/matching/taskListManager.go
Outdated
| @@ -51,6 +51,8 @@ const ( | |||
| const ( | |||
| _defaultTaskDispatchRPS = 100000.0 | |||
| _defaultTaskDispatchRPSTTL = 60 * time.Second | |||
|
|
|||
| maxAddTasksIdleTime = 5 * time.Minute | |||
There was a problem hiding this comment.
Can you convert this to a config knob.
samarabbas
approved these changes
Jul 3, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Currently in matching getTaskPump there is a timer to check whether tasklist is "idle" and need to recycle. But it only treat no poller within 5 min as idle to recycle.
This PR add check idle to include if there are tasks added recently (5 min).
The default live time it currently hardcoded as 5 min for both poller and writer. Which can be make configurable if necessary.