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

Fix #6490 add mybatis extension #10671

Closed
wants to merge 8 commits into from

Conversation

zhfeng
Copy link
Contributor

@zhfeng zhfeng commented Jul 13, 2020

No description provided.

@boring-cyborg boring-cyborg bot added the area/dependencies Pull requests that update a dependency file label Jul 13, 2020
@machi1990 machi1990 requested review from gsmet and aloubyansky July 13, 2020 10:32
for (String mapper : mappers) {
try {
configuration.addMapper(Resources.classForName(mapper));
} catch (ClassNotFoundException e) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe log.debug here?

bom/runtime/pom.xml Outdated Show resolved Hide resolved
Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

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

It would be nice to have integration tests (including native generation) too

@zhfeng zhfeng force-pushed the mybatis_extension branch from e1613c6 to b57fb1d Compare July 15, 2020 06:23
@zhfeng
Copy link
Contributor Author

zhfeng commented Jul 15, 2020

Thanks @gastaldi - I just made the changes as your suggestions and add the integration tests for the mybatis. The JVM test works fine but the native test looks weird. The native image build is failing with the following messges

[INFO] --- quarkus-maven-plugin:999-SNAPSHOT:native-image (native-image) @ quarkus-mybatis-integration-test ---
[INFO] [io.quarkus.deployment.pkg.steps.JarResultBuildStep] Building native image source jar: /home/zhfeng/work/zhfeng/quarkus/integration-tests/mybatis/target/quarkus-mybatis-integration-test-999-SNAPSHOT-native-image-source-jar/quarkus-mybatis-integration-test-999-SNAPSHOT-runner.jar
[INFO] [io.quarkus.deployment.pkg.steps.NativeImageBuildStep] Building native image from /home/zhfeng/work/zhfeng/quarkus/integration-tests/mybatis/target/quarkus-mybatis-integration-test-999-SNAPSHOT-native-image-source-jar/quarkus-mybatis-integration-test-999-SNAPSHOT-runner.jar
[INFO] [io.quarkus.deployment.pkg.steps.NativeImageBuildStep] Running Quarkus native-image plugin on GraalVM Version 20.1.0 (Java Version 11.0.7)
[INFO] [io.quarkus.deployment.pkg.steps.NativeImageBuildStep] /home/zhfeng/Applications/graalvm-ce-java11-20.1.0/bin/native-image -J-Dsun.nio.ch.maxUpdateArraySize=100 -J-DCoordinatorEnvironmentBean.transactionStatusManagerEnable=false -J-Djava.util.logging.manager=org.jboss.logmanager.LogManager -J-Dio.netty.leakDetection.level=DISABLED -J-Dio.netty.allocator.maxOrder=1 -J-Dvertx.logger-delegate-factory-class-name=io.quarkus.vertx.core.runtime.VertxLogDelegateFactory -J-Dvertx.disableDnsResolver=true -J-Duser.language=en -J-Dfile.encoding=UTF-8 --initialize-at-build-time= -H:InitialCollectionPolicy=com.oracle.svm.core.genscavenge.CollectionPolicy$BySpaceAndTime -H:+JNI -jar quarkus-mybatis-integration-test-999-SNAPSHOT-runner.jar -H:FallbackThreshold=0 -H:+ReportExceptionStackTraces -H:-AddAllCharsets -H:-IncludeAllTimeZones -H:EnableURLProtocols=http,https --enable-all-security-services -H:NativeLinkerOption=-no-pie --no-server -H:-UseServiceLoaderFeature -H:+StackTrace quarkus-mybatis-integration-test-999-SNAPSHOT-runner
-H:IncludeAllTimeZones and -H:IncludeTimeZones are now deprecated. Native-image includes all timezonesby default.
[quarkus-mybatis-integration-test-999-SNAPSHOT-runner:674369]    classlist:   6,446.38 ms,  0.94 GB
[quarkus-mybatis-integration-test-999-SNAPSHOT-runner:674369]        (cap):     718.20 ms,  0.94 GB
[quarkus-mybatis-integration-test-999-SNAPSHOT-runner:674369]        setup:   2,509.04 ms,  0.94 GB
14:18:26,050 INFO  [org.jbo.threads] JBoss Threads version 3.1.1.Final
[quarkus-mybatis-integration-test-999-SNAPSHOT-runner:674369]     analysis:  20,234.45 ms,  2.64 GB
2 fatal errors detected:
Fatal error:com.oracle.graal.pointsto.util.AnalysisError$ParsingError: Error encountered while parsing com.oracle.svm.reflect.Class_getInterfaces0_afd19408bba3599ae1dfbbcd6448ba114ce3d3fc.invoke(java.lang.Object, java.lang.Object[]) 
Parsing context:
	parsing java.lang.reflect.Method.invoke(Method.java:566)
	parsing io.quarkus.arc.impl.Qualifiers.invoke(Qualifiers.java:105)
	parsing io.quarkus.arc.impl.Qualifiers.hasQualifier(Qualifiers.java:66)
	parsing io.quarkus.arc.impl.Qualifiers.hasQualifier(Qualifiers.java:47)
	parsing io.quarkus.arc.impl.Qualifiers.hasQualifiers(Qualifiers.java:39)
	parsing io.quarkus.arc.impl.ArcContainerImpl.matches(ArcContainerImpl.java:689)
	parsing io.quarkus.arc.impl.ArcContainerImpl.getMatchingBeans(ArcContainerImpl.java:585)
	parsing io.quarkus.arc.impl.ArcContainerImpl.getBeans(ArcContainerImpl.java:441)
	parsing io.quarkus.arc.impl.BeanManagerImpl.getBeans(BeanManagerImpl.java:94)
	parsing io.quarkus.runtime.ApplicationLifecycleManager.run(ApplicationLifecycleManager.java:95)
	parsing io.quarkus.runtime.Quarkus.run(Quarkus.java:61)
	parsing io.quarkus.runtime.Quarkus.run(Quarkus.java:38)
	parsing io.quarkus.runtime.Quarkus.run(Quarkus.java:106)
	parsing io.quarkus.runner.GeneratedMain.main(GeneratedMain.zig:29)
	parsing com.oracle.svm.core.JavaMainWrapper.runCore(JavaMainWrapper.java:149)
	parsing com.oracle.svm.core.JavaMainWrapper.run(JavaMainWrapper.java:184)
	parsing com.oracle.svm.core.code.IsolateEnterStub.JavaMainWrapper_run_5087f5482cc9a6abc971913ece43acb471d2631b(generated:0)

	at com.oracle.graal.pointsto.util.AnalysisError.parsingError(AnalysisError.java:138)
	at com.oracle.graal.pointsto.flow.MethodTypeFlow.doParse(MethodTypeFlow.java:340)
	at com.oracle.graal.pointsto.flow.MethodTypeFlow.ensureParsed(MethodTypeFlow.java:311)
	at com.oracle.graal.pointsto.flow.MethodTypeFlow.addContext(MethodTypeFlow.java:112)
	at com.oracle.graal.pointsto.DefaultAnalysisPolicy$DefaultVirtualInvokeTypeFlow.onObservedUpdate(DefaultAnalysisPolicy.java:228)
	at com.oracle.graal.pointsto.flow.TypeFlow.notifyObservers(TypeFlow.java:470)
	at com.oracle.graal.pointsto.flow.TypeFlow.update(TypeFlow.java:542)
	at com.oracle.graal.pointsto.BigBang$2.run(BigBang.java:530)
	at com.oracle.graal.pointsto.util.CompletionExecutor.lambda$execute$0(CompletionExecutor.java:173)
	at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1426)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: com.oracle.svm.hosted.substitute.DeletedElementException: Unsupported method java.lang.Class.getInterfaces0() is reachable: The declaring class of this element has been substituted, but this element is not present in the substitution class
