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

Graal support #2533

Merged
merged 58 commits into from
Sep 19, 2024
Merged

Graal support #2533

merged 58 commits into from
Sep 19, 2024

Conversation

timyates
Copy link
Collaborator

@timyates timyates commented Jun 28, 2024

Closes #2532

Instructions for local building

  • To compile, install a GraalVM JDK for Java 21 (i.e. sdk install java 21.0.3-graal)

  • Run ./gradlew nativeCompile

  • Create a nativeRun.sh script which is a copy of run.sh but with

    ./server/build/native/nativeCompile/check-ins
    

    In place of ./gradlew run at the end

Usage on Cloud-run

I propose for cloud-run we make use of ./gradlew dockerBuildNative.
This PR has updated the Gradle file to configure the image name when executed via Github actions to be the same as the current process...
We should be able to just replace the Build the Docker image step in the actions with one to run this task.

Tests

  • When using the GraalVM JDK (above), you can run ./gradlew nativeTest to run the tests against a native binary.
  • 22 tests have been disabled when run this way via the @DisabledInNativeImage annotation
  • This is because I cannot get the mocking working when run natively (not sure it's even possible)
  • Raised Investigate reducing the usage of Mockito #2534 to investigate replacing some of this mocking

@timyates timyates self-assigned this Jun 28, 2024
@timyates timyates marked this pull request as ready for review July 3, 2024 08:31
@timyates timyates changed the title WIP: Graal support Graal support Jul 3, 2024

// When we are building on CI, we do not want to include the dev migrations.
def isCI = System.getenv("CI") != null
def migrationLocations = isCI ? "db/common" : "db/common,db/dev"
Copy link
Member

Choose a reason for hiding this comment

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

What if we are building for the dev environment? We run the dev migrations there.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good call...

I'm wondering whether we should pivot to simply use dockerBuildNative on CI to generate the docker image automatically for us...

I've pushed a commit which does this (and tests the image building works on CI), and also switched to check the SERVICE_NAME and var (and disable the dev db load when it exists and equals checkins-master)

@timyates
Copy link
Collaborator Author

@mkimberlin I have aligned both the gradle-deploy-develop.yml and gradle-deploy-native-develop.yml actions

They currently both also trigger on pushes to the 'feature-2532/graal' branch (this branch) for testing, but we probably want to get rid of that....

Should the deploy job depend on the test job passing?

We could just inline the Run tests with gradle step if so, but we may already be running enough testing before it gets to the develop branch 🤔

I tried to pull commonality out into separate actions, and stitch them together as a github workflow-run, but I could not work out how to get the docker auth to work when it was run in a separate action, so I rolled it back and went with this...

It may be worth pulling the URL out into the env and referencing it with ${{ env.URL }} as currently each action references it 3 times, and they all need to be the same

@mkimberlin
Copy link
Member

@mkimberlin I have aligned both the gradle-deploy-develop.yml and gradle-deploy-native-develop.yml actions

They currently both also trigger on pushes to the 'feature-2532/graal' branch (this branch) for testing, but we probably want to get rid of that....

Agreed.

Should the deploy job depend on the test job passing?

Let's leave it as is for the moment. Broken code in dev doesn't worry me too much. Rollback is pretty easy, and this allows us to validate while the test suite is running.

We could just inline the Run tests with gradle step if so, but we may already be running enough testing before it gets to the develop branch 🤔

It was inline, but I pulled it out because of how long the tests take.

I tried to pull commonality out into separate actions, and stitch them together as a github workflow-run, but I could not work out how to get the docker auth to work when it was run in a separate action, so I rolled it back and went with this...

I went down a similar path at one point and just hit eject on that.

It may be worth pulling the URL out into the env and referencing it with ${{ env.URL }} as currently each action references it 3 times, and they all need to be the same

Agreed on that, too. I thought about that after we got off of stand-up earlier and then neglected to mention it in chat.

@timyates
Copy link
Collaborator Author

Ok, extracted the URL in the actions to an ENV var TARGET_URL

There's an introspection issue with the native tests which I'm looking into now

We don't need to assemble, or build a docker image, or cache sonar things (as we're not doing sonar)
@timyates
Copy link
Collaborator Author

I have removed the feature-2532/graal branch from the deployment actions, so they now only run on develop 👍

@mkimberlin mkimberlin merged commit 93c43b7 into develop Sep 19, 2024
6 checks passed
@mkimberlin mkimberlin deleted the feature-2532/graal branch September 19, 2024 14:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use GraalVM to create a native image
2 participants