Quartz - Defer driver discovery to runtime in order to provide more flexibility to users

[manovotn]

Resolves #48545

[manovotn]

Deliberately kept a draft for now as I have yet to verify this with a case similar to what was reported in the issue.
Our own testing does not contain a dual DB setup.

EDIT: I have tested this with the reproducer provided by the user (see this comment) and it passes.

[manovotn]

I am aware that this PR does not move the build time config property into runtime config because I am frankly not sure if that would be breaking - technically it will have the same name but anything operating with the build time config might be affected by it. We could also temporarily duplicate them in both and deprecate one.

Anyhow, @mkouba thoughts on the approach in this PR and the config change? :)

[mkouba]

I can't say that I completely understand the use case 🤔.

If a user sets the quarkus.quartz.datasource then we should IMO still try to use this DS and fail at build time if not available. If the quarkus.quartz.datasource is not set (i.e. the default DS should be used) then we could defer the driver discovery to runtime. Am I missing something?

[manovotn]

I can't say that I completely understand the use case 🤔.

Take a look at the discussion on the issue - #48545 (comment)
The reproducer is also a nice depiction of it.

If a user sets the quarkus.quartz.datasource then we should IMO still try to use this DS and fail at build time if not available. If the quarkus.quartz.datasource is not set (i.e. the default DS should be used) then we could defer the driver discovery to runtime. Am I missing something?

Maybe I don't fully understand but it seems that wouldn't work here.
In this case, the reproducer contains ConfigSourceInterceptor - an interceptor for config value of quarkus.quartz.datasource. And it returns either postgresql or mssql based on which datasource is active.
And the activity of datasource is determined at runtime.
So whatever information about quarkus.quartz.datasource you read during build time might not reflect reality at runtime.

Of course it is a matter of how much runtime flexibility we want to give it. This PR is just my attempt at grasping the use case and coming up with something that would make the reproducer pass.

[mkouba]

I can't say that I completely understand the use case 🤔.

Take a look at the discussion on the issue - #48545 (comment) The reproducer is also a nice depiction of it.

If a user sets the quarkus.quartz.datasource then we should IMO still try to use this DS and fail at build time if not available. If the quarkus.quartz.datasource is not set (i.e. the default DS should be used) then we could defer the driver discovery to runtime. Am I missing something?

Maybe I don't fully understand but it seems that wouldn't work here. In this case, the reproducer contains ConfigSourceInterceptor - an interceptor for config value of quarkus.quartz.datasource. And it returns either postgresql or mssql based on which datasource is active. And the activity of datasource is determined at runtime. So whatever information about quarkus.quartz.datasource you read during build time might not reflect reality at runtime.

Of course it is a matter of how much runtime flexibility we want to give it. This PR is just my attempt at grasping the use case and coming up with something that would make the reproducer pass.

My point is - if a user sets quarkus.quartz.datasource=foo then the foo DS should be used no matter what. Currently, it's a build time property so we can validate the existence of DS etc. If no DS is set then we fall back to the default DS. Which is also fine.

Now, it seems that the user needs to define multiple data sources, deactivate some and select one for Quartz at runtime. This means we would need to relax the validation at build time. Which is 👎.

Also I think it's breaking because the behavior's changed. And the ConfigSourceInterceptor completely ignores the value set by the user which is IMO weird.

The question is: could we introduce two new config properties that would be used if no DS is selected at build time (quarkus.quartz.datasource is not used). I.e. something like quarkus.quartz.datasource.none-selected-strategy (buildtime) with values default - use the default DS (current behavior), defer - select the DS at runtime. And then quarkus.quartz.datasource.deferred-name (runtime), only used if quarkus.quartz.datasource.none-selected-strategy=defer, that could be set by the ConfigSourceInterceptor or even build item. WDYM?

[manovotn]

And the ConfigSourceInterceptor completely ignores the value set by the user which is IMO weird.

Yes but that's not what the reproducer does. In it, there is no value set by user and only the interceptor exists so that it can give you the config value "dynamically", so to speak.