To diagnose the issue, you can add the option --report-unsupported-elements-at-runtime. The unsupported element is then reported at run time when it is accessed the first time.
	at com.oracle.svm.hosted.substitute.AnnotationSubstitutionProcessor.lookup(AnnotationSubstitutionProcessor.java:183)
	at com.oracle.graal.pointsto.infrastructure.SubstitutionProcessor$ChainedSubstitutionProcessor.lookup(SubstitutionProcessor.java:128)
	at com.oracle.graal.pointsto.infrastructure.SubstitutionProcessor$ChainedSubstitutionProcessor.lookup(SubstitutionProcessor.java:128)
	at com.oracle.graal.pointsto.meta.AnalysisUniverse.lookupAllowUnresolved(AnalysisUniverse.java:397)
	at com.oracle.graal.pointsto.meta.AnalysisUniverse.lookup(AnalysisUniverse.java:377)
	at com.oracle.graal.pointsto.meta.AnalysisUniverse.lookup(AnalysisUniverse.java:75)
	at com.oracle.graal.pointsto.infrastructure.UniverseMetaAccess.lookupJavaMethod(UniverseMetaAccess.java:93)
	at com.oracle.graal.pointsto.meta.AnalysisMetaAccess.lookupJavaMethod(AnalysisMetaAccess.java:66)
	at com.oracle.graal.pointsto.meta.AnalysisMetaAccess.lookupJavaMethod(AnalysisMetaAccess.java:39)
	at com.oracle.svm.reflect.hosted.ReflectionSubstitutionType$ReflectiveInvokeMethod.buildGraph(ReflectionSubstitutionType.java:511)
	at com.oracle.graal.pointsto.meta.AnalysisMethod.buildGraph(AnalysisMethod.java:333)
	at com.oracle.graal.pointsto.flow.MethodTypeFlowBuilder.parse(MethodTypeFlowBuilder.java:189)
	at com.oracle.graal.pointsto.flow.MethodTypeFlowBuilder.apply(MethodTypeFlowBuilder.java:352)
	at com.oracle.graal.pointsto.flow.MethodTypeFlow.doParse(MethodTypeFlow.java:322)
	... 13 more
