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

Restart test jvm every few test classes #6093

Merged
merged 3 commits into from
Nov 6, 2019
Merged

Conversation

lbergelson
Copy link
Member

  • Adding forkEvery 5 to cause the test jvm to be restarted every few test classes.
  • This might fix a theoretical memory leak causing our builds to fail often.

This might help?

Copy link
Collaborator

@cmnbroad cmnbroad left a comment

Choose a reason for hiding this comment

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

Seems worth a shot (assuming tests pass).

@cmnbroad
Copy link
Collaborator

@lbergelson we might want to monitor how much this affects test time too. Maybe we should use a bigger number ?

@lbergelson
Copy link
Member Author

It looks like it made the tests substantially slower.... I'm not totally clear on why. Maybe because it has to re-optimize code every time it restarts the jvm.

@cmnbroad
Copy link
Collaborator

I'd try 100 or even 200.

@lbergelson
Copy link
Member Author

Unless I misunderstand, it's forking every 5 test classes not every 5 tests. So I'm surprised it's having such an impact. We only have a few hundred test classes, so I'm not sure 100 or 200 is going to do anything. I'll try 50 and see what it looks like time wise.

@@ -9,7 +9,7 @@ tasks.withType(Test) {
* anything else : run the non-cloud tests
*/
String TEST_TYPE = "$System.env.TEST_TYPE"

forkEvery 50
Copy link
Collaborator

Choose a reason for hiding this comment

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

50 seems much better, though its a little hard to tell since there is so much variability in the normal case.

@lbergelson
Copy link
Member Author

It still looks like it's adding a few minutes to the times... Are we still having a lot of a failures? I haven't been noticing them.

@droazen
Copy link
Contributor

droazen commented Oct 30, 2019

@lbergelson Can we try this branch with "forkEvery" set to something high like 100 and see if it resolves the currently crop of intermittent Java 11 memory errors without adding to the runtime too much?

* Adding forkEvery 5 to cause the test jvm to be restarted every few test classes.
* This might fix a theoretical memory leak causing our builds to fail often.
@lbergelson lbergelson merged commit 3248594 into master Nov 6, 2019
@lbergelson lbergelson deleted the lb_try_forking_jvm branch November 6, 2019 19:49
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.

3 participants