Upgrade to Oracle JDBC driver 23.4

[Sanne]

Helping @Felk with his PR #41949 , as native-image metadata got a bit tricky to handle.

Needs @tsegismont approval to ensure compatibility with the vert.x oracle pgclient. @DavideD could test this version too in Hibernate Reactive?

[quarkus-bot]

/cc @gsmet (hibernate-orm), @yrodiere (hibernate-orm)

[tsegismont]

@Sanne LGTM, thanks

I sent and merged eclipse-vertx/vertx-sql-client#1448

The test suite passes and the upgrade will be present in Vert.x 4.5.10

[Sanne]

Excellent, thanks!

[quarkus-bot]

Status for workflow Quarkus CI

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

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

[Sanne]

Davide told me it's working fine with Hibernate Reactive as well: good to merge.

[gsmet]

Nice work!

[zakkak]

FYI this update seems to have quite some impact on the image size, see below

Test	Total Size (MB)	Increase from last datapoint (%)	Mandrel Version	Quarkus Version	Previous Q version
jpa-oracle	109.5023	3.60%	Mandrel-23.1.3.1-Final	1a31077	8a5ffdb
reactive-oracle-client	80.0294	6.64%	Mandrel-23.1.3.1-Final	1a31077	8a5ffdb

I know there is probably not much (if any) to be done, but sharing so people are aware.

[Sanne]

Thanks @zakkak ! Very interesting.

I suspect something related to:

• 4181c28#diff-4e56842dfe3e884c6ef5b7abe0a47bb995437353b87356f0ee20a26c9b167a69R16

But as you say, not much we can do about that; even if we could, they intend this to be used so I'd not take responsibility to change it, it might affect our users from being able to get support by the Oracle team. We'll let the Oracle JDBC team know.

[mabartos]

For our Keycloak project, removing these conditionals[1] for "native-related" operations broke our code. Some time ago I provided a PR with the necessary changes for us[2], and now it's reverted again.

@Sanne We do not use native mode and executing the ExtendedCharactersSupport, and OracleMetadataOverrides should not be necessary for non-native apps, right?

[1] 19d9ed1#diff-1744c4a730f826e51f66dd21c87ab13fc749bddbc011373540bcb2afd1cdd2ddR9
[2] #35403

[Sanne]

@mabartos sorry about that - could you describe what's happening in detail? Why would it break your code?

It really shouldn't matter, and eventually we hope to remove all such conditionals: the "onlyIf" attribute should be deprecated as it leads to anti-patterns in extension development which is making it really hard to refactor the Quarkus core to prepare for further optimisations.

So in a way it's great that we're having this conversation - sorry for the inconvenience though!

cc/ @dmlloyd

[gsmet]

So for the OracleMetadataOverrides, I'm pretty sure the conditionals don't really make a difference. For the ExtendedCharactersSupport, I think it can be a problem as this step actually doesn't produce or consume any build item so my guess is that if it's executed (I thought we wouldn't execute it if not producing a consumed build item but maybe it is when there is a recorder), then it would be executed always which would be a problem.

I'm going to revert 19d9ed1 for now until things can be properly asserted.

[mabartos]

@Sanne @gsmet Thanks for addressing the issue and for the revert and sorry for quite comprehensive problem description below :)

I understand your concerns about anti-patterns in extensions, but our current Keycloak use case kinda requires us to do things a little bit differently. Ignoring these build steps for a "native" executable is only the tip of the iceberg and I need to admit it's part of our "hacks" to your extensions eco-system :)

Story

EDIT: mentioned term /lib below is the actual value for quarkus.package.output-directory property

For Keycloak, we use mutable-jar packaging and our lib/ folder contains all the necessary JARs to run the application. It means users might reaugment Keycloak by changing build options to use different JDBC drivers, enable/disable features, etc. AFAIK, your extensions process handles all build steps found in the present Quarkus extensions as part of /deployment folder.

This is an issue when using Quarkus JDBC extensions. We provide support for several JDBC drivers (h2, postgres, mariadb, oracle,...), so it means we have included their Quarkus JDBC extensions in the distribution (in /lib). In normal cases, when a user wants to use only PostgreSQL DB, build steps for quarkus-jdbc-postgresql-deployment extension are executed.
However, even build steps for other included JDBC extensions are executed.

To mitigate the issue, we do certain "hacks" like visible in example: if a user wants to use only PostgreSQL DB, they set --db=postgres build option, and then we ignore all artifacts for other JDBC extensions by leveraging quarkus.class-loading.removed-artifacts property. These artifacts are then ignored by class loaders and we're good.

Problem with Oracle JDBC

To comply with CNCF policies, we are not able to deliver Keycloak distribution with non-Apache 2.0 dependencies as stated in this issue. It means we do not include com.oracle.database.jdbc:ojdbc11, and com.oracle.database.nls:orai18n JARs and if a user wants to use Oracle DB, they need to explicitly add them to the distribution.

Problem with removed conditionals in the build steps

