Skip to content

control-service: Classify OOM as User Errors - #479

Merged
doks5 merged 1 commit into
mainfrom
person/andonova/fix-oom-error
Nov 9, 2021
Merged

control-service: Classify OOM as User Errors#479
doks5 merged 1 commit into
mainfrom
person/andonova/fix-oom-error

Conversation

@doks5

@doks5 doks5 commented Nov 4, 2021

Copy link
Copy Markdown
Contributor

Currently, if a data job execution exceeds the allowed memory
quota, the pod of the execution is immediately killed by
Kubernetes and restarted. Which in turn usually results in a
similar failure. This causes a Platform Error to be raised, whereas
it should be classified as a User Error.

This change re-classifies such errors as User Errors.

Testing Done: Added unit test.

Signed-off-by: Andon Andonov andonova@vmware.com

@doks5 doks5 linked an issue Nov 4, 2021 that may be closed by this pull request
Classify job failures due to OOM as User Error #410
Closed
antoniivanov
antoniivanov approved these changes Nov 5, 2021
View reviewed changes

@antoniivanov antoniivanov left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Let's merge this. But we need to make sure we can report to the user what is going on correctly (see my other comment). We can handle this in separate PR.

Comment thread ...rol_service/src/main/java/com/vmware/taurus/service/execution/JobExecutionResultManager.java Outdated
@tpalashki

tpalashki commented Nov 5, 2021

Copy link
Copy Markdown
Contributor

Are you sure that the reason field of the Kubernetes job, in this case, will be "OOMKilled"? I think the job will have a status.condition[0].reason: BackoffLimitExceeded and the OOMKilled will be set as a status of the pod itself:

status:
  containerStatuses:
  - ...
    lastState:
      terminated:
        containerID: ...
        exitCode: 137
        finishedAt: "2021-10-31T09:55:22Z"
        reason: OOMKilled
        startedAt: "2021-10-31T09:47:17Z"

@doks5 doks5 self-assigned this Nov 5, 2021
@doks5
doks5 force-pushed the person/andonova/fix-oom-error branch 3 times, most recently from 95bf58e to ec81690 Compare November 8, 2021 08:58
tpalashki
tpalashki reviewed Nov 8, 2021
View reviewed changes
Comment thread ...rol_service/src/main/java/com/vmware/taurus/service/execution/JobExecutionResultManager.java Outdated
Comment thread ...cts/pipelines_control_service/src/main/java/com/vmware/taurus/service/KubernetesService.java Outdated
@doks5
doks5 force-pushed the person/andonova/fix-oom-error branch from ec81690 to 64935b2 Compare November 8, 2021 11:41
@doks5 doks5 mentioned this pull request Nov 8, 2021
Closed
@doks5
doks5 force-pushed the person/andonova/fix-oom-error branch from 64935b2 to c54d9ed Compare November 8, 2021 14:54
@doks5
doks5 requested a review from tpalashki November 8, 2021 16:10
tpalashki
tpalashki reviewed Nov 8, 2021
View reviewed changes
Comment thread ...cts/pipelines_control_service/src/main/java/com/vmware/taurus/service/KubernetesService.java Outdated
Comment thread ...rol_service/src/main/java/com/vmware/taurus/service/execution/JobExecutionResultManager.java
@doks5
doks5 force-pushed the person/andonova/fix-oom-error branch from c54d9ed to 5535add Compare November 8, 2021 16:31
@doks5
doks5 requested a review from tpalashki November 9, 2021 05:47
tpalashki
tpalashki approved these changes Nov 9, 2021
View reviewed changes
Comment thread ...cts/pipelines_control_service/src/main/java/com/vmware/taurus/service/KubernetesService.java
@doks5
doks5 force-pushed the person/andonova/fix-oom-error branch from 5535add to e8891c2 Compare November 9, 2021 08:15
@doks5
control-service: Classify OOM as User Errors
4c1359d 
Currently, if a data job execution exceeds the allowed memory
quota, the pod of the execution is immediately killed by
Kubernetes and restarted. Which in turn usually results in a
similar failure. This causes a Platform Error to be raised, whereas
it should be classified as a User Error.

This change re-classifies such errors as User Errors.

Testing Done: Added unit test.

Signed-off-by: Andon Andonov <andonova@vmware.com>
@doks5
doks5 force-pushed the person/andonova/fix-oom-error branch from e8891c2 to 4c1359d Compare November 9, 2021 08:48
@doks5
doks5 merged commit 13d9e37 into main Nov 9, 2021
@doks5
doks5 deleted the person/andonova/fix-oom-error branch November 9, 2021 09:10
@exalate-issue-sync exalate-issue-sync Bot mentioned this pull request Aug 26, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

3 more reviewers
@mivanov1988 mivanov1988 mivanov1988 left review comments
@antoniivanov antoniivanov antoniivanov approved these changes
@tpalashki tpalashki tpalashki approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

@doks5 doks5

Labels

cla-not-required

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Classify job failures due to OOM as User Error

5 participants

@doks5 @tpalashki @antoniivanov @mivanov1988 @vmwclabot