Fatal error:com.oracle.graal.pointsto.util.AnalysisError$ParsingError: Error encountered while parsing com.oracle.svm.reflect.Class_allPermDomain_6c5e23148388eb3ffe80e2382beb0f2363f3eeab.get(java.lang.Object) 
Parsing context:
	parsing java.lang.reflect.Field.get(Field.java:418)
	parsing java.net.HttpConnectSocketImpl.doTunnel(HttpConnectSocketImpl.java:171)
	parsing java.net.HttpConnectSocketImpl$2.run(HttpConnectSocketImpl.java:151)
	parsing java.net.HttpConnectSocketImpl$2.run(HttpConnectSocketImpl.java:149)
	parsing com.oracle.svm.core.jdk.Target_java_security_AccessController.doPrivileged(SecuritySubstitutions.java:119)
	parsing java.util.ServiceLoader.getConstructor(ServiceLoader.java:667)
	parsing java.util.ServiceLoader.loadProvider(ServiceLoader.java:898)
	parsing java.util.ServiceLoader$LayerLookupIterator.hasNext(ServiceLoader.java:951)
	parsing java.util.ServiceLoader$3.hasNext(ServiceLoader.java:1384)
	parsing javax.enterprise.inject.spi.CDI.findAllProviders(CDI.java:120)
	parsing javax.enterprise.inject.spi.CDI.getCDIProvider(CDI.java:82)
	parsing javax.enterprise.inject.spi.CDI.current(CDI.java:64)
	parsing io.quarkus.runtime.ApplicationLifecycleManager.run(ApplicationLifecycleManager.java:94)
	parsing io.quarkus.runtime.Quarkus.run(Quarkus.java:61)
	parsing io.quarkus.runtime.Quarkus.run(Quarkus.java:38)
	parsing io.quarkus.runtime.Quarkus.run(Quarkus.java:106)
	parsing io.quarkus.runner.GeneratedMain.main(GeneratedMain.zig:29)
	parsing com.oracle.svm.core.JavaMainWrapper.runCore(JavaMainWrapper.java:149)
	parsing com.oracle.svm.core.JavaMainWrapper.run(JavaMainWrapper.java:184)
	parsing com.oracle.svm.core.code.IsolateEnterStub.JavaMainWrapper_run_5087f5482cc9a6abc971913ece43acb471d2631b(generated:0)

	at com.oracle.graal.pointsto.util.AnalysisError.parsingError(AnalysisError.java:138)
	at com.oracle.graal.pointsto.flow.MethodTypeFlow.doParse(MethodTypeFlow.java:340)
	at com.oracle.graal.pointsto.flow.MethodTypeFlow.ensureParsed(MethodTypeFlow.java:311)
	at com.oracle.graal.pointsto.flow.MethodTypeFlow.addContext(MethodTypeFlow.java:112)
	at com.oracle.graal.pointsto.DefaultAnalysisPolicy$DefaultVirtualInvokeTypeFlow.onObservedUpdate(DefaultAnalysisPolicy.java:228)
	at com.oracle.graal.pointsto.flow.TypeFlow.notifyObservers(TypeFlow.java:470)
	at com.oracle.graal.pointsto.flow.TypeFlow.update(TypeFlow.java:542)
	at com.oracle.graal.pointsto.BigBang$2.run(BigBang.java:530)
	at com.oracle.graal.pointsto.util.CompletionExecutor.lambda$execute$0(CompletionExecutor.java:173)
	at java.base/java.util.concurrent.ForkJoinTask$RunnableExecuteAction.exec(ForkJoinTask.java:1426)
	at java.base/java.util.concurrent.ForkJoinTask.doExec(ForkJoinTask.java:290)
	at java.base/java.util.concurrent.ForkJoinPool$WorkQueue.topLevelExec(ForkJoinPool.java:1020)
	at java.base/java.util.concurrent.ForkJoinPool.scan(ForkJoinPool.java:1656)
	at java.base/java.util.concurrent.ForkJoinPool.runWorker(ForkJoinPool.java:1594)
	at java.base/java.util.concurrent.ForkJoinWorkerThread.run(ForkJoinWorkerThread.java:177)
