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

chore: Upgrade to Java 11 #393

Merged
merged 4 commits into from
Jan 10, 2024
Merged

chore: Upgrade to Java 11 #393

merged 4 commits into from
Jan 10, 2024

Conversation

tjsilver
Copy link
Contributor

@tjsilver tjsilver commented Jan 9, 2024

This is a re-attempt to upgrade Java to v11. This was attempted in #379 and reverted in #381.

Requires: https://github.com/guardian/janus/pull/3947 which should be merged first, as Java 11 is backwards compatible with Java 8.

build.sbt Show resolved Hide resolved
@@ -88,15 +88,8 @@ lazy val root = (project in file("."))
Universal / javaOptions ++= Seq(
"-Dconfig.file=/etc/gu/janus.conf", // for PROD, overridden by local sbt file
"-Dpidfile.path=/dev/null",
"-J-XX:MaxRAMFraction=2",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The error in the last attempt was thought to be caused by "-J-XX:+PrintGCDateStamps", and there were warnings caused by the RAM fraction options-J-XX:MaxRAMFraction=2 and -J-XX:InitialRAMFraction=2. The RAM fractions have been replaced with initial heap size(Xms) and max heap size (Xmx). After discussion with @adamnfish and @jacobwinch all the advanced options (-XX) were deemed unnecessary and thus removed.

@tjsilver tjsilver requested review from adamnfish, jacobwinch and a team January 9, 2024 15:46
Copy link
Contributor

@adamnfish adamnfish left a comment

Choose a reason for hiding this comment

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

As mentioned below, maybe we tweak the RAM amount to match what was there for now?

build.sbt Outdated
"-J-Xloggc:/var/log/${packageName.value}/gc.log",
"-J-XX:+UseCompressedOops",
"-J-XX:+UseStringDeduplication"
"-J-Xms2048m",
Copy link
Contributor

Choose a reason for hiding this comment

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

This application is running on an instance that has 2GB of RAM in total, so I think this value (and the one below) are too high. We need to leave some room for the OS as well! At the moment, the JVM will happily consume up to 2GB of RAM, and in doing so at some point may request memory that the OS does not have space to offer. This will cause an OOM crash.

The RAMFRaction setting was 2, which means this is currently using half the available RAM. Let's start by setting it to the same, 1024m or 1g?

@tjsilver tjsilver dismissed adamnfish’s stale review January 10, 2024 13:30

Jacob has approved

@tjsilver tjsilver merged commit 5537949 into main Jan 10, 2024
4 checks passed
@tjsilver tjsilver deleted the ts/upgrade-java branch January 10, 2024 13:31
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