Context Propagation performance issue and init issue

[FroMage]

The original issue I wanted to fix is a performance issue noticed by @franz1981 at smallrye/smallrye-context-propagation#443 (comment), because in Quarkus, we don't need a per-classloader Map since we have a single class loader and a single ContextManager.

Doing this, I noticed that some extensions were using CP between static init and runtime init. They were relying on CP auto-booting itself during that period, so it "worked", but it had only a partial number of contexts to propagate, and it started an ExecutorService instead of using the Quarkus one, which is only available at runtime init.

There were four types of problematic users:

• Extensions that did not wait for Mutiny or CP to be initialised. For this I added a ContextPropagationInitializedBuildItem to consume.
• Extensions that used CP during class init, which caused problems when building a native-image which initialises classes at build time. For this I delayed the problematic class initialisation until runtime.
• Extensions that used Vert.x, via Mutiny before runtime init, such as spring-cloud-config-client which is a config-reading extension, so it can't run before runtime init. I didn't find how to make this wait for our initialisation, I don't think it's possible, because the executor needs config loaded to be created, so it's a chicken/egg problem.
• The MP TCK which fails in OT for some reason that I'm not sure is really caused by this.

For the last two issues, I created a no-config/no-executor ContextManager that lives between static and runtime init, before it's replaced by the properly initialised one.

This is not perfect, because this might be used by something initialised too early and live on past runtime init and keep not propagating any contexts, but this was already the case before this PR. I could log something, but it'd have to be a WARN and any user of spring-cloud-config-client would have that warning. Perhaps it's acceptable?

At least, if anybody tries to use the temporary executor, they will get a meaningful exception to let them know not to do it. And we avoid creating an executor, also.

Anyway, let's see how far CI goes, with this.

Note that Mutiny suffers from the same issue that a lot of extensions are using it before it's fully initialised (for the same reason) and it also doesn't have a build item to depend on.

[quarkus-bot]

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 08ba1d1.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.

---

Flaky tests - Develocity

⚙️ JVM Tests - JDK 17

📦 extensions/micrometer/deployment

✖ io.quarkus.micrometer.deployment.binder.VertxTcpMetricsNoClientMetricsTest.testTcpMetricsWithoutClientMetrics - History

• event executor terminated - java.util.concurrent.RejectedExecutionException

java.util.concurrent.RejectedExecutionException: event executor terminated
	at io.netty.util.concurrent.SingleThreadEventExecutor.reject(SingleThreadEventExecutor.java:934)
	at io.netty.util.concurrent.SingleThreadEventExecutor.offerTask(SingleThreadEventExecutor.java:351)
	at io.netty.util.concurrent.SingleThreadEventExecutor.addTask(SingleThreadEventExecutor.java:344)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:836)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute0(SingleThreadEventExecutor.java:827)
	at io.netty.util.concurrent.SingleThreadEventExecutor.execute(SingleThreadEventExecutor.java:817)
	at io.vertx.core.impl.EventLoopExecutor.execute(EventLoopExecutor.java:35)

---

⚙️ JVM Tests - JDK 21

📦 extensions/smallrye-reactive-messaging-kafka/deployment

✖ io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase.sseStream - History

• Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase Expecting size of: [] to be greater than or equal to 2 but was 0 within 10 seconds. - org.awaitility.core.ConditionTimeoutException

org.awaitility.core.ConditionTimeoutException: 
Assertion condition defined as a Lambda expression in io.quarkus.smallrye.reactivemessaging.kafka.deployment.dev.KafkaDevServicesDevModeTestCase 
Expecting size of:
  []
to be greater than or equal to 2 but was 0 within 10 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:119)
	at org.awaitility.core.AssertionCondition.await(AssertionCondition.java:31)

[FroMage]

@franz1981 can you profile this to see if it solves your issue?

[franz1981]

@FroMage yep, just verified that it brought the same performance improvement of the original branch (which is still the same - but wasn't fixing the other things you mentioned in the first comment!). When we brought it in we will know, by tomorrow, if it has other effects if its changes impact quick-start config or getting-started (are the tests we have for daily regression checks, along with others)

[franz1981]

LGTM!

[FroMage]

At least 1 approving review is required by reviewers with write access.

@franz1981 you don't have write access? @geoand perhaps can review? @gsmet perhaps we want to grant @franz1981 reviewing power?

[geoand]

I could log something, but it'd have to be a WARN and any user of spring-cloud-config-client would have that warning. Perhaps it's acceptable?

I would not want that because using this extension is perfectly legit and warning the user about something they have no control about does not make sense IMHO

[geoand]

Looks great!

[FroMage]

I would not want that because using this extension is perfectly legit and warning the user about something they have no control about does not make sense IMHO

It would not be good for this extension, for sure. But it would be good if any other extension turns out to be using CP or Mutiny before they're initialised. Oh well, hopefully we'll remember that if something weird happens :)

[Ladicek]

Added the backport labels due to https://issues.redhat.com/browse/QUARKUS-4477, although technically, to fix that bug, only one commit from this PR is necessary (b9cc3c2).

[gsmet]

We backported to 3.8 the commits about OIDC and FT initialization but not the other two.