Caused by: com.oracle.svm.hosted.substitute.DeletedElementException: Unsupported field java.lang.Class.allPermDomain is reachable: The declaring class of this element has been substituted, but this element is not present in the substitution class
To diagnose the issue, you can add the option --report-unsupported-elements-at-runtime. The unsupported element is then reported at run time when it is accessed the first time.
	at com.oracle.svm.hosted.substitute.AnnotationSubstitutionProcessor.lookup(AnnotationSubstitutionProcessor.java:140)
	at com.oracle.graal.pointsto.infrastructure.SubstitutionProcessor$ChainedSubstitutionProcessor.lookup(SubstitutionProcessor.java:123)
	at com.oracle.graal.pointsto.infrastructure.SubstitutionProcessor$ChainedSubstitutionProcessor.lookup(SubstitutionProcessor.java:123)
	at com.oracle.graal.pointsto.meta.AnalysisUniverse.lookupAllowUnresolved(AnalysisUniverse.java:355)
	at com.oracle.graal.pointsto.meta.AnalysisUniverse.lookup(AnalysisUniverse.java:325)
	at com.oracle.graal.pointsto.meta.AnalysisUniverse.lookup(AnalysisUniverse.java:75)
	at com.oracle.graal.pointsto.infrastructure.UniverseMetaAccess.lookupJavaField(UniverseMetaAccess.java:98)
	at com.oracle.graal.pointsto.meta.AnalysisMetaAccess.lookupJavaField(AnalysisMetaAccess.java:71)
	at com.oracle.graal.pointsto.meta.AnalysisMetaAccess.lookupJavaField(AnalysisMetaAccess.java:39)
	at com.oracle.svm.reflect.hosted.ReflectionSubstitutionType$ReflectiveReadMethod.buildGraph(ReflectionSubstitutionType.java:384)
	at com.oracle.graal.pointsto.meta.AnalysisMethod.buildGraph(AnalysisMethod.java:333)
	at com.oracle.graal.pointsto.flow.MethodTypeFlowBuilder.parse(MethodTypeFlowBuilder.java:189)
	at com.oracle.graal.pointsto.flow.MethodTypeFlowBuilder.apply(MethodTypeFlowBuilder.java:352)
	at com.oracle.graal.pointsto.flow.MethodTypeFlow.doParse(MethodTypeFlow.java:322)
	... 13 more
Error: Image build request failed with exit status 1

I tried to add --report-unsupported-elements-at-runtime and the native image build passes, but the test fails with the NPE which seems that the mybatis mapper bean can not be injected.

@zhfeng
Copy link
Contributor Author

zhfeng commented Jul 17, 2020

Hi @gastaldi ,

Finally I got the native integration tests working but it has to add the --report-unsupported-elements-at-runtime option to build the native image. The MyBatis has a heavy use of the Java reflections and dynamic object proxies, so it needs more investigation. Anyway, we can support the basic SQL mapping to query, insert and delete currently. I think this is a good start for the mybatis extension and also add the mybatis.adoc in the docs. I hope it is helpful.

Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

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

Found some minor typos in the doc

docs/src/main/asciidoc/mybatis.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/mybatis.adoc Outdated Show resolved Hide resolved
docs/src/main/asciidoc/mybatis.adoc Outdated Show resolved Hide resolved
@zhfeng
Copy link
Contributor Author

zhfeng commented Jul 22, 2020

@gastaldi can you take a review of these changes ?

@gastaldi
Copy link
Contributor

I think this is cool. Can you start a conversation on https://groups.google.com/forum/#!forum/quarkus-dev about this extension before we merge this PR? Maybe someone could help on figuring out how to make the native build smoother for this extension

@zhfeng
Copy link
Contributor Author

zhfeng commented Jul 22, 2020

Thanks @gastaldi - I will start a thread on the quarkus forum.

@aloubyansky
Copy link
Member

Even if it happens to be true, btw, the decision should not depend on popularity in any specific country. As to the home for the extension, in my view, it's a question of maintainability. @zhfeng could you clarify your intentions wrt this work? Are you planning to be actively maintaining it?

@zhfeng
Copy link
Contributor Author

zhfeng commented Jul 24, 2020

