Flyway transactional lock property

[OleKsimov]

Description

Please consider adding new Flyway property to Quarkus configuration with default value set to "false" A discussion about this property and issue with its current default (true) value can be found here

Implementation ideas

No response

[quarkus-bot[bot]]

/cc @cristhiank, @gastaldi, @geoand, @gsmet

[OleKsimov]

This feature request is based on this issue

[geoand]

So this is a postgresql specific configuration?

[OleKsimov]

I believe it's Flyway-specific property to choose what lock should be used: session or transactional. I would say some problems with default value (true = transactional advisory locks) appear because of Agroal connection pooling implementation. You can find a reproducer in the issue I mentioned before.

[geoand]

Yeah, but my point is that in Flyway this property seems to be PostgreSQL specific.

[OleKsimov]

Yeah, you are right, it is obviously Postgres configuration property only.

[Philipp-Borchert-ISH]

I think this could be a blocker issue for some people running quarkus-flyway + postgres. As described in the links to flyway docs with "out of the box" flyway you can bypass this by setting a system property, however that doesn't seem to work with quarkus-flyway because there is no call to FluentConfiguration::configuration or ClassicConfiguration::configure from FlywayCreator::createFlyway.

I've tested setting the system property for dev mode / test lifecycle via maven configuration (quarkus-maven-plugin/configuration/systemProperties + maven-surefire-plugin/configuration/systemPropertyVariable) but that doesn't work due to the way the config is loaded in quarkus-flyway, i don't think there is any alternative workaround other than not upgrading quarkus :-(

[geoand]

Yeah, I think we'll need to provide the property. Do you agree @gsmet @gastaldi ?

[gsmet]

Yup. Sounds like a good opportunity for a pull request :)

[gastaldi]

+1, this would be nice to have

[geoand]

I've added the good first issue tag for anyone that wants to pick it up.

[gsmet]

My advice would be to add a postgresql config group then a property inside it to mimic the Flyway structure and be ready in case they add more.

[jcsim1024]

Hi @geoand If it's not already taken care of, could I give this a try ?

[geoand]

@jcsim1024 go right ahead!

[gsmet]

@jcsim1024 have a look at this comment before starting your work: https://github.com/quarkusio/quarkus/issues/28502#issuecomment-1277916719

Thanks!

[geoand]

@jcsim1024 just checking in, how is this going? Do you need any help?

[piyush-1234]

hi all, i want to pick up this issue if you guys would not mind, I m new here and wanted to start contributing to quarkus,

[geoand]

Hi @piyush-1234,

Thanks for the offer. Let's see how far @jcsim1024 has got

[piyush-1234]

thanks @geoand for taking me in consideration

[jcsim1024]

Hello @geoand I am still reading the doc and trying to reproduce

[OleKsimov]

@jcsim1024 The reproducer is present in this issue if it helps.

[piyush-1234]

@geoand & @OleKsimov, I m working on this parallel. any suggestion?

[jcsim1024]

Hello @geoand, I am following the "create a config group and then add a property" advice. I am trying to find an elegant way to configure PostgreSQLConfigurationExtension then I'll make the test work. Any suggestion is welcomed https://github.com/quarkusio/quarkus/compare/main...jcsim1024:quarkus:main

[geoand]

@jcsim1024 you should not have to populate the new config group, Quarkus will do that automatically, see this for more information.

[jcsim1024]

Just to keep you up to date, I have not made a lot of progress this weekend , I've mostly read the documentation I'll try to have something working this sunday

[jcsim1024]

Update: I now have a working config group

• I need to add IT test
• I still have to know when database is postgres in public Flyway createFlyway(DataSource dataSource)

[jcsim1024]

I added the IT test in a module different than flyway's IT tests as it already has H2 database in it. Is it ok or should I put it in a a specific flyway-postgres IT module? @geoand

I think I should investigate more on the root cause of the stuck migration when there is both a tansaction lock and a concurent operation.

[geoand]

We probably want to add a test like the ones you can see here.

[OleKsimov]

Hi guys! I just wanted to mention that this issue blocks the teams which use Flyway + Postgres. Is there anything that can be done to add priority to this issue? Two months are gone, and it would be critical to wait for another two :(

[geoand]

@jcsim1024 how is your work going? Do you need any help or should we perhaps take over?

Thanks

[jcsim1024]

Sorry I did not had a lot of free time this last 2 weeks, I'll have look on the tests this weekend

[jcsim1024]

Hello will add a maven submodule named postgres-extension in "integration-tests/flyway/" folder and move what's in it in h2-extension

[OleKsimov]

@geoand Is it possible to assign this issue to someone?

[geoand]

As this does has the potential to block a lot of users and since we would not want to wait too much longer for a fix, I will handle it

[geoand]

With https://github.com/quarkusio/quarkus/pull/30656, users will now be able to provide arbitrary customization for Flyway, one of which can be the one needed for this issue.

I think this design is cleaner than adding a postgres specific property.