-
Notifications
You must be signed in to change notification settings - Fork 812
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[history] add domain status check in taskfilter #5140
[history] add domain status check in taskfilter #5140
Conversation
a74cb9a
to
4a3bde3
Compare
@@ -514,24 +514,6 @@ func newTransferQueueStandbyProcessor( | |||
if !ok { | |||
return false, errUnexpectedQueueTask | |||
} | |||
if task.TaskType == persistence.TransferTaskTypeCloseExecution || |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These logic was put here on purpose, because we don't want to hide it
… transfer task queues
900d563
to
c9866ae
Compare
domainEntry, err := shard.GetDomainCache().GetDomainByID(domainID) | ||
if err != nil { | ||
if _, ok := err.(*types.EntityNotExistsError); ok { | ||
return true, nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be more conservative here? Normally we never delete domains
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we have a timer and we cannot find the domain, something must be wrong.
@@ -189,3 +191,20 @@ func (t *taskAllocatorImpl) Lock() { | |||
func (t *taskAllocatorImpl) Unlock() { | |||
t.locker.Unlock() | |||
} | |||
|
|||
// isDomainNotRegistered checks either if domain does not exist or is in deprecated or deleted status | |||
func isDomainNotRegistered(shard shard.Context, domainID string) (bool, error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's change this to isDomainDeprecated
What changed? add domain status check in taskAllocator verification Why? tasks in queues should drop for deprecated or deleted domains
What changed?
Why?
How did you test it?
Potential risks
Release notes
Documentation Changes