Also I think it's breaking because the behavior's changed.

Breaking in what way? Currently you'd only not get as fast failure if you misconfigured it but otherwise it should appear to behave equally on the outside?

Now, it seems that the user needs to define multiple data sources, deactivate some and select one for Quartz at runtime. This means we would need to relax the validation at build time. Which is 👎.

It is not the first time we hear about the need to have multiple DS, it's just that Quartz bit didn't have to deal with it yet.
I agree that having as much as possible done in build time is the way to go of course.

The question is: could we introduce two new config properties that would be used if no DS is selected at build time (quarkus.quartz.datasource is not used). I.e. something like quarkus.quartz.datasource.none-selected-strategy (buildtime) with values default - use the default DS (current behavior), defer - select the DS at runtime. And then quarkus.quartz.datasource.deferred-name (runtime), only used if quarkus.quartz.datasource.none-selected-strategy=defer, that could be set by the ConfigSourceInterceptor or even build item. WDYM?

Hm, interesting idea. However, in such a case, wouldn't it be easier to keep quarkus.quartz.datasource as is and just add something like quarkus.quartz.datasource.defer-resolution (default false of course)? The latter would just control whether we attempt to read and resolve the datasource property in build or runtime.
As far as I am concerned, the less configuration knobs, the better :)

[mkouba]

However, in such a case, wouldn't it be easier to keep quarkus.quartz.datasource as is as is...

The idea is to keep quarkus.quartz.datasource as is, which means a build time config property.

The latter would just control whether we attempt to read and resolve the datasource property in build or runtime.

I don't think it's a good idea. You can't treat a build config property as runtime and vice versa. They are processed differently. I.e. there is no way to tell Quarkus that quarkus.quartz.datasource is a build time property when quarkus.quartz.datasource.defer-resolution=false.

[manovotn]

You can't treat a build config property as runtime and vice versa.

Well, it is available in both and we'd mostly process it in build time anyway but I get that it's not right on the theoretical level.
Guess I am simply not a fan of having two extra properties just for this one special case.

Either way, I will revert this to draft, circle back to it early next week and try to come up with something more profound :)

[mkouba]

Well, it is available in both and we'd mostly process it in build time anyway but I get that it's not right on the theoretical level.

There is more to it. Runtime and build-time properties are processed differently which might lead to strange and/or well-hidden errors. There are things like RunTimeConfigurationDefaultBuildItem etc. 🤷

[manovotn]

I've reworked the PR and there are now two new config knobs:

• Build time boolean config deferDatasourceCheck (default false) which can be used to indicate that DS resolution should be deferred
  • Combining this with the definition of DS at build time results in an exception
• Runtime config deferredDatasourceName (String) which can be used to specify the name of the DS at runtime
  • This will only be used if the resolution wasn't completed in build time already