I wrote a article to introduce Quarkus in Chinese two months ago and got some feedback from the developers that they want to see the MyBatis support in Quarkus which could be very helpful and attract more people to have a try.
Also there are two issue #6490 and #1958 in Quarkus and apache/camel-quarkus#1384 in Camel Quarkus to show the interesting of the MyBatis extension.

@aloubyansky yeah, I'm planning to maintain this extension. The next work could be

  • Add the support of XML mapping
  • Support @Cache annotation in the native
  • Get rid of the --report-unsupported-elements-at-runtime in the native image building

@zhfeng zhfeng force-pushed the mybatis_extension branch from 629b84a to 2656ef1 Compare July 30, 2020 06:54
@zhfeng
Copy link
Contributor Author

zhfeng commented Jul 30, 2020

@gastaldi I just update to fix the conflict and test with @CacheNamespace. I tried to take a look at the mybatis source codes deeply and it looks like they are using MethodHandles in the MapperProxy to invoke the default method of the interface dynamically. But according to the https://github.com/oracle/graal/blob/master/substratevm/LIMITATIONS.md#invokedynamic-bytecode-and-method-handles, these seem not supported by the Graal VM currently.

Also I found the mybatis/mybatis-3#1552 which are looking for the Graal support in MyBatis. So there will be chances we can work together with the Mybatis community to support the GraalVM.

In the perspective of the Quarkus, is there anything I can do before merging this PR ?

@gastaldi
Copy link
Contributor

gastaldi commented Jul 30, 2020

@zhfeng thanks for the update, you've done an excellent job in this.
I think we need to first resolve the need for --report-unsupported-elements-at-runtime since that means that would require some extra work (substituting classes for example), given that none of our extensions require that.

Meanwhile if you are interested, it would be really cool if this extension ended up in https://github.com/quarkiverse instead. WDYT @aloubyansky ?

@zhfeng
Copy link
Contributor Author

zhfeng commented Jul 31, 2020

yeah, I'm working to get rid of --report-unsupported-elements-at-runtime.

@zhfeng
Copy link
Contributor Author

zhfeng commented Aug 4, 2020

Hi @gastaldi ,

I still got some analyist parse erros during the native image builidng. And sometimes, these errors were very different in every building. Most of these are related to the java.lang.Class.invoke. I'm not very sure how to fix these error in the mybatis by using some substition classes. Can you give some hints please ?

@gastaldi
Copy link
Contributor

gastaldi commented Aug 4, 2020

@zhfeng hey! My knowledge on writing substitution classes is based on trial and error, so I am not sure I can give some hints in that regard :)

@dhaalves
Copy link

dhaalves commented Oct 5, 2020

Any update on this?

@gastaldi
Copy link
Contributor

gastaldi commented Oct 6, 2020

Hi @dhaalves!

We strongly recommend our community extensions to reside in the https://github.com/quarkiverse organization. This means free repository hosting (including build, CI and release publishing setup) for Quarkus extension projects contributed by the community. Plus it should integrate with our Quarkus Registry - making it visible in code.quarkus.io and related tooling (we're still working to make this happen automatically).

@zhfeng are you interested in moving this code to a quarkiverse-mybatis repository in the quarkiverse organization and become the project lead of it? I'll be more than happy to help you set up the organization and get you started ASAP. This also applies to any other extension you have in mind.

More details here: https://github.com/quarkiverse/quarkiverse/wiki

@zhfeng
Copy link
Contributor Author

zhfeng commented Oct 9, 2020

Thanks @gastaldi - I'm happy to move to quarkiverse.

@gastaldi
Copy link
Contributor

gastaldi commented Oct 9, 2020

@zhfeng fantastic! I'l create the repository tomorrow morning and give you the necessary push permissions.

Ping me in Zulip if you have any questions.

@gastaldi
Copy link
Contributor

gastaldi commented Oct 9, 2020

@zhfeng Repository created in https://github.com/quarkiverse/quarkiverse-mybatis. You should have received an invitation to join the organization (You should have the Maintainer role there).

Closing this PR now

@gastaldi gastaldi closed this Oct 9, 2020
@gastaldi gastaldi mentioned this pull request Oct 9, 2020
@gastaldi gastaldi added the triage/moved This issue/PR is moved to another repository label Oct 9, 2020
@gsmet gsmet added triage/wontfix This will not be worked on triage/invalid This doesn't seem right area/quarkiverse This issue/PR is part of the Quarkiverse organization and removed triage/wontfix This will not be worked on labels Oct 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/dependencies Pull requests that update a dependency file area/documentation area/quarkiverse This issue/PR is part of the Quarkiverse organization triage/invalid This doesn't seem right triage/moved This issue/PR is moved to another repository
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants