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-647][TEST]Fix travis failed on batch session should not be gc-ed until application is finished #222

Closed
wants to merge 1 commit into from

Conversation

runzhiwang
Copy link
Contributor

@runzhiwang runzhiwang commented Sep 2, 2019

What changes were proposed in this pull request?

Fix travis failed on "batch session should not be gc-ed until application is finished"

The cause of failed is as follows:

  1. When create BatchSessionManager, the GarbageCollector thread will be created, which collect garbage according to session.state. However the session was mocked, and the test thread execute doReturn(s).when(session).state several times, if the test thread do half of the statement doReturn(s).when(session).state, and GarbageCollector thread check session.state at this time, the exception will threw by GarbageCollector thread.

  2. So the fix avoid executing doReturn(s).when(session).state after session has been registered into SessionManager.

How was this patch tested?

Existed UT and IT.

@codecov-io
Copy link

codecov-io commented Sep 2, 2019

Codecov Report

Merging #222 into master will increase coverage by 0.41%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #222      +/-   ##
============================================
+ Coverage     68.34%   68.75%   +0.41%     
- Complexity      919      923       +4     
============================================
  Files           100      100              
  Lines          5715     5742      +27     
  Branches        867      884      +17     
============================================
+ Hits           3906     3948      +42     
+ Misses         1242     1234       -8     
+ Partials        567      560       -7
Impacted Files Coverage Δ Complexity Δ
...main/scala/org/apache/livy/server/LivyServer.scala 35.96% <0%> (+0.49%) 11% <0%> (ø) ⬇️
...ain/java/org/apache/livy/rsc/driver/RSCDriver.java 80.08% <0%> (+2.11%) 42% <0%> (+1%) ⬆️
...c/main/scala/org/apache/livy/repl/ReplDriver.scala 33.33% <0%> (+2.56%) 7% <0%> (ø) ⬇️
.../scala/org/apache/livy/sessions/SessionState.scala 61.11% <0%> (+2.77%) 2% <0%> (ø) ⬇️
...cala/org/apache/livy/scalaapi/ScalaJobHandle.scala 55.88% <0%> (+2.94%) 7% <0%> (ø) ⬇️
...apache/livy/server/batch/BatchSessionServlet.scala 95.45% <0%> (+4.27%) 3% <0%> (ø) ⬇️
...ain/scala/org/apache/livy/utils/SparkYarnApp.scala 70.58% <0%> (+5.22%) 40% <0%> (ø) ⬇️
...in/java/org/apache/livy/rsc/driver/JobWrapper.java 88.57% <0%> (+5.71%) 9% <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 d0d8028...fa21b32. Read the comment docs.

@runzhiwang runzhiwang changed the title [LIVY-647]Fix travis failed on batch session should not be gc-ed until application is finished [LIVY-647][WIP]Fix travis failed on batch session should not be gc-ed until application is finished Sep 2, 2019
@runzhiwang runzhiwang force-pushed the LIVY-647 branch 2 times, most recently from cabea88 to b019117 Compare September 3, 2019 02:19
@runzhiwang runzhiwang changed the title [LIVY-647][WIP]Fix travis failed on batch session should not be gc-ed until application is finished [LIVY-647]Fix travis failed on batch session should not be gc-ed until application is finished Sep 3, 2019
@jerryshao
Copy link
Contributor

Run the test again.

@jerryshao jerryshao closed this Sep 4, 2019
@jerryshao jerryshao reopened this Sep 4, 2019
@jerryshao jerryshao closed this Sep 4, 2019
@jerryshao jerryshao reopened this Sep 4, 2019
@jerryshao jerryshao closed this Sep 5, 2019
@jerryshao jerryshao reopened this Sep 5, 2019
@runzhiwang runzhiwang changed the title [LIVY-647]Fix travis failed on batch session should not be gc-ed until application is finished [LIVY-647][TEST]Fix travis failed on batch session should not be gc-ed until application is finished Sep 6, 2019
@jerryshao jerryshao closed this Sep 6, 2019
@jerryshao jerryshao reopened this Sep 6, 2019
@jerryshao jerryshao closed this Sep 6, 2019
@jerryshao jerryshao reopened this Sep 6, 2019
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. Merging to master branch.

@jerryshao jerryshao closed this in a9b2ddb Sep 17, 2019
@runzhiwang runzhiwang deleted the LIVY-647 branch November 9, 2019 06:23
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