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

[RunAllTests] Fix part of #2844: work around Bazel-specific CI issue that's causing an OpenJDK crash/flake #2846

Merged
merged 5 commits into from
Apr 1, 2021

Conversation

BenHenning
Copy link
Member

@BenHenning BenHenning commented Mar 6, 2021

Fix part of #2844.

This attempts to fix part of #2844 by normalizing test compiling/running on JDK 9 (for parity with Gradle tests--see #2845), and by working around what appears to be a resource constraint issue Bazel is hitting. For the latter, see bazelbuild/bazel#3236 (comment). All configuration changes in this PR are done via changes to the CI environment's ~/.bazelrc file, so nothing here should affect local builds.

Note that configuring Java versions in Bazel is complicated. In particular, at any time there can be 5 different versions of Java at play:

  1. The version of Java used to build sources (defaults to a remote JDK for build reproducibility). Not sure what the default is here, but it seems to be 11.
  2. The version of Java used to run tests/binaries (defaults to the local environment's JDK as determined by $JAVA_HOME).
  3. The version of Java used to build tools used by Bazel. Defaults to a remote JDK 11.
  4. The version of Java used to run tools used during the build. Defaults to local JDK 11 (if available--not sure what happens if it's not available; it might use the Java version in (5)).
  5. The version of Java used to run Bazel itself. This defaults to an embedded JDK 11.

Now, from what I can tell, I think we only need to configure (1) since (2) should already be using the correct version of Java (JDK 9). We do want the majority of the build to be on JDK 9 since that's what we're running on, but this isn't super important. See also https://docs.bazel.build/versions/master/bazel-and-java.html#configuring-the-jdk & bazelbuild/bazel#5594 for more details on how this Java resolution works.

Finally, similar to #2845 this cannot be observed as "correct" until we observe the flake itself disappearing over the next few weeks.

Update Bazel builds to use faster tools repository, and to use JDK 9 for building.
Remove tools override since it's now part of ~/.bazelrc.
@BenHenning BenHenning changed the title Update unit_tests.yml [RunAllTests] Update unit_tests.yml Mar 7, 2021
Attempt to use the solution described in: bazelbuild/bazel#3236 (comment).
@BenHenning BenHenning changed the title [RunAllTests] Update unit_tests.yml [RunAllTests] Fix part of #2844: work around Bazel-specific CI issue that's causing an OpenJDK crash/falke Mar 7, 2021
@BenHenning BenHenning changed the title [RunAllTests] Fix part of #2844: work around Bazel-specific CI issue that's causing an OpenJDK crash/falke [RunAllTests] Fix part of #2844: work around Bazel-specific CI issue that's causing an OpenJDK crash/flake Mar 7, 2021
@BenHenning BenHenning marked this pull request as ready for review March 8, 2021 17:54
@vinitamurthi
Copy link
Contributor

Is this still required?

@BenHenning
Copy link
Member Author

@vinitamurthi this is expected to help, but it may not be sufficient to fully fix the Bazel-caused flakes.

@BenHenning BenHenning removed their assignment Mar 24, 2021
@vinitamurthi vinitamurthi removed their assignment Mar 28, 2021
Copy link
Contributor

@anandwana001 anandwana001 left a comment

Choose a reason for hiding this comment

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

LGTM

Conflicts:
	.github/workflows/build_tests.yml
	.github/workflows/unit_tests.yml
@BenHenning
Copy link
Member Author

Thanks all! Enabling auto-merge after resolving conflicts since some of the changes here may help with work I'm doing in #1861 to make Bazel CI runs a bit faster.

@BenHenning BenHenning enabled auto-merge (squash) March 31, 2021 23:42
@BenHenning BenHenning merged commit 7de212e into develop Apr 1, 2021
@BenHenning BenHenning deleted the move-bazel-ci-to-java-9 branch April 1, 2021 00:14
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