Skip to content
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

[FLINK-10400] Fail JobResult if application finished in CANCELED or FAILED state #6742

Closed
wants to merge 1 commit into from

Conversation

tillrohrmann
Copy link
Contributor

What is the purpose of the change

When generating a JobExecutionResult from a JobResult, we fail the conversion if the application status was not SUCCEEDED.

In case of the CANCELED state, the client will throw an JobCancellationException.
In case of the FAILED state, the client will throw an JobExecutionException.

Brief change log

  • Add ApplicationStatus field to JobResult
  • Throw JobCancellationException if ApplicationStatus was CANCELED
  • Throw JobExecutionException if ApplicationStatus was FAILED

Verifying this change

  • Added JobResultTest#testCancelledJobIsFailureResult, #testFailedJobIsFailureResult, #testCancelledJobThrowsJobCancellationException and #testFailedJobThrowsJobExecutionException.

Does this pull request potentially affect one of the following parts:

  • Dependencies (does it add or upgrade a dependency): (no)
  • The public API, i.e., is any changed class annotated with @Public(Evolving): (no)
  • The serializers: (no)
  • The runtime per-record code paths (performance sensitive): (no)
  • Anything that affects deployment or recovery: JobManager (and its components), Checkpointing, Yarn/Mesos, ZooKeeper: (no)
  • The S3 file system connector: (no)

Documentation

  • Does this pull request introduce a new feature? (no)
  • If yes, how is the feature documented? (not applicable)

@@ -108,22 +118,39 @@ public long getNetRuntime() {
*
* @param classLoader to use for deserialization
* @return JobExecutionResult
* @throws WrappedJobException if the JobResult contains a serialized exception
* @throws JobExecutionException if the job execution did not succeed
Copy link
Member

Choose a reason for hiding this comment

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

this method might throws JobCancellationException if the job is cancelled, should be documented.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point @tisonkun. Will add it.

Copy link
Contributor

@zentol zentol left a comment

Choose a reason for hiding this comment

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

+1

…AILED state

In case of the CANCELED state, the client will throw an JobCancellationException.
In case of the FAILED state, the client will throw an JobExecutionException.

This closes apache#6742.
tillrohrmann added a commit to tillrohrmann/flink that referenced this pull request Sep 26, 2018
…AILED state

In case of the CANCELED state, the client will throw an JobCancellationException.
In case of the FAILED state, the client will throw an JobExecutionException.

This closes apache#6742.
tillrohrmann added a commit to tillrohrmann/flink that referenced this pull request Sep 26, 2018
…AILED state

In case of the CANCELED state, the client will throw an JobCancellationException.
In case of the FAILED state, the client will throw an JobExecutionException.

This closes apache#6742.
@tillrohrmann
Copy link
Contributor Author

Thanks for the review @tisonkun and @zentol. Merging this PR once Travis gives green light (after rebasing).

asfgit pushed a commit that referenced this pull request Sep 27, 2018
…AILED state

In case of the CANCELED state, the client will throw an JobCancellationException.
In case of the FAILED state, the client will throw an JobExecutionException.

This closes #6742.
@asfgit asfgit closed this in 2f88d42 Sep 27, 2018
@tillrohrmann tillrohrmann deleted the failJobResult branch September 27, 2018 10:05
Clarkkkkk pushed a commit to Clarkkkkk/flink that referenced this pull request Mar 7, 2019
…AILED state

In case of the CANCELED state, the client will throw an JobCancellationException.
In case of the FAILED state, the client will throw an JobExecutionException.

This closes apache#6742.

(cherry picked from commit 2f88d42)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants