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

Quarkus 3.0.1 behaves different in reactive native mode #33106

Closed
michael-simons opened this issue May 3, 2023 · 13 comments · Fixed by #33146
Closed

Quarkus 3.0.1 behaves different in reactive native mode #33106

michael-simons opened this issue May 3, 2023 · 13 comments · Fixed by #33146
Milestone

Comments

@michael-simons
Copy link
Contributor

This is a reminder for @maxandersen for

quarkiverse/quarkus-neo4j#161 (comment)

and quarkiverse/quarkus-neo4j#165 (comment)

We didn't change anything on hour side between 3.0.0 and 3.0.1 and I am out of options for that.

Issues persists with 3.0.2, too

@maxandersen
Copy link
Member

@Sanne @cescoffier difference between hibernate reactive
Or mutiny on native?

@michael-simons
Copy link
Contributor Author

Mutiny most likely, we are not using hibernate in any way or form.

@gsmet
Copy link
Member

gsmet commented May 3, 2023

/cc @jponge

@jponge
Copy link
Member

jponge commented May 3, 2023

There's nothing native-specific in Mutiny, and the fact that the tests pass in JVM mode suggest that it might rather be an issue with the driver / integration itself.

What exact version of Mutiny is involved?

/cc @cescoffier

@michael-simons
Copy link
Contributor Author

michael-simons commented May 3, 2023 via email

@jponge
Copy link
Member

jponge commented May 3, 2023

Both Quarkus 3.0.0.Final and 3.0.1.Final use Mutiny 2.1.0.

@michael-simons
Copy link
Contributor Author

So, I have now managed to create a full reproducer.

Please bring up a Neo4j instance with

docker run --publish=7474:7474 --publish=7687:7687 -e 'NEO4J_AUTH=neo4j/verysecret' neo4j:5.7

Download the attached project full-reproducer.zip and compile and run with

full-reproducer.zip

./mvnw -Dnative clean package
./target/code-with-quarkus-1.0.0-SNAPSHOT-runner

After that, compare the both the following resources GreetingResource which is broken and FixedResource, which works.

Check with

curl -v  localhost:8080/fixed # returns 10 items, 
curl -v  localhost:8080/broken # hangs, while the application clearly queries the database and closes the session

The difference for the resources? The methods behind the paths above are literally the same:

@Path("/broken")
public class GreetingResource {

	@Inject
	Driver driver;

	static Uni<Void> sessionFinalizer(ReactiveSession session) { // <.>
		return Uni.createFrom().publisher(session.close()).onItem()
			.invoke(() -> System.out.println("Session closed"))
			.replaceWithVoid();
	}

	private Multi<Long> get0() {
		return Multi.createFrom().resource(() -> driver.session(ReactiveSession.class), // <.>
				session -> session.executeRead(tx -> {
					var result = tx.run("MATCH (f:Foobar) RETURN f.value as value ORDER BY f.value");
					System.out.println("Created result, now publishing");
					return Multi.createFrom().publisher(result).flatMap(ReactiveResult::records);
				}))
			.withFinalizer(GreetingResource::sessionFinalizer) // <.>
			.map(record -> record.get("value").asLong());
	}

	@GET
	@Produces(MediaType.SERVER_SENT_EVENTS)
	public Flow.Publisher<Long> get() {

		return get0();

	}
}

The difference in the working resource is one additional method:

	@GET
	@Produces(MediaType.SERVER_SENT_EVENTS)
	@Path("/asMulti")
	public Multi<Long> asMulti() {

		return get0();

	}

Both - the broken and the fixed version - work in JVM mode. That indicates to me that native image and the method type detection of the JAX RS resources changed between Quarkus 3.0.0 and 3.0.1

The last indicator:

This resource does not work at all in native Mode:

@Path("/noNeo")
public class NoNeoResource {

	@GET
	@Produces(MediaType.SERVER_SENT_EVENTS)
	public Flow.Publisher<Long> get() {

		return Multi.createFrom().items(LongStream.rangeClosed(1, 10).boxed());

	}
}

Curling it curl -v localhost:8080/noNeo yields in