Furthermore the classes registered for reflection in case of deferred resolution are now curated based on the set of known JDBC DS build items (as per Guillaume's comment).

I've also tested this with the user provided dual-DB reproducer and got a pass.

[manovotn]

The CI failure is definitely related, taking a look...

[github-actions]

🎊 PR Preview 4745e81 has been successfully built and deployed to https://quarkus-pr-main-48579-preview.surge.sh/version/main/guides/

• Images of blog posts older than 3 months are not available.
• Newsletters older than 3 months are not available.

[mkouba]

It looks very good but I wonder if we shouldn't add a test as well?

[manovotn]

It looks very good but I wonder if we shouldn't add a test as well?

Done, the following changes are now part of the PR:

• Additional config validation for when runtime DS name is missing and should be present
• Tests for invalid combinations of config knobs
  • Defining a build time DS name and using the defer config at the same time
  • Deferring resolution and not defining the runtime DS name
• Integration test (as a separate commit) with all the knobs for deferred DS setup
  • Unlike the original reproducer, this is simplified and uses only a single non-default DB/DS setup
  • Test-wise this is a clone of the existing quartz module, the idea is to assert the same but use runtime DS setup
  • This also exposed a needed extra change in QuartzSchedulerImpl which now correctly recognizes the runtime DS config

[manovotn]

Addressed all of the comments and rebased onto main.

[quarkus-bot]

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit c87230c.

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

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

[quarkus-bot]

Status for workflow Quarkus CI

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

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

---

Flaky tests - Develocity

⚙️ JVM Integration Tests - JDK 17

📦 integration-tests/kafka-devservices

✖ io.quarkus.it.kafka.continuoustesting.DevServicesDevModeTest.testDevModeServiceDoesNotRestartContainersOnCodeChange - History

• New containers: [Container(command=sh -c 'while [ ! -f /work/run.sh ]; do sleep 0.1; done; sleep 0.1; /work/run.sh', created=1751977687, id=4e4d575b3879b97b7fd6a08f04afb3075abbbf41f825405b85cb971510550029, image=quay.io/ogunalp/kafka-native:latest, imageId=sha256:99fa71388449ac7ff46bda7f6d645fb4435d9a21dce3d4dbcaedfc8a738afd6b, names=[/nervous_kalam], ports=[ContainerPort(ip=0.0.0.0, privatePort=9092, publicPort=32859, type=tcp), ContainerPort(ip=::, privatePort=9092, publicPort=32859, type=tcp)], labels={architecture=x86_64, build-date=2025-03-25T21:45:00Z, com.redhat.component=ubi9-micro-container, com.redhat.license_terms=https://www.redhat.com/en/about/red-hat-end-user-license-agreements\#UBI, description=Very small image which doesn't install the package manager., distribution-scope=public, io.buildah.version=1.39.0-dev, io.k8s.description=Very small image which doesn't install the package manager., io.k8s.display-name=Red Hat Universal Base Image 9 Micro, io.openshift.expose-se... - org.opentest4j.AssertionFailedError

org.opentest4j.AssertionFailedError: 
New containers: [Container(command=sh -c 'while [ ! -f /work/run.sh ]; do sleep 0.1; done; sleep 0.1; /work/run.sh', created=1751977687, id=4e4d575b3879b97b7fd6a08f04afb3075abbbf41f825405b85cb971510550029, image=quay.io/ogunalp/kafka-native:latest, imageId=sha256:99fa71388449ac7ff46bda7f6d645fb4435d9a21dce3d4dbcaedfc8a738afd6b, names=[/nervous_kalam], ports=[ContainerPort(ip=0.0.0.0, privatePort=9092, publicPort=32859, type=tcp), ContainerPort(ip=::, privatePort=9092, publicPort=32859, type=tcp)], labels={architecture=x86_64, build-date=2025-03-25T21:45:00Z, com.redhat.component=ubi9-micro-container, com.redhat.license_terms=https://www.redhat.com/en/about/red-hat-end-user-license-agreements#UBI, description=Very small image which doesn't install the package manager., distribution-scope=public, io.buildah.version=1.39.0-dev, io.k8s.description=Very small image which doesn't install the package manager., io.k8s.display-name=Red Hat Universal Base...

---

⚙️ JVM Integration Tests - JDK 21

📦 integration-tests/kafka-devservices

✖ io.quarkus.it.kafka.continuoustesting.DevServicesDevModeTest.testDevModeServiceDoesNotRestartContainersOnCodeChange - History

• expected: <false> but was: <true> - org.opentest4j.AssertionFailedError

org.opentest4j.AssertionFailedError: expected: <false> but was: <true>
	at org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
	at org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
	at org.junit.jupiter.api.AssertFalse.failNotFalse(AssertFalse.java:63)
	at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:36)
	at org.junit.jupiter.api.AssertFalse.assertFalse(AssertFalse.java:31)
	at org.junit.jupiter.api.Assertions.assertFalse(Assertions.java:231)
	at io.quarkus.it.kafka.continuoustesting.DevServicesDevModeTest.testDevModeServiceDoesNotRestartContainersOnCodeChange(DevServicesDevModeTest.java:81)