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.

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

PostgreSQL search path not restored properly after migrations when auto-comit == false. #1959

Closed
jezovuk opened this issue Mar 19, 2018 · 11 comments

Comments

@jezovuk
Copy link

jezovuk commented Mar 19, 2018

Which version and edition of Flyway are you using?

Flyway 5.0.7.

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)

Initially noticed with 5.0.5, repeated with 5.0.7. Seems problematic with current code in master (only from code inspection, I haven't actually tried it).

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

Java API.

Which database are you using (type & version)?

PostgreSQL 9.6

Which operating system are you using?

Windows 10.

What did you do?

(Please include the content causing the issue, any relevant configuration settings, the SQL statement that failed (if relevant) and the command you ran.)

Migrating multiple db schemas via multiple Flyway instances (using same DataSource instance - same database user, same database config etc.).

Relevant part of code:

final Flyway flyway = new Flyway();
flyway.setDataSource(dataSource);
flyway.setTable("flyway_metadata");
final String schemaName = dbSchema().getName();
flyway.setLocations("db.migration." + schemaName);
flyway.setSchemas(schemaName);
flyway.migrate();

Code above is common for several modules/classes, all using same (Spring-injected) DataSource instance, and each its own db schema (dbSchema() returns jOOQ Schema instance).

We're running database migrations for all system modules upon system startup, sequentially (no parallel execution). Several db schemas contain views with same name (v_output_data). Typical db migration creating v_output_data view (repeatable migration) is of following form:

drop view if exists v_output_data cascade;

create view v_output_data as
<defining select>;

We're using drop + create instead of create or replace since PostgreSQL doesn't allow reducing number/changing type of view columns via create or replace.

What did you expect to see?

The expectation was that DB migrations for each schema were completely isolated from each other (and hence that drop view was perfectly safe even for newly created schemas).

What did you see instead?

In this specific scenario, we already had one existing schema (let's call it A) in the DB with existing view v_output_data (and no further migrations to be applied) and one new schema (B) with repeatable migration for view creation (formed as above - drop view is redundant for first run, but was there for consistency and future use).

What happened was following:

  • b.v_output_data was created as expected
  • a.v_output_data was dropped - this was not expected!

After some debugging we found out that the problem is that restoring of current schema in DbMigrate.migrate() (connectionUserObjects.restoreCurrentSchema() in finally block at the end of the method) is executed non-transactionally. PostgreSQL's set search_path is transaction-aware.
The other important piece of the puzzle is the fact that for PG current schema is actually set by prepending desired schema name to existing search path (PostgreSQLConnection.changeCurrentSchemaTo()).
Finally, we have configured our data source not to use auto-commit, and all non-transactional changes are lost once connection is closed (which happens in finally block near the end of Flyway.execute(), when Database object is closed).

In our case, migrations for schema A were checked first (no migration needed, but search path transactionally set to start with A and not restored properly). After that, migrations for schema B were appied, with search path set to B,A,<default-search-path>. At this point, drop view in migration script intended for B dropped the view from A (since the view has not yet been created in B).

I've checked current state of related code here on GitHub, and although it is refactored quite a bit, the problem seems to still be there (restoring is done in Connection.close(), still no transaction).

@axelfontaine
Copy link
Contributor

We completely reimplemented search_path support on the master branch as part of the changes for #1926.

I am unable to reproduce the scenario you described here with that code.

Please build the latest sources, try again and confirm it is now working for you.

@jezovuk
Copy link
Author

jezovuk commented Apr 4, 2018

Same problem with latest sources (cloned few hours ago from master).

@axelfontaine
Copy link
Contributor

axelfontaine commented Apr 4, 2018

Thanks for checking. Then we must be missing something in our reproduction test case, as in our tests the search_path is always properly restored. How exactly is your datasource configured? Or even better, could you share a small repo that reproduces the problem?

@jezovuk
Copy link
Author

jezovuk commented Apr 4, 2018

Datasource is configured via Spring Boot, using HikariCP (version 2.7.8).
Datasource-related configuration in application.properties is as follows:

spring.datasource.url=jdbc:postgresql://localhost:5432/MYDB
spring.datasource.username=user
spring.datasource.password=pass
spring.datasource.hikari.initialization-fail-timeout=0
spring.datasource.hikari.connection-timeout=600000
spring.datasource.hikari.auto-commit=false

Flyway object is configured as listed earlier, using dataSource object injected by Spring.

From my understanding, the problem (with latest code) is in following:

org.flywaydb.core.Flyway.migrate() 
> org.flywaydb.core.Flyway.execute(Command<T>)
     org.flywaydb.core.internal.database.Database<C> instance created here
     near the end of execute(), in finally block, database.close() is called
> org.flywaydb.core.internal.database.Database.close()
     migrationConnection.close() is called from Database.close()
> org.flywaydb.core.internal.database.Connection.close()
    doChangeCurrentSchemaOrSearchPathTo(originalSchemaNameOrSearchPath) 
    called from Connection.close()
> org.flywaydb.core.internal.database.postgresql.PostgreSQLConnection.doChangeCurrentSchemaOrSearchPathTo(String)
    jdbcTemplate.execute("SELECT set_config('search_path', ?, false)", schema)
    is executed here; according to Postgres docs, set_config function corresponds to
    SET command (and is thus transaction-aware)

Since there is no transaction around entire database/connection closing, nor around doChangeCurrentSchemaOrSearchPathTo() execution, search path change is reverted/rolled-back when auto-commit == false.

The problem does not appear with following config change:

spring.datasource.hikari.auto-commit=true

If this doesn't help, I'll try to produce minimal test project.

@axelfontaine
Copy link
Contributor

@jezovuk Thanks for the details. Our tests actually already covered by autocommit and non-autocommit modes and didn't uncover the behavior you describe, so if you can, sharing a minimal test project would be awesome.

@jezovuk
Copy link
Author

jezovuk commented Apr 4, 2018

Will share tomorrow.

@jezovuk
Copy link
Author

jezovuk commented Apr 5, 2018

https://github.com/jezovuk/DemoForFlywayIssue1959

axelfontaine pushed a commit to flyway/flywaydb.org that referenced this issue Apr 6, 2018
@axelfontaine
Copy link
Contributor

Thank you so much for the sample project. That was immensely useful. Somehow we could only ever reproduce this with HikariCP and not with any of the other datasources we normally use. I still haven't fully figured out why, but at least we now have a reliable way to verify this. Fixed.

@jezovuk
Copy link
Author

jezovuk commented Apr 6, 2018

You're welcome. Thanks for fixing.

@jkenefick
Copy link

@jezovuk does this issue re-occur for you with the current version of flyway? It seems to have regressed for me.

@jezovuk
Copy link
Author

jezovuk commented Apr 9, 2020

Just tried the demo project linked above (https://github.com/jezovuk/DemoForFlywayIssue1959), with Flyway 6.3.2 & 6.3.3. Both seem to work fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants