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

tillrohrmann commented Sep 24, 2018

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

This comment has been minimized.

Copy link
@TisonKun

TisonKun Sep 24, 2018

Contributor

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

This comment has been minimized.

Copy link
@tillrohrmann

tillrohrmann Sep 26, 2018

Author Contributor

Good point @TisonKun. Will add it.

@zentol
zentol approved these changes Sep 24, 2018
Copy link
Contributor

zentol left a comment

+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 #6742.
@tillrohrmann tillrohrmann force-pushed the tillrohrmann:failJobResult branch from 0d3c552 to b69c6eb Sep 26, 2018
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

This comment has been minimized.

Copy link
Contributor Author

tillrohrmann commented Sep 26, 2018

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 tillrohrmann:failJobResult branch Sep 27, 2018
Clarkkkkk added 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
4 participants
You can’t perform that action at this time.