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.

Sign up for GitHub

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

Jump to bottom

JdbcUtils#openConnection sets unrelated polyglot.engine.WarnInterpreterOnly property #3882

Closed
myazinn opened this issue Apr 26, 2024 · 6 comments
Closed

JdbcUtils#openConnection sets unrelated polyglot.engine.WarnInterpreterOnly property #3882

myazinn opened this issue Apr 26, 2024 · 6 comments

Comments

@myazinn
Copy link

myazinn commented Apr 26, 2024 •
edited
Loading

Which version and edition of Flyway are you using?

10.11.1

If this is not the latest version, can you reproduce the issue with the latest one as well? (Many bugs are fixed in newer releases and upgrading will often resolve the issue)

It is the latest one

Which client are you using? (Command-line, Java API, Maven plugin, Gradle plugin)

Java API

Which database are you using? (Type & version)

Postgresql 15.5

Which operating system are you using?

Doesn't matter

What did you do? (Please include the content causing the issue, any relevant configuration settings, the SQL statement(s) that failed (if any), and the command you ran)

In our project, we use flyway and a bunch of graalvm libs

groupId = "org.graalvm.sdk",     artifactId = "graal-sdk"
groupId = "org.graalvm.js",      artifactId = "js"
groupId = "org.graalvm.truffle", artifactId = "truffle-api"

of version 21.1.0.
With newer versions of flyway our service stopped working because org.graalvm.polyglot.Context.Builder can no longer create a org.graalvm.polyglot.Context and fails with the following error

Could not find option with name engine.WarnInterpreterOnly.

This flag was introduced in 21.2.0, while we use 21.1.0. There are reason why we use this particular old version, so unfortunately we cannot just bump it. We were surprised to find out that this flag is being set by flyway for Postgres, which doesn't have any graalvm-related dependencies.
It seems the issue was introduced here. The commit message states that it was intended to set property to hide warning from polyglot engine, but why? Why would one library set a system property that belongs to other, completely unrelated, library?
I see there was an issue regarding this flag related to MongoDB. Perhaps it makes sense for Mongo specifically (even though sounds weird to me), but why do this in JdbcUtils?

What did you expect to see?

That everything works fine

What did you see instead?

For some bizarre reason org.flywaydb.core.internal.jdbc.JdbcUtils#openConnection sets polyglot.engine.WarnInterpreterOnly system property to "false".
Making matters worse, it's not deterministic and depends on the initialisation order. If org.graalvm.polyglot.Context happens to be created before we open any DB connection, that everything works fine. Otherwise, we get an error.
In our case, we had to remove this property manually (System.clearProperty("polyglot.engine.WarnInterpreterOnly")) to make it work. But that's neither a good workaround, nor a real solution.
@akenO8 akenO8 mentioned this issue Apr 30, 2024
Open
@Barry-RG
Copy link
Contributor

Thank you for your issue. The reason is due to the MongoDB driver use by Flyway requiring that option to be set global. As Flyway does not usualy interact with GraalVM it was not seen as an issue initially. However, I appreciate the way you are using Flyway has hit this issue and I will raise a ticket in our development pipeline to deal with this appropriately.

@selliera
Copy link

Oh, I just got caught by this, because I do have code that tries to reset properties between tests. It does this using System.clearProperty, and at least with java17, results in:

java.lang.ClassCastException: java.lang.Boolean incompatible with java.lang.String
	at java.base/java.lang.System.clearProperty(System.java:956)

As this method assumes the property values are strings.

@jazdw
Copy link

jazdw commented Aug 6, 2024

It seems the issue was introduced here. The commit message states that it was intended to set property to hide warning from polyglot engine, but why? Why would one library set a system property that belongs to other, completely unrelated, library?
I see there was an #3856 regarding this flag related to MongoDB. Perhaps it makes sense for Mongo specifically (even though sounds weird to me), but why do this in JdbcUtils?

I totally agree that this is weird and shouldn't something that Flyway should be doing. Caught us out, we use Graal but not Mongo within our application and we started getting this exception -

java.lang.IllegalArgumentException: Invalid boolean option value 'null'. The value of the option must be 'true' or 'false'.
	at org.graalvm.options.OptionType$4.apply(OptionType.java:219) ~[?:?]
	at org.graalvm.options.OptionType$4.apply(OptionType.java:212) ~[?:?]
	at org.graalvm.options.OptionType$3.convert(OptionType.java:105) ~[?:?]
	at org.graalvm.options.OptionType.convert(OptionType.java:186) ~[?:?]
	at com.oracle.truffle.polyglot.OptionValuesImpl.put(OptionValuesImpl.java:153) ~[?:?]
	at com.oracle.truffle.polyglot.OptionValuesImpl.putAll(OptionValuesImpl.java:129) ~[?:?]
	at com.oracle.truffle.polyglot.PolyglotImpl.createEngineOptions(PolyglotImpl.java:325) ~[?:?]
	at com.oracle.truffle.polyglot.PolyglotImpl.buildEngine(PolyglotImpl.java:257) ~[?:?]
	at org.graalvm.polyglot.Engine$Builder.build(Engine.java:625) ~[?:?]
	at com.oracle.truffle.js.scriptengine.GraalJSScriptEngine.<init>(GraalJSScriptEngine.java:310) ~[?:?]
	at com.oracle.truffle.js.scriptengine.GraalJSScriptEngine.create(GraalJSScriptEngine.java:758) ~[?:?]

Seems like the issue of it being set as a boolean instead of a string was at least fixed in version 10.11.1 see b674678

@JasonLuo-Redgate
Copy link

Issue fixed in Flyway 10.11.1: set System Properties as String rather than Boolean
https://documentation.red-gate.com/flyway/release-notes-and-older-versions/release-notes-for-flyway-engine

@myazinn
Copy link
Author

myazinn commented Sep 17, 2024 •
edited
Loading

@JasonLuo-Redgate respectfully, it is not. I am using 10.11.1. The issue is not about which value is being set as a system property, it's about why the value is being set in the first place.
Is there any good reason why jdbc driver messes with polyglot.engine properties even though they are completely unrelated?

@JasonLuo-Redgate
Copy link

Hi @myazinn ,

The reason is due to the MongoDB driver use by Flyway requiring that option to be set globally.
We plan to refactor the current MongoDB support with a new solution, which will eventually eliminate the need to set this option.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@selliera @jazdw @myazinn @Barry-RG @JasonLuo-Redgate