Bugfix: deadlock when domain failover #919
Merged
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
Contributor
wxing1292
commented
Jul 3, 2018
wxing1292
commented
- fix deadlock issue in timer queue processor failover
- fix domain cache copy without locking issue
fix domain cache copy without locking issue
samarabbas
reviewed
Jul 3, 2018
| @@ -143,7 +143,9 @@ func (t *timerQueueProcessorImpl) FailoverDomain(domainID string) { | |||
| failoverTimerProcessor := newTimerQueueFailoverProcessor(t.shard, t.historyService, domainID, | |||
| standbyClusterName, minLevel, maxLevel, t.matchingClient, t.logger) | |||
| failoverTimerProcessor.Start() | |||
| failoverTimerProcessor.timerQueueProcessorBase.readAndFanoutTimerTasks() | |||
| // use a fake timer to trigger db scan | |||
There was a problem hiding this comment.
'readAndFanoutTimerTasks' is still called from other places and I thought that is the one which could block.
There was a problem hiding this comment.
readAndFanoutTimerTasks will be called by an separate go routine, so we should be fine.
samarabbas
reviewed
Jul 3, 2018
| c.beforeCallbacks[shard] = beforeCallback | ||
| c.afterCallbacks[shard] = afterCallback | ||
| domainNotificationVersion := c.domainNotificationVersion | ||
| c.Unlock() |
There was a problem hiding this comment.
Looks like both before and after callbacks were called within the lock before this change and now they are called outside of the lock. Do we even need 2 separate callbacks?
There was a problem hiding this comment.
we do not really need a 2 type of callbacks, one before one after, i thought providing 2 type of callbacks can be useful
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.