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

[LIVY-620][LIVY-641] Fix travis failed on test: should end with status dead when batch session exits with no 0 return code #214

Closed
wants to merge 1 commit into from

Conversation

runzhiwang
Copy link
Contributor

@runzhiwang runzhiwang commented Aug 26, 2019

What changes were proposed in this pull request?

Fix travis failed on test: should end with status dead when batch session exits with no 0 return code

  1. Travis failed because of thread in SparkProcApp.scala and thead in BatchSession.scala change SessionState to different value when stopSession in test.

  2. The changes of BatchSession.scala is to revert the commit of 01da43d.

  3. The changes of SparkYarnApp.scala and BatchSessionSpec.scala are the new changes

How was this patch tested?

  1. Existed UT and IT.
  2. Create Batch Session In Yarn and kill SparkSubmit, then check the SessionState.

@runzhiwang runzhiwang changed the title [LIVY-641] Fix travis failed on test: should end with status dead when batch session exits with no 0 return code [LIVY-620][LIVY-641] Fix travis failed on test: should end with status dead when batch session exits with no 0 return code Aug 26, 2019
@runzhiwang
Copy link
Contributor Author

@gumartinm Hi, Could you help review this PR?

@jerryshao
Copy link
Contributor

Looks like there's some issues in IT, can you please check?

@runzhiwang
Copy link
Contributor Author

runzhiwang commented Aug 26, 2019

@jerryshao UT and IT have passed.

@codecov-io
Copy link

codecov-io commented Aug 26, 2019

Codecov Report

Merging #214 into master will decrease coverage by 0.04%.
The diff coverage is 33.33%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #214      +/-   ##
============================================
- Coverage     68.76%   68.71%   -0.05%     
+ Complexity      916      915       -1     
============================================
  Files           100      100              
  Lines          5696     5700       +4     
  Branches        862      864       +2     
============================================
  Hits           3917     3917              
- Misses         1219     1225       +6     
+ Partials        560      558       -2
Impacted Files Coverage Δ Complexity Δ
...la/org/apache/livy/server/batch/BatchSession.scala 86.17% <ø> (-0.43%) 14 <0> (-2)
...ain/scala/org/apache/livy/utils/SparkYarnApp.scala 74.49% <33.33%> (-5.09%) 34 <1> (+1)
...ain/java/org/apache/livy/rsc/driver/RSCDriver.java 78.81% <0%> (-0.43%) 41% <0%> (-1%)
...main/scala/org/apache/livy/server/LivyServer.scala 35.96% <0%> (+0.49%) 11% <0%> (ø) ⬇️
...c/main/scala/org/apache/livy/repl/ReplDriver.scala 33.33% <0%> (+2.56%) 7% <0%> (ø) ⬇️
...cala/org/apache/livy/scalaapi/ScalaJobHandle.scala 55.88% <0%> (+2.94%) 7% <0%> (ø) ⬇️
...in/java/org/apache/livy/rsc/driver/JobWrapper.java 88.57% <0%> (+5.71%) 9% <0%> (+1%) ⬆️
...la/org/apache/livy/utils/LineBufferedProcess.scala 85.71% <0%> (+7.14%) 8% <0%> (+1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3626382...d1b1cf5. Read the comment docs.

@jerryshao
Copy link
Contributor

jerryshao commented Aug 27, 2019

So the fix here is just reverting the previous commit, or you add some new changes? If it is reverted, then did you fix the previous issue?

@runzhiwang
Copy link
Contributor Author

@jerryshao The changes in SparkYarnApp.scala and BatchSessionSpec.scala are the new changes. The new changes have fixed the previous issue.

Copy link
Contributor

@jerryshao jerryshao left a comment

Choose a reason for hiding this comment

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

LGTM.

@jerryshao jerryshao closed this Aug 27, 2019
@jerryshao jerryshao reopened this Aug 27, 2019
@jerryshao jerryshao closed this in 8b92397 Aug 27, 2019
@runzhiwang runzhiwang deleted the LIVY-641-SESSION-STATUS branch August 27, 2019 06:12
captainzmc pushed a commit to captainzmc/incubator-livy that referenced this pull request Sep 6, 2019
…s dead when batch session exits with no 0 return code

## What changes were proposed in this pull request?

Fix travis failed on test: should end with status dead when batch session exits with no 0 return code

1. Travis failed because of thread in SparkProcApp.scala and thead in BatchSession.scala change SessionState to different value when stopSession in test.

2. The changes of BatchSession.scala is to revert the commit of apache@01da43d.

3. The changes of SparkYarnApp.scala and BatchSessionSpec.scala are the new changes

## How was this patch tested?

1. Existed UT and IT.
2. Create Batch Session In Yarn and kill SparkSubmit, then check the SessionState.

Author: runzhiwang <runzhiwang@tencent.com>

Closes apache#214 from runzhiwang/LIVY-641-SESSION-STATUS.
captainzmc pushed a commit to captainzmc/incubator-livy that referenced this pull request Sep 11, 2019
…s dead when batch session exits with no 0 return code

## What changes were proposed in this pull request?

Fix travis failed on test: should end with status dead when batch session exits with no 0 return code

1. Travis failed because of thread in SparkProcApp.scala and thead in BatchSession.scala change SessionState to different value when stopSession in test.

2. The changes of BatchSession.scala is to revert the commit of apache@01da43d.

3. The changes of SparkYarnApp.scala and BatchSessionSpec.scala are the new changes

## How was this patch tested?

1. Existed UT and IT.
2. Create Batch Session In Yarn and kill SparkSubmit, then check the SessionState.

Author: runzhiwang <runzhiwang@tencent.com>

Closes apache#214 from runzhiwang/LIVY-641-SESSION-STATUS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants