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

Updates so everything compiles with GraalVM #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

mraible
Copy link
Owner

@mraible mraible commented Jun 11, 2023

No description provided.

@atomfrede
Copy link

Just for reference the upstream issue jhipster/generator-jhipster#22488.

Regarding h2 + native, I remember we had a similar problem with the native blueprint too, but I am not sure anymore how we fixed it (or just switched to psql for everything). When using native support for 2.x we needed to switch to tomcat as undertow was not compatible at that time, but I guess that is fixed.

@hide212131
Copy link

hide212131 commented Jun 18, 2023

While there are quite a few conditions attached, I managed to start it "for the time being". Here are the changes I've made.

I'll write down how I avoided errors up to the launch.

  • The --enable-http argument in native-compile seemed necessary. I added it to the pom.xml (though it might not be necessary).

    <buildArgs>
        <buildArgss>--enable-http</buildArgss>
    </buildArgs>
  • The H2 error can be circumvented by fixing the profile to dev, which allows the class targeted by Class.forName() to be found.

    ./mvnw -Pdev -Pnative native:compile -DskipTests
    
  • The errors with Hibernate and Liquibase can be circumvented by manually creating configuration files like reflect-config.json. After activating the agent's trace with the following environment variable setting and running through the CURD process from startup with the non-native ./mvnw application, the main class call information is traced. Then, you create folders that correspond to the artifacts, such as src/main/resources/META-INF/native-image/org.liquibase/liquibase-core, and manually store the resulting configuration files in them (note that these files are overly redundant and need to be organized).

    export JAVA_TOOL_OPTIONS="-agentlib:native-image-agent=config-output-dir=./config-output-dir-{pid}-{datetime}/"
    
  • Unfortunately, even after doing this, Hibernate and Liquibase are still not running. However, if you create a DB schema once with non-native ./mvnw, you can start it without errors by executing target/blog.

The operation is extremely unstable. Sometimes http://localhost:8080/ succeeds and sometimes fails. Via http://localhost:4200/ with npm start, you can fetch a list of Tags from the DB, but Blogs and Posts don't work.

I'll continue to investigate this issue.
I hope this helps, even if it's just a little.

@atomfrede
Copy link

Good News! With boot 3 you can define the needed meta data in code instead of json files.

@hide212131
Copy link

hide212131 commented Jun 22, 2023

I have made some progress since the previous stage.

Here are the changes I've made.
Here are the changes I've made from the beginning.

  • During the first startup, initialization by Liquibase and Hibernate seems to have gone well.
  • Besides Tag, it appears that CRUD operations for Blog and Post are working fine.
  • Except for the metrics view (which I believe is unavoidable), the app seems to be functioning properly.
  • When I tested the native app with npm run e2e, all specs passed.

Here are the fixes I made:

  • The agent's trace for creating manifest(typo) metadata files should start at the point of DB initialization. Previously, I started it partway through.
  • The correction here that @mraible previously wrote was effective again. (I came across it by chance while investigating the error. Hello:flushed:)

Also, I apologize, some parts of my previous comment were incorrect and redundant (I will correct these with a strike-through line in my previous comment).

  • --enable-http was unrelated to this issue.
  • It was sufficient to store just one set of metadata files in src/main/resources/META-INF/native-image. Creating folders that correspond to the artifacts was unnecessary.

Additionally, I was considering using npm run e2e to ensure the creation of metadata files through the agent's trace, but unfortunately, this caused the JVM to crash.
Is there a way to make sure the metadata files are created correctly?
@atomfrede, I was unable to find 'in code instead of json files' due to my lack of research.

I hope this helps, even if it's just a little.

@hide212131
Copy link

hide212131 commented Jul 2, 2023

In the dev environment, I've managed to get the app running with a minimum metadata. The number of definitions in the metadata is less than 30. All tests pass with 'npm run e2e'.

Here are the changes I've made from the beginning.

Ultimately, I couldn't figure out how to reduce the numerous metadata, so I decided to start from scratch, and from there, went through a process of trial and error, diligently eliminating errors one by one.
I also referred to generator-jhipster-native, some of which was still relevant.
I hope this is helpful.

I'm currently working on prod (mysql), but progress has been slow.

@hide212131
Copy link

There are challenges, but I have managed to get the CRUD operations for Blog/Post/Tag working in the GraalVM's native environment with the following configurations:

  • MySQL in production
  • Hibernate's Lazy loading

Here are the changes I've made.

The implemented solutions are as follows:

  1. Added a bit of metadata for MySQL
  2. Introduced the compile-time class extension for Hibernate: hibernate-enhance-maven-plugin.
  3. Addressed the issue of LazyInitializationException arising during compile-time class extension.
  4. Fixed a bug where the PersistentSet wouldn't materialize.
    • I couldn't find a root cause solution and had to address it using Hibernate.initialize().

Issues 3 and 4 occur due to the hibernate-enhance-maven-plugin. It doesn't work in non-native environments either. If it works there, it will also work in Native.

I've spent a month analyzing issue 4 but to no avail. Specifically, I can't retrieve Post.getTags(). Even if I consciously try to fetch it Eagerly, the PersistentSet doesn't materialize. It currently looks like this:

problem

With the use of Hibernate.initialize(), it barely looks like this:

initialized

@Transactional propagation seems to be affected. (For example, removing @Transactional from PostResource works.) But I couldn't find anything more.

...

This is the situation, but it somewhat works as it is.
What should we do moving forward? Should we continue to find a solution (can someone knowledgeable help...), or should we shift to another platform like Postgresql, integrate it into the generator for the time being, or turn it into a Blueprint, etc...?

@mraible
Copy link
Owner Author

mraible commented Aug 14, 2023

@hide212131 Thanks for all your work on this!

I think the coolest solution would be to integrate native support into the main generator by default. The only issue with this is that start.spring.io doesn't integrate native by default. You have to add it as a dependency. If you select Spring Data JPA on start.spring.io, it also adds the hibernate-enhance-maven-plugin. Because of this, I could see putting everything in a blueprint too.

Or, we could add an additional question to the generator asking if the user wants to add GraalVM support. Then we could make changes accordingly. I believe this might also be useful for the Micronaut and Quarkus generators.

I'm curious to hear what others think. @mshima, @DanielFran, @atomfrede, @deepu105

@mshima
Copy link

mshima commented Aug 14, 2023

I think we should keep as a blueprint for now.
Integration tests takes too long.
An official feature without per PR integration tests is too difficult to maintain.
Using as a blueprint like it is now, we can fix once per release and keep related complexity out of main repository.

@mraible
Copy link
Owner Author

mraible commented Aug 16, 2023

@mshima You make an excellent point about CI.

@hide212131 Are you willing and able to make these changes in the https://github.com/jhipster/generator-jhipster-native blueprint?

@hide212131
Copy link

@mraible Thank you for calling on me for this. Yes, I'm willing, and I'll try to create a PR for the changes.

@mshima Please allow me to ask questions if I have any uncertainties regarding the new blueprint specifications.

@deepu105
Copy link

I'm also in favour of Blueprint as the repo is too big now.

@atomfrede
Copy link

I would also go with a blueprint. I guess there is not much a central approach can provide when it comes to supporting native in e.g. micronaut or quarkus blueprint. From what I have seen, it is mostly configuration each blueprint needs to handle anyways.

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.

5 participants