Produce `quarkus.uuid` lazily #45434

geoand · 2025-01-08T08:27:33Z

This is done because bootstrapping the plumbing
needed by the JDK to produce a UUID value
is expensive, it thus doesn't make sense to
pay this cost when the property isn't actually
needed

The goal is the same as that of #33941

franz1981 · 2025-01-08T08:33:15Z

Nice one!

geoand · 2025-01-08T08:34:27Z

🙏🏽 ❤️

gsmet

That would be awesome if it worked. It's a shame everybody pays the price for it.

Now I suppose anyone ending using the random infra will pay the price at some point but simple apps might not have any random around so it would clearly be a win.

gsmet · 2025-01-08T09:02:00Z

core/runtime/src/main/java/io/quarkus/runtime/ConfigConfig.java

-    /**
-     * A property that allows accessing a generated UUID.
-     * It generates that UUID at startup time. So it changes between two starts including in dev mode.
-     * <br>
-     * Access this generated UUID using expressions: `${quarkus.uuid}`.
-     */
-    Optional<String> uuid();
-


Does it actually change something to remove this? Asking because it's the only way to document it in the config.

If a property is declared in a mapping (or the old root), it is always eagerly populated. It wouldn't matter if quarkus.uuid is lazy. This is always trigger the load.

OK. I suppose we need to make sure it's properly documented elsewhere then.

I think we need to do it manually. We don't have a way to do it in the processor. We can ignore properties for documentation, but we can't ignore properties for mapping.

gsmet · 2025-01-08T09:03:11Z

core/runtime/src/main/java/io/quarkus/runtime/configuration/RuntimeConfigBuilder.java

@@ -12,7 +15,38 @@ public class RuntimeConfigBuilder implements SmallRyeConfigBuilderCustomizer {
    @Override
    public void configBuilder(final SmallRyeConfigBuilder builder) {
        new QuarkusConfigBuilderCustomizer().configBuilder(builder);
-        builder.withDefaultValue("quarkus.uuid", UUID.randomUUID().toString());
+        builder.withSources(new ConfigSource() {


Looks interesting. If the ConfigSource are kept around at runtime, we might want to move this to a static nested class to avoid keeping the builder reference around?

That being said, better to wait for @radcortez 's feedback before polishing.

+1 on all counts

If we want a quick solution, I'm okay with moving forward with this.

My idea was to add some sort of lazy type that can be declared in the mapping. This would allow keeping the declaration for documentation purposes and flattening sources if there are other cases.

Alternatively, we could just do this as a build step for the extensions that need it (it would have worked if it was done that way when it was first introduced). Unfortunately, we can't tell if users are now relying on it outside of the required extensions.

If we want a quick solution, I'm okay with moving forward with this.

That's totally up to you. The reason I vote for having this now is that we usually do the performance optimization in sprints (I don't mean the Agile sense of the word) and if we don't get this in now, who knows when we'll get around to it).

In any case, I updated the PR to use an inner class instead of an anonymous one.

I think it's worth merging this but I also agree having a lazy type in SmallRye Config would be nice for the future and we could revisit this patch then.

BTW, another alternative (breaking) is to totally drop this from the Config system and add a specific API for it (this is similar to the issue we have with ports).

I thought of that, but this property seems pretty pervasive so I didn't want to cause such problems until after the next LTS :)

gsmet · 2025-01-08T19:49:44Z

FWIW, the fault tolerance issue is unrelated, it popped up in other PRs. I pinged @Ladicek with the details in another PR.

geoand · 2025-01-08T19:51:19Z

🙏

radcortez · 2025-01-08T23:10:13Z

FWIW, the fault tolerance issue is unrelated, it popped up in other PRs. I pinged @Ladicek with the details in another PR.

Ah, good to hear, I was going crazy thinking I caused it in another PR :)

gsmet · 2025-01-09T08:45:16Z

We need to be careful though as it gets the build stuck so PRs are not properly tested: anything beyong these tests is not exercised...

This is done because bootstrapping the plumbing needed by the JDK to produce a UUID value is expensive, it thus doesn't make sense to pay this cost when the property isn't actually needed

quarkus-bot · 2025-01-09T20:11:22Z

Status for workflow `Quarkus CI`

This is the status report for running Quarkus CI on commit 8a1ab70.

✅ 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 21

📦 extensions/infinispan-cache/deployment

✖ io.quarkus.cache.infinispan.InfinispanCacheTest.testGetAsyncWithParallelCalls - History

expected: "thread1" but was: "thread2" - org.opentest4j.AssertionFailedError

org.opentest4j.AssertionFailedError: 

expected: "thread1"
 but was: "thread2"
	at io.quarkus.cache.infinispan.InfinispanCacheTest.testGetAsyncWithParallelCalls(InfinispanCacheTest.java:283)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at io.quarkus.test.QuarkusUnitTest.runExtensionMethod(QuarkusUnitTest.java:513)
	at io.quarkus.test.QuarkusUnitTest.interceptTestMethod(QuarkusUnitTest.java:427)

⚙️ JVM Tests - JDK 17 Windows

📦 integration-tests/grpc-hibernate

✖ com.example.grpc.hibernate.VertxBlockingRawTest.shouldAdd - History

Condition with Lambda expression in com.example.grpc.hibernate.BlockingRawTestBase was not fulfilled within 30 seconds. - org.awaitility.core.ConditionTimeoutException

org.awaitility.core.ConditionTimeoutException: Condition with Lambda expression in com.example.grpc.hibernate.BlockingRawTestBase was not fulfilled within 30 seconds.
	at org.awaitility.core.ConditionAwaiter.await(ConditionAwaiter.java:167)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:78)
	at org.awaitility.core.CallableCondition.await(CallableCondition.java:26)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:1006)
	at org.awaitility.core.ConditionFactory.until(ConditionFactory.java:975)
	at com.example.grpc.hibernate.BlockingRawTestBase.shouldAdd(BlockingRawTestBase.java:59)
	at java.base/java.lang.reflect.Method.invoke(Method.java:569)