2023-05-04 11:05:19,973 ERROR [io.qua.ver.cor.run.VertxCoreRecorder] (vert.x-eventloop-thread-3) Uncaught exception received by Vert.x: java.lang.RuntimeException: java.lang.NoSuchMethodException: org.acme.NoNeoResource.get()
	at org.jboss.resteasy.reactive.server.spi.ResteasyReactiveResourceInfo.getMethod(ResteasyReactiveResourceInfo.java:72)
	at org.jboss.resteasy.reactive.server.spi.ResteasyReactiveResourceInfo.getAnnotations(ResteasyReactiveResourceInfo.java:89)
	at org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext.getMethodAnnotations(ResteasyReactiveRequestContext.java:553)
	at org.jboss.resteasy.reactive.server.core.ResteasyReactiveRequestContext.getAllAnnotations(ResteasyReactiveRequestContext.java:531)
	at org.jboss.resteasy.reactive.server.core.SseUtil.serialiseDataToString(SseUtil.java:137)
	at org.jboss.resteasy.reactive.server.core.SseUtil.serialiseEvent(SseUtil.java:74)
	at org.jboss.resteasy.reactive.server.core.SseUtil.send(SseUtil.java:39)
	at org.jboss.resteasy.reactive.server.handlers.PublisherResponseHandler$SseMultiSubscriber.onNext(PublisherResponseHandler.java:61)
	at io.smallrye.mutiny.helpers.HalfSerializer.onNext(HalfSerializer.java:30)
	at io.smallrye.mutiny.helpers.StrictMultiSubscriber.onItem(StrictMultiSubscriber.java:84)
	at io.smallrye.mutiny.operators.multi.builders.StreamBasedMulti$StreamSubscription.pull(StreamBasedMulti.java:113)
	at io.smallrye.mutiny.operators.multi.builders.StreamBasedMulti$StreamSubscription.request(StreamBasedMulti.java:87)

I have created both examples for pure mutiny and Quarkus without JAX RS, attached as well. They don't expose the problem.

pure-mutiny-no-issues.zip
quarkus-cmd-no-issues.zip

Have fun ;)

michael-simons added a commit to quarkiverse/quarkus-neo4j that referenced this issue May 4, 2023
Also change return type to Mutiny Multi, standard Flow.Publisher has issues

See quarkusio/quarkus#33106.

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Michael Simons <[email protected]>
@cescoffier
Copy link
Member

Thanks for all the information @michael-simons!

It might be because of f73ec98, but I'm not totally sure.

(Note: we should add a test for this)

@michael-simons
Copy link
Contributor Author

michael-simons commented May 5, 2023

My pleasure, Clement!

This is a good candidate for test in native functionality

@Path("/noNeo")
public class NoNeoResource {

	@GET
	@Produces(MediaType.SERVER_SENT_EVENTS)
	public Flow.Publisher<Long> get() {

		return Multi.createFrom().items(LongStream.rangeClosed(1, 10).boxed());

	}
}

it works as expected in JVM, not in native. Didn't get anything smaller reproduced.

@michael-simons
Copy link
Contributor Author

That condition here f73ec98#diff-f687ef5f49c43c73a60a5b8fbd573e7d38a827fce537bd3097b71a4753635f68R553 would explain why the above resource would start working when you add another method returning Multi (but not Uni). So in fact I think there are two cases missing, checking for Uni and Flow.Publisher return types.

@cescoffier
Copy link
Member

Uni cannot be used for SSE (does not make sense). But yes...

cescoffier added a commit to cescoffier/quarkus that referenced this issue May 5, 2023
In addition to Multi, also handle the Flow.Publisher case.
@cescoffier
Copy link
Member

@michael-simons Can you try with #33146?

It passed my local test, and I also added an IT.

(I need to catch a plane, I will continue on Tuesday if it does not work).

@michael-simons
Copy link
Contributor Author

I can confirm that it works for all my use cases in the full-reproducer project above, with and without Neo4j. Excellent work thank you!

cescoffier added a commit to cescoffier/quarkus that referenced this issue May 5, 2023
In addition to Multi, also handle the Flow.Publisher case.
@quarkus-bot quarkus-bot bot added this to the 3.1 - main milestone May 5, 2023
@gsmet gsmet modified the milestones: 3.1 - main, 3.0.3.Final May 9, 2023
gsmet pushed a commit to gsmet/quarkus that referenced this issue May 9, 2023
In addition to Multi, also handle the Flow.Publisher case.

(cherry picked from commit 1f7cfc0)
manofthepeace pushed a commit to manofthepeace/quarkus that referenced this issue May 16, 2023
In addition to Multi, also handle the Flow.Publisher case.
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 a pull request may close this issue.

5 participants