lightning: when lightning exits while processing the task, it should return 1 #53381
Conversation
Signed-off-by: zeminzhou <zhouzemin@pingcap.com>
ti-chi-bot
bot
added
do-not-merge/needs-linked-issue
do-not-merge/needs-tests-checked
release-note-none
|
Hi @zeminzhou. Thanks for your PR. PRs from untrusted users cannot be marked as trusted with I understand the commands that are listed here. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #53381 +/- ##
================================================
+ Coverage 72.5545% 74.6346% +2.0801%
================================================
Files 1505 1527 +22
Lines 429830 438838 +9008
================================================
+ Hits 311861 327525 +15664
+ Misses 98694 90740 -7954
- Partials 19275 20573 +1298
Flags with carried forward coverage won't be shown. Click here to find out more.
|
ti-chi-bot
bot
added
do-not-merge/needs-triage-completed
and removed
do-not-merge/needs-linked-issue
do-not-merge/needs-tests-checked
labels
lance6716
changed the title
| @@ -99,7 +99,9 @@ func main() { | |||
| finished := true | |||
| if common.IsContextCanceledError(err) { | |||
| err = nil | |||
| finished = false | |||
| if app.TaskCanceled() { | |||
There was a problem hiding this comment.
I see lightning CI failed. To reduce behaviour change, you can check it's ServerMode
There was a problem hiding this comment.
It was fixed, thanks! PTAL~
There was a problem hiding this comment.
For non-server mode it's a compatibility change. I need to ask PM about it tomorrow. It may break user's script.
There was a problem hiding this comment.
@zeminzhou anything affected by current behavior of lightning? any workaround?
There was a problem hiding this comment.
In cloud, when k8s' job controller reschedules lightning pod to other node(because current node is evicted), job controller will send SIGINT to lightning pod. Because lightning pod returns 0, job controller think the job is done and will not reschedule it.
There was a problem hiding this comment.
I added compatibility with the previous non-server mode. /cc @lance6716
Signed-off-by: zzm <zhouzemin@pingcap.com>
Signed-off-by: zzm <zhouzemin@pingcap.com>
Signed-off-by: zzm <zhouzemin@pingcap.com>
ti-chi-bot
bot
added
approved
needs-1-more-lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: D3Hunter, lance6716 The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
lgtm
do-not-merge/needs-tests-checked
and removed
needs-1-more-lgtm
|
there's no test, so i unchecked |
|
/test check-dev |
|
@zeminzhou: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
What problem does this PR solve?
Issue Number: close #53384
Problem Summary:
What changed and how does it work?
Check List
Tests
SIGINTto lightning serverSide effects
Documentation
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.