geoand · 2025-01-15T10:51:57Z

It seems like there are plenty more places where UUID.randomUUID() is used during startup that would need to be "fixed" as well.

Follow up of: quarkusio#45434

quarkus-bot bot added area/config area/core labels Jan 8, 2025

geoand mentioned this pull request Jan 8, 2025

Load quarkus.uuid lazily #33941

Closed

geoand changed the title ~~Make production of quarkus.uuid lazy~~ Produce quarkus.uuid lazily Jan 8, 2025

gsmet reviewed Jan 8, 2025

View reviewed changes

geoand mentioned this pull request Jan 8, 2025

Places to potentially find startup performance improvements #45436

Open

geoand force-pushed the quarkus.uuid branch from aef3a1f to 83e1b0f Compare January 8, 2025 12:45

radcortez approved these changes Jan 8, 2025

View reviewed changes

geoand marked this pull request as ready for review January 8, 2025 13:21

geoand added the triage/waiting-for-ci label Jan 8, 2025

gsmet approved these changes Jan 8, 2025

View reviewed changes

This comment has been minimized.

Sign in to view

gsmet force-pushed the quarkus.uuid branch from 83e1b0f to 228e98e Compare January 9, 2025 09:14

This comment has been minimized.

Sign in to view

quarkus-bot bot added the triage/flaky-test label Jan 9, 2025

geoand force-pushed the quarkus.uuid branch from 228e98e to 8a1ab70 Compare January 9, 2025 14:43

geoand merged commit 62bdeaa into quarkusio:main Jan 10, 2025
52 checks passed

quarkus-bot bot added this to the 3.18 - main milestone Jan 10, 2025

geoand deleted the quarkus.uuid branch January 10, 2025 04:41

quarkus-bot bot removed the triage/waiting-for-ci label Jan 10, 2025

This was referenced Jan 10, 2025

JFRTest.jfrPerfTest and RuntimesSmokeTest.quarkusFullMicroProfile fails due to warning about quarkus.uuid Karm/mandrel-integration-tests#307

Closed

Native apps throw a warning now that quarkus.uuid is lazily produced #45495

Closed

geoand mentioned this pull request Jan 15, 2025

Remove the use of UUID in startup code #45610

Merged

imperatorx mentioned this pull request Jan 26, 2025

3.18 regression: quarkus.uuid is the same for every run in native images #45863

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Produce `quarkus.uuid` lazily #45434

Produce `quarkus.uuid` lazily #45434

geoand commented Jan 8, 2025 •

edited

Loading

franz1981 commented Jan 8, 2025

geoand commented Jan 8, 2025

gsmet left a comment

gsmet Jan 8, 2025

radcortez Jan 8, 2025

gsmet Jan 8, 2025

radcortez Jan 8, 2025

gsmet Jan 8, 2025

geoand Jan 8, 2025

radcortez Jan 8, 2025

geoand Jan 8, 2025

geoand Jan 8, 2025

gsmet Jan 8, 2025

radcortez Jan 8, 2025

geoand Jan 8, 2025

This comment has been minimized.

gsmet commented Jan 8, 2025

geoand commented Jan 8, 2025

radcortez commented Jan 8, 2025

gsmet commented Jan 9, 2025

This comment has been minimized.

quarkus-bot bot commented Jan 9, 2025 •

edited by github-actions bot

Loading

geoand commented Jan 15, 2025

Produce quarkus.uuid lazily #45434

Produce quarkus.uuid lazily #45434

Conversation

geoand commented Jan 8, 2025 • edited Loading

franz1981 commented Jan 8, 2025

geoand commented Jan 8, 2025

gsmet left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment has been minimized.

gsmet commented Jan 8, 2025

geoand commented Jan 8, 2025

radcortez commented Jan 8, 2025

gsmet commented Jan 9, 2025

This comment has been minimized.

quarkus-bot bot commented Jan 9, 2025 • edited by github-actions bot Loading

Status for workflow Quarkus CI

Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 extensions/infinispan-cache/deployment

⚙️ JVM Tests - JDK 17 Windows

📦 integration-tests/grpc-hibernate

geoand commented Jan 15, 2025

Produce `quarkus.uuid` lazily #45434

Produce `quarkus.uuid` lazily #45434

geoand commented Jan 8, 2025 •

edited

Loading

quarkus-bot bot commented Jan 9, 2025 •

edited by github-actions bot

Loading

Status for workflow `Quarkus CI`