Skip to content

SQL annotations didn't work anymore with Quarkus starting with 1.3 #72

@loicmathieu

Description

@loicmathieu
Contributor

Describe the bug
SQL annotations didn't work with Quarkus starting with 1.3.
No issues are detected.

I chased down the issue to SqlRecorderRegistry that uses an InheritableThreadLocal to stores the SqlRecorder.
Starting with Quarkus 1.3, when inside the DataSourceQuickPerfListener the list of sql recorders is empty?

I'm not sure if it's due to the changes in Classloader inside Quarkus or the fact that it is now run on Vert.X for all wordload (so possibly on a separate thread).

To Reproduce
Update the Quarkus example application of Quickperf to use Quarkus 1.3.0.Final.

Versions

  • QuickPerf: master
  • JDK: 11
  • OS: Unbuntu
  • Database (if SQL annotation bug): All

Additional context (Add any other context about the problem here.)

Activity

loicmathieu

loicmathieu commented on Aug 24, 2020

@loicmathieu
ContributorAuthor

I have a better understanding on what's going on here.

Quarkus uses Vert.x and all I/O are done via a worker thread.
The SqlRecorderRegistry is called inside the main thread and the DataSourceQuickPerfListener is called via a worker thread as it is called via the datasource layer.

So we cannot use a ThreadLocal storage to store the SqlRecorder result as it is done today, it cannot work for Quarkus (and it will not work for other framework that performs I/O on a separate thread, and it's certainly the case for all reactive frameworks).

So we need to find a way to store the SQL_RECORDERS_WHEN_ONE_JVM not in a ThreadLocal, maybe a Map with some execution identifier as key ?

jeanbisutti

jeanbisutti commented on Aug 26, 2020

@jeanbisutti
Collaborator

Thanks for these explanations.

Tests may be executed in parallel. They have to be independent of each other. The thread local implementation allows this when these tests are executed in only one JVM and when the thread executing the test methods also calls the datasource. When the test method and the datasource are called from different threads, the thread local implementation cannot work. When a test is executed in a specific JVM, and so isolated from the other tests, the implementation does not use a thread local (see SqlRecorderRegistry). This is why, in quickperf-examples, the test methods of Spring RET controllers use the @heapsize annotation.

So we need to find a way to store the SQL_RECORDERS_WHEN_ONE_JVM not in a ThreadLocal, maybe a Map with some execution identifier as key ?

I don't think it could work without adding code instrumenation on frameworks (Spring, Quarkus, ...). I don't prefer to do that.

Could you please test by adding @heapsize on Quarkus tests?
`

loicmathieu

loicmathieu commented on Aug 26, 2020

@loicmathieu
ContributorAuthor

Could you please test by adding @heapsize on Quarkus tests?

I can't as forking the VM didn't work for Quarkus because we fork too late.
If I remove the threadlocal it works, but I agree it will not work when tests are executed in parallel.

I think supporting tests in parallel is less important than supporting all the major microservice frameworks.
If test are executed in parallel, we could still be able to have a unique test identifier somehow ...

jeanbisutti

jeanbisutti commented on Aug 26, 2020

@jeanbisutti
Collaborator

Could you please provide a project example reproducing the problem? Thanks

jeanbisutti

jeanbisutti commented on Aug 31, 2020

@jeanbisutti
Collaborator

With Quarkus 1.3 and a JVM annotation, I got an Address already in use: bind because the beforeAll(ExtensionContext context) method of QuarkusTestExtension is called two times, once in each JVM (the main JVM and the JVM where the test method is executed).

To have Quarkus JUnit 5 extension executed only in the JVM where the test method is executed, I wanted to test the following solution:

    @RegisterExtension
    Extension quarkusTestExtension = buildQuarkusExtensionInNewJVM()
    
     private Extension buildQuarkusExtensionInNewJVM() {
        if(SystemProperties.TEST_CODE_EXECUTING_IN_NEW_JVM.evaluate()) {
            return new QuarkusTestExtension();
        }
        return new Extension() {}; // Extension doing nothing
    }

    @RegisterExtension
    QuickPerfTestExtension quickPerfTestExtension = new QuickPerfTestExtension();

However, the following Quarkus code without QuickPerf code does not work

@RegisterExtension
    Extension quarkusTestExtension = new QuarkusTestExtension();
java.lang.NullPointerException
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:564)
	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:342)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:281)
	at org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
	.
	.
	.
	at com.intellij.rt.junit.JUnitStarter.main(JUnitStarter.java:53)
	Suppressed: java.lang.NullPointerException
		at io.quarkus.test.junit.QuarkusTestExtension.afterEach(QuarkusTestExtension.java:136)
		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeAfterEachCallbacks$11(TestMethodTestDescriptor.java:248)
		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeAllAfterMethodsOrCallbacks$13(TestMethodTestDescriptor.java:268)
		at org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeAllAfterMethodsOrCallbacks$14(TestMethodTestDescriptor.java:268)
		at java.base/java.util.ArrayList.forEach(ArrayList.java:1510)
		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeAllAfterMethodsOrCallbacks(TestMethodTestDescriptor.java:267)
		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeAfterEachCallbacks(TestMethodTestDescriptor.java:247)
		at org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:143)


The example is here.

loicmathieu

loicmathieu commented on Sep 2, 2020

@loicmathieu
ContributorAuthor

@jeanbisutti as QuarkusTestExtension implements BeforeAllCallback and AfterAllCallback, you need to use a static attribute when using the extension with @RegisterExtension.

@RegisterExtension
static Extension quarkusTestExtension = new QuarkusTestExtension();

See for details Static Fields section of the dcumentation: https://junit.org/junit5/docs/5.1.1/api/org/junit/jupiter/api/extension/RegisterExtension.html

jeanbisutti

jeanbisutti commented on Sep 2, 2020

@jeanbisutti
Collaborator

@loicmathieu, thanks

The example is updated with a static field.

However, with this modification, mvn clean test -Dtest=PlayerControllerTest leads to the following error report:

org.junit.jupiter.api.extension.TestInstantiationException: Failed to create test instance
Caused by: java.lang.RuntimeException: java.lang.reflect.InvocationTargetException
Caused by: java.lang.reflect.InvocationTargetException
Caused by: javax.enterprise.inject.UnsatisfiedResolutionException: No bean found for required type

As you kwow well Quarkus could you please have a look?

loicmathieu

loicmathieu commented on Sep 2, 2020

@loicmathieu
ContributorAuthor

@jeanbisutti can you try with Quarkus 1.7.0 ?
And if you can share your code in a branch I'll be able to work on it more easily

loicmathieu

loicmathieu commented on Sep 4, 2020

@loicmathieu
ContributorAuthor

So you need to defines the annotation as a static attribute and upgrade to 1.7.0.Final:

    @RegisterExtension
    static Extension quarkusTestExtension = buildQuarkusExtensionInNewJVM();

    private static Extension buildQuarkusExtensionInNewJVM() {
        if(SystemProperties.TEST_CODE_EXECUTING_IN_NEW_JVM.evaluate()) {
            System.out.println("Using the extension");
            return new QuarkusTestExtension();
        }
        System.out.println("Disable the extension");
        return new Extension() {}; // Extension doing nothing
    }

Then we're facing a new issue: No bean found for required type [class org.quickperf.quarkus.quarkustest.controller.PlayerControllerTest] and qualifiers [[]].

Quarkus register the test classes as a CDI bean to be able to make injection inside it.
And as the Test is not annotated with @QuarkusTest it is not discovered as a CDI bean and we cannot make injection inside it ...
I will raise this issue with the Quarkus developpers ...

loicmathieu

loicmathieu commented on Sep 4, 2020

@loicmathieu
ContributorAuthor

It's a tricky issue, we must register the extension with the QuarkusTestExtension type not the Extension type.

jeanbisutti

jeanbisutti commented on Sep 4, 2020

@jeanbisutti
Collaborator

It's a tricky issue, we must register the extension with the QuarkusTestExtension type not the Extension type.

It's better with Quarkus 1.7 and QuarkusTestExtension .

I quickly add new code in this project to have Quarkus and QuickPerf extensions working together. It does not work yet. I imagine that things should be adjusted in QuarkusTestExtensionDoingNothing class. In the following days, I will not have the time to look more in-depth. Don't hesitate to work on the implementation if you like.

loicmathieu

loicmathieu commented on Sep 4, 2020

@loicmathieu
ContributorAuthor

I was trying exactly the same a few hours ago ;) and didn't succeed either.
I'll try to find what's going on because this seems to me that it should works.

malys

malys commented on Jun 1, 2021

@malys

Hi @loicmathieu,
Some news about quick-perf and quarkus compatibility?
It's not working with Quarkus neither 1.13.4.Final nor 2.0.0.CR2

loicmathieu

loicmathieu commented on Jun 1, 2021

@loicmathieu
ContributorAuthor

@malys unfortunatly no, the reactive nature of Quarkus and it's specific classloader made it very difficult for quickperf to works on it. I tried multiple things without success.

malys

malys commented on Jun 1, 2021

@malys

What a shame! because Quick-perf seems very interesting. Do you know any alternative compatible with Quarkus? performance test oriented regression detection
Thanks @loicmathieu

loicmathieu

loicmathieu commented on Jun 1, 2021

@loicmathieu
ContributorAuthor

Do you know any alternative compatible with Quarkus? performance test oriented regression detection

@malys you can ping me on twitter or via email to discuss this subject but I still keep this issue on my todo list and I hope I will find a way to make Quickperf working with Quarkus as I'm a contributor to both project that are both awesome !

coiouhkc

coiouhkc commented on Sep 13, 2021

@coiouhkc

Hi @loicmathieu , any updates/alternatives so far?
Till I found this issue i tried to add quickperf to my quarkus 2.2.1.Final based solution:

  • needed to add junit-platform.properties to make it running
  • @ExpectSelect still wasn't detecting any statements.
loicmathieu

loicmathieu commented on Sep 13, 2021

@loicmathieu
ContributorAuthor

@coiouhkc no update so far, Quarkus has its own classloader and this causes challenge to integrate it with Quickperf.
I need to find the time to think of a custom solution for it.

loicmathieu

loicmathieu commented on May 31, 2022

@loicmathieu
ContributorAuthor

@jcunliffe1 this is a little more complex, the datasource is created in a different classloader than when the SQL queries are exectuted so the way we register the profile information didn't work. I currently don't have time to think of a different way to do it.

jcunliffe1

jcunliffe1 commented on Jun 14, 2022

@jcunliffe1

bummer, your library is awesome - would have loved to get it included.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    🐛 bugSomething isn't workingquarkusQuarkussqlSQL annotations

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @malys@coiouhkc@loicmathieu@jeanbisutti@jcunliffe1

        Issue actions

          SQL annotations didn't work anymore with Quarkus starting with 1.3 · Issue #72 · quick-perf/quickperf