As I've pointed out in the previous comment, it broke our codebase as during building our distribution, we do not ignore these artifacts, and build steps for Quarkus Oracle JDBC extension were executed. As we do not use native builds, these additional build steps are executed now. However, they use directly code for Oracle JDBC, but these JARs are not provided in the distribution, and the build fails.

In that case, we'd like to have the condition back (at least for now) to not execute these build steps. I know it's a short-term solution, but it'll help us to save some time before there's a better approach.

How Quarkus can support us?

As I've mentioned, we probably use Quarkus extensions in a way that is not completely aligned with the Quarkus extensions principles, but we do not have more options right now.

What could really help us would be to have some mechanism, for how we can disable Quarkus extensions even when they're present in /lib. Having some build time option to specify which extensions should be disabled and all their build steps would be skipped. Moreover, it'd be great if class loaders would even ignore these artifacts and their transitive ones. It'd behave the same as these extensions would never be present "in the world", meaning like not loaded to memory, not found by service loaders or leveraging reflection.

@geoand Do you think it might be feasible in some way? Have you discussed anything like this in the past?

cc: @vmuzikar

[geoand]

when they're present in /lib

Which /lib are we talking about?

What could really help us would be to have some mechanism

AFAIR, there has never been such a need nor has it been discussed in the past to the best of my knowledge. Maybe @aloubyansky remembers something?

[yrodiere]

What could really help us would be to have some mechanism

AFAIR, there has never been such a need nor has it been discussed in the past to the best of my knowledge. Maybe @aloubyansky remembers something?

Not sure which need you're referring to, but the need to disable extensions at build time has been mentioned before, and even implemented for Hibernate extensions. There's a whole issue about making this more consistent, but I stopped after the analysis... #42244

[geoand]

Not sure which need you're referring to, but the need to disable extensions at build time has been mentioned before, and even implemented for Hibernate extensions

How does that work?

[aloubyansky]

Moreover, it'd be great if class loaders would even ignore these artifacts and their transitive ones.

To me it sounds like what would be desirable is to bundle some Maven repo and resolve requested dependencies and the ApplicationModel from that before (re)augmentation.

[geoand]

Yeah, it sounds like something general, not something each one extension would need to be coded against

[mabartos]

Which /lib are we talking about?

@geoand Ahh, sorry. It's our value for quarkus.package.output-directory property.

To me it sounds like what would be desirable is to bundle some Maven repo and resolve requested dependencies and the ApplicationModel from that before (re)augmentation.

+1

[aloubyansky]

Perhaps the mutable-jar distribution could be re-worked that way and then slimming down could also be easier. I.e. content from the "bundled Maven repo" could be removed w/o affecting bootstrap as long as those dependencies are not used, unlike with the current approach.

[Sanne]

However, they use directly code for Oracle JDBC, but these JARs are not provided in the distribution, and the build fails.

In this case, the conditional evaluation should be based on the removed code to actually exist. That would make more sense and also give you less-so problematic side effects should you want to build a native image.

[geoand]

Perhaps the mutable-jar distribution could be re-worked that way and then slimming down could also be easier. I.e. content from the "bundled Maven repo" could be removed w/o affecting bootstrap as long as those dependencies are not used, unlike with the current approach.

Yeah, I think this warrants some rethinking of the mutable-jar approach

[yrodiere]

Not sure which need you're referring to, but the need to disable extensions at build time has been mentioned before, and even implemented for Hibernate extensions

How does that work?

Currently, a quarkus.hibernate-orm.enabled property that is used as a build-step condition (onlyIf) at the HibernateOrmProcessor level.

I completely agree a more general approach would be better. But I'd suggest not limiting yourself to mutable jars -- disabling extensions that happen to be in the classpath in a "simple" (no reaugmentation) build can be useful too.

I think the most common use case I've seen is users deploying a same application multiple times, with different profiles, where one deployment/profile uses a database but another doesn't (it disables that part of the app).

[geoand]

I completely agree a more general approach would be better. But I'd suggest not limiting yourself to mutable jars -- disabling extensions that happen to be in the classpath in a "simple" (no reaugmentation) build can be useful too.

If we can find a non intrusive way to do that that is also not a terrible user experience, it would be great

[mabartos]

Thanks for your input! We should create a new issue for it to have it properly traced.

@aloubyansky @geoand @yrodiere @Sanne Should I create it based on our "requirements", or will you do it with your additional findings aggregated together?

[aloubyansky]

If we are considering a significantly different distribution and bootstrap for reaugmentation, I'd consider leaving the mutable-jar alone and introducing a new one to satisfy the new requirements which should be collected.

[mabartos]

If we are considering a significantly different distribution and bootstrap for reaugmentation, I'd consider leaving the mutable-jar alone and introducing a new one to satisfy the new requirements which should be collected.

Wouldn't be possible to first provide some mechanism to disable a Quarkus extension for mutable-jar based on a build property? The same as quarkus.hibernate-orm.enabled(mentioned by @yrodiere) for all extensions? Having build time option quarkus.<extension-name>.enabled(default true) which would just skip executing build steps.

IMO, it'd be a good starting point and probably would not require so much effort. WDYT?