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
Include TASK_FINISHED as failure state when upgrading #4865
Conversation
Can one of the admins verify this patch? |
1 similar comment
Can one of the admins verify this patch? |
Issue #4866 |
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.
Looks good to me. @zen-dog you are on call next, could take a look as well?
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.
Looks good. Minor change requested.
@@ -138,7 +138,7 @@ object TaskReplaceActor { | |||
private[this] val log = LoggerFactory.getLogger(getClass) | |||
|
|||
val KillComplete = "^TASK_(ERROR|FAILED|FINISHED|LOST|KILLED)$".r |
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.
Since KillComplete
and FailedToStart
are now equal, I would use one and call it smth. like IsTerminal
.
If we upgrade an app with health check, and newly launched task finished before passing health check, the current task replace actor ignore this TASK_FINISHED message and run into a strange state: 1. it waits for task status update event, but it won't get any; 2. it won't try to launch a new task.
130bc0e
to
72e2151
Compare
updated, replace FailedToStart and KillComplete with IsTerminal |
ok to test |
test this please |
retest this please |
If we upgrade an app with health check, and newly launched task
finished before passing health check, the current task replace actor
ignore this TASK_FINISHED message and run into a strange state: