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

update support-workers to java21 runtime #5792

Merged
merged 1 commit into from
Mar 13, 2024
Merged

Conversation

johnduffell
Copy link
Member

This PR bumps support workers to java 21. This is supposed to give us performance improvements.
https://aws.amazon.com/blogs/compute/java-17-runtime-now-available-on-aws-lambda/
https://aws.amazon.com/about-aws/whats-new/2023/11/aws-lambda-support-java-21/

We are using scala 2.13.12 which is compatible with JRE21
https://docs.scala-lang.org/overviews/jdk-compatibility/overview.html

I have tested in CODE and looking at one lambda (create payment method) you can clearly see the performance improvement for the cold start
image

This is replicated for the whole state machine execution, although it is diluted by the different paths it can take through the state machine.
image

Copy link
Contributor

Size Change: 0 B

Total Size: 1.78 MB

ℹ️ View Unchanged
Filename Size
./public/compiled-assets/javascripts/ausMomentMap.js 107 kB
./public/compiled-assets/javascripts/contributionsRedirectStyles.js 20 B
./public/compiled-assets/javascripts/digitalSubscriptionLandingPage.js 216 kB
./public/compiled-assets/javascripts/downForMaintenancePage.js 67.7 kB
./public/compiled-assets/javascripts/error404Page.js 67.7 kB
./public/compiled-assets/javascripts/error500Page.js 67.6 kB
./public/compiled-assets/javascripts/favicons.js 616 B
./public/compiled-assets/javascripts/paperSubscriptionCheckoutPage.js 168 kB
./public/compiled-assets/javascripts/paperSubscriptionLandingPage.js 84.4 kB
./public/compiled-assets/javascripts/payPalErrorPage.js 66.2 kB
./public/compiled-assets/javascripts/payPalErrorPageStyles.js 20 B
./public/compiled-assets/javascripts/promotionTerms.js 70.9 kB
./public/compiled-assets/javascripts/subscriptionsLandingPage.js 70 kB
./public/compiled-assets/javascripts/subscriptionsRedemptionPage.js 124 kB
./public/compiled-assets/javascripts/supporterPlusLandingPage.js 277 kB
./public/compiled-assets/javascripts/unsupportedBrowserStyles.js 20 B
./public/compiled-assets/javascripts/weeklySubscriptionCheckoutPage.js 185 kB
./public/compiled-assets/javascripts/weeklySubscriptionLandingPage.js 84.6 kB
./public/compiled-assets/webpack/385.js 81.8 kB
./public/compiled-assets/webpack/67.js 18.1 kB
./public/compiled-assets/webpack/735.js 2 kB
./public/compiled-assets/webpack/775.js 18 kB
./public/compiled-assets/webpack/991.js 2.16 kB

compressed-size-action

Copy link
Member

@tomrf1 tomrf1 left a comment

Choose a reason for hiding this comment

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

👏

@johnduffell johnduffell merged commit 14abd31 into main Mar 13, 2024
12 checks passed
@johnduffell johnduffell deleted the jd-java21-workers branch March 13, 2024 11:40
@prout-bot
Copy link

Seen on PROD (merged by @johnduffell 13 minutes and 20 seconds ago)

Sentry Release: support-client-side, support

@johnduffell
Copy link
Member Author

in PROD you can see the improvement in cold starts,
image
although there's one outlier which was quicker just before the deploy. This does sometimes happen as you can see also on the extreme left of the graph.

@johnduffell
Copy link
Member Author

still looking good overnight
image

rtyley added a commit to guardian/gha-scala-library-release-workflow that referenced this pull request Apr 30, 2024
This allows projects to specify, with an `asdf` (https://asdf-vm.com/)-formatted
`.tool-versions` file, the Java version to be used by the workflow for building
the project- `gha-scala-library-release-workflow` has always used a recent LTS
version Java for the build (eg Java 17), and this has sometimes been _too recent_
(guardian/atom-maker#94) for some projects at the Guardian.

We _want_ projects to update to Java 21 (see guardian/support-frontend#5792,
guardian/scala-steward-public-repos#67 etc), but tying
dozens of projects tightly to what `gha-scala-library-release-workflow` is using
will make updating that version pretty hard. If, at some later point, _some_ projects
want to experiment with Java 25, we shouldn't have to force all other projects to
be compatible with that first.

## How to express the version of Java required...

### Configuration proliferation is a problem...

One option would have been to simply add a new input parameter to the workflow,
specifying the required Java version, but that has a downside: it proliferates the
number of places in a project where the desired Java version is declared - and there
are many in use at the Guardian, with no well-agreed canonical source-of-truth:

* GHA `ci.yml`, in the [`java-version`](https://github.com/guardian/etag-caching/blob/7ecc04981f5a42a0f2ecb10631f28da571a49836/.github/workflows/ci.yml#L22) field of `actions/setup-java` - this has been my favourite in the past, as whatever CI runs with is usually pretty close to the truth
* In sbt, the `scalacOptions` of `-target`, `-release`, `-java-output-version` (currently `-release` preferred, tho' once [support](scala/scala#10654) is pervasive, `-java-output-version` is probably best) and the `javacOptions` of `-target` & `-source` - these all effectively set lower-bounds on the version of Java supported, rather than guaranteeing a minimum upper bound of Java version the way CI does.
* In apps running on Amigo AMI images; the Java version baked into the AMI, usually [referenced](https://github.com/guardian/mobile-apps-api/blob/3231e6bf064163c6d0e72c8dc862678c68eb0b62/mobile-fronts/conf/riff-raff.yaml#L10) by `riff-raff.yaml`
* In AWS Lambdas; the cloudformation [`Runtime`](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/aws-resource-lambda-function.html#cfn-lambda-function-runtime) parameter, often set [by CDK](https://github.com/guardian/mobile-save-for-later/blob/1ac12e4c0100edb976ebae9e2a9975ad2321e26e/cdk/lib/mobile-save-for-later.ts#L44)

### ...`asdf`'s `.tool-versions` flle offers some consolidation

Benefits of using `.tool-versions`:

* Developers will automatically get the right Java version if they have `asdf` installed
* actions/setup-java#606 has added early support for reading the version of Java used in CI by `setup-java` from the `.tool-versions` flle - unfortunately, it's of limited use to us at the moment because of actions/setup-java#615, but it's a promising start _(`setup-java` also has support for `jEnv` files, but they are not commonly used at the Guardian)_
* Format of the file is simple enough that it can be easily understood and used, even if `asdf` is not in use - **including the file doesn't _force_ developers to use `asdf`** (I know some devs have been having some problems with it, and maybe there are/will be better alternatives out there), but it clearly documents the Java version to be used.

This does mean that **all library repos need to add a `.tool-versions` file** before this PR is merged. Helpful error messaging has been added with this PR to handle cases where the file is missing or invalid:

#### Only Java _major_ version is guaranteed

Note that although `asdf` requires a fully-specified Java version (eg `21.0.3.9.1`) in the `.tool-versions` file, currently the workflow will only match the *major* version of Java specified in the file (eg `21`), and will _always_ use the AWS Corretto distribution of Java. This is due to [limitations](actions/setup-java#615) in [`actions/setup-java`](https://github.com/actions/setup-java).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants