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

Support Multiple Migrations within a single Transaction #181

Closed
flyway opened this issue Jun 25, 2013 · 34 comments
Closed

Support Multiple Migrations within a single Transaction #181

flyway opened this issue Jun 25, 2013 · 34 comments

Comments

@flyway
Copy link
Collaborator

flyway commented Jun 25, 2013

Original author: ke...@fidelma.com (April 24, 2013 05:01:42)

When upgrading a database I would expect all un-applied migrations to be applied within the same transaction. Currently each migration runs within its own transaction.

My use-case looks something like this:

  1. Version 1.0 is deployed into Production
  2. Developers work on features for Version 2.0, each creating a migration script matching their new feature.
  3. As development progresses, V2.x migrations, and the associated application, are put into QA.
  4. Eventually we decide we've done enough, and want to release v2, and it's myriad migration scripts.
  5. When we run the deployment against v1 in production we expect all the v2.x migrations to run, and if there is a problem with any of them, then the database is rolled back, and we revert the application to v1 while we figure out what went wrong.

We really don't want some of the v2 scripts to be committed to the production database.

We're using Postgres, so DDL rolls back nicely.

I'm happy to do the change, if nobody else is interested, but it seems like an "obvious" feature to me. None of your competition seem to offer it either. Am I missing something?

Original issue: http://code.google.com/p/flyway/issues/detail?id=484

@flyway
Copy link
Collaborator Author

flyway commented Jun 25, 2013

From axel.fontaine.business@gmail.com on April 24, 2013 14:45:02
Hi Kerry,

thanks for your suggestion. Interesting idea. I'll keep it in mind for future releases.

Cheers
Axel

@flyway
Copy link
Collaborator Author

flyway commented Jun 25, 2013

From raphael...@gmail.com on May 01, 2013 19:32:20
I would appreciate this feature too.

@flyway
Copy link
Collaborator Author

flyway commented Jun 25, 2013

From rbrico on May 02, 2013 16:53:21
I really need that! please!

@kerryland
Copy link

+1

@kerryland
Copy link

Is anybody actively working on this? If not, I'm happy to pick it up (but I don't want to duplicate anybody else's work!)

@axelfontaine
Copy link
Contributor

I don't believe anybody is right now.

@kerryland
Copy link

I finally got around to taking a look at implementing this. The logic seems simple enough, and although everything looked like it was working fine (logs indicating autocommit was turned off when commands were executed, and ROLLBACKs occurring at the right places), at the end of the day some data was actually still committed.

Here's the MigrationTestCase I wrote, that wouldn't pass, just in case somebody else feels like taking on the challenge:

    @Test
    public void failedMigrationSingleTransaction() throws Exception {
        flyway.setValidationMode(ValidationMode.NONE);
        flyway.setLocations("migration/future_failed");
        flyway.setSingleTransaction(true); // new property to turn one "single transaction mode"

        try {
            flyway.migrate();
            fail();
        } catch (FlywayException e) {
            //Expected
        }

        if (dbSupport.supportsDdlTransactions()) {
            assertFalse(dbSupport.getCurrentSchema().getTable("test_user").exists());
        } else {
            assertEquals(0, jdbcTemplate.queryForInt("select count(*) from test_user"));
        }
    }

@alexeev
Copy link

alexeev commented Mar 18, 2015

+1
I would appreciate this feature!

@alexey-anufriev
Copy link

+1

@orange-buffalo
Copy link

+1
We really missing this feature !

@atomixxx
Copy link

+1

@janeklb
Copy link

janeklb commented Nov 30, 2015

+1

@janeklb
Copy link

janeklb commented Nov 30, 2015

@kerryland do you still have your implementation kicking around for this?

@kerryland
Copy link

On 1 Dec 2015 04:51, "Janek Lasocki-Biczysko" notifications@github.com
wrote:

@kerryland https://github.com/kerryland do you still have your
implementation kicking around for this?

I've had a look. It's a hacky mess. In summary I've tried to move the
commits around:

TransactionTemplate.execute should not commit or rollback
simply :

T result = transactionCallback.doInTransaction();

Flyway.migrate: after dbMigrator.migrate() add
connectionMetaDataTable.commit()
connectionUserObjects.commit();

I also commented out the next line. Honestly not sure why:

dbSupportUserObjects.setCurrentSchema(schemaUserObjects);

Flyway.configure I disabled autocommits:

 connectionMetaDataTable.setAutoCommit(false);
 connectionUserObjects.setAutoCommit(false);

Hope this helps.

@janeklb
Copy link

janeklb commented Dec 16, 2015

Thanks @kerryland - much appreicated, I'll see if I can work from here..

@gionn
Copy link

gionn commented Apr 7, 2016

I am surprised that this feature is still missing, how do you rollback if a production deployment fails? Are you always writing backward-compatible migrations?

@axelfontaine if you can give us some guidance I can try to pick up the task

@fabioformosa
Copy link

+1
We got this problem, not only over multiple migrations, but over a single migration also (from V00 to V01) obtained by running multiple scripts with a progressive sequence number.
Any update?

@janeklb
Copy link

janeklb commented May 18, 2016

@fabioformosa ive tried to follow @kerryland 's advice but haven't made much progress, probably due to my poor understanding of JDBC. Happy to push what I have to a branch but I don't know if it would be of any help..

I expect that @axelfontaine won't be picking this up on his own because of Flyway's business model (funded feature requests), and rightly so. So if we want to see this happen, someone here is going to have to pick up the tab.

@hmvs
Copy link

hmvs commented Jul 21, 2016

Must have feature. Used to it in our own written dbchangemanager for sql server. Just integrated flyway instead of it (because of moving to pgsql) and found that it's not capable to do all migrations in one transaction :(

@vovcacik
Copy link

This would make my continuous deployment virtually atomic.

Eviradnus added a commit to Eviradnus/flyway that referenced this issue Jan 6, 2017
@Eviradnus Eviradnus mentioned this issue Jan 6, 2017
@highlowhighlow
Copy link

highlowhighlow commented Mar 16, 2017

Stumbled on this discussion today.

From DB perspective, not sure how this could happen. Currently each migration is wrapped in one explicit transaction of BEGIN/COMMIT(assuming nothing goes wrong).

To accomplish this, the explicit transaction wrapping for each individual migration needs to be taken out, instead, it needs to be done on the group of migration scripts - since I don't think you can have one explicit transaction within another one (in postgresql at least. Haven't tested in Oracle but suspect the same). I guess a flag can be added to signal if each migration script should be wrapped in one transaction. Savepoint might help too.

@vovcacik
Copy link

@patricia-finra I don't see problem with that. Flyway is already managing transactions for us. Right now it starts and commits transaction for each script individually and this feature request is basically matter of changing transaction scope from individual scripts to batch of scripts. No need to mess with savepoints.

@highlowhighlow
Copy link

@vovcacik Yes, but to do that they have to take out wrapping the transaction for each migration. Currently that seems the default. Now they have to say: ok now it is batch of scripts, so don't wrap each migration in transaction, only wrap it for the batch. That's why I said they might need a flag to know at which level to set the transaction.

@cowwoc
Copy link

cowwoc commented Mar 16, 2017

I agree with @vovcacik. This doesn't seem too hard to me. Ideally, we want all pending migrations applied in a single transaction so that if a failure occurs the database is left in a good state.

@highlowhighlow
Copy link

highlowhighlow commented Mar 16, 2017

@cowwoc Not sure I've made myself clear. Currently lets say I have 2 migrations, one with 2 updates: update tblA; update tblB; and a 2nd migration with just 1 update: update tblC;

When flyway gets it what it does is: let's call this case1
begin;
update tblA;
update tblB;
commit;
begin;
update tblC;
commit;

The transaction is added by flyway by default. lets say now those 2 migrations are for the same release, so we want to apply them in 1 transaction: lets call this case2
begin;
/*** do not issue BEGIN here /
update tblA;
/
*** do not COMMIT here either. ditto below */
update tblB;
update tblC;
commit;

To get to caes2 from case1, flyway needs to not wrap each migration in an explicit transaction, which is what they are doing by default now. That's all. Not hard, but need a flag or something to know not to wrap in a transaction for each migration.

@cowwoc
Copy link

cowwoc commented Mar 17, 2017

@patricia-finra I don't understand the difficulty here.

As you said, it is flyway who decides to open and close each transaction. Nothing prevents it from beginning a transaction, running all pending scripts, and committing the transaction at the end.

Flyway is in control of its own logic. Am I missing anything here?

@highlowhighlow
Copy link

@cowwoc It is all about transaction control. Postgresql and Oracle don't allow nested transactions, so you can't do:
. . .
begin transaction for the batch;
begin transaction for 1 migration; <--this will be ignored. currently flyway adds this by default
do something here; <--whatever is in the migration script
commit; <--it will commit statements so far, which is not what you wanted.
. . .

Not to mention different types of databases may have different transaction control. If I understand it right, flyway is supposed to be database agnostic, but seems to me it is counting on database transaction control mechanism.

@cowwoc
Copy link

cowwoc commented Mar 17, 2017

@patricia-finra We don't need nested transactions. Flyway controls when it starts each transaction so it can run all migrations under a single top-level transaction.

@janeklb
Copy link

janeklb commented Apr 12, 2017

thanks @axelfontaine !

@kerryland
Copy link

Excellent news. Thanks very much @axelfontaine

@cowwoc
Copy link

cowwoc commented Apr 13, 2017

For what it's worth: http://stackoverflow.com/a/4736346/14731

It seems the vast majority of databases support transactional DDL operations. Hopefully MySQL will add support in the future.

@bseib
Copy link

bseib commented Apr 24, 2017

In the 4.2 code, if a migration fails, is a rollback still attempted, regardless of the type of database you are connected to? Or does it not even attempt a rollback if supportsDdlTransactions() returns false?

If non-DDL code fails to migrate, it would be nice if it still rolled back. That might require migration filenames to indicate if they contained DDL commands or not.

@cowwoc
Copy link

cowwoc commented Apr 25, 2017

@bseib

If non-DDL code fails to migrate, it would be nice if it still rolled back.

Why? Especially given that this feature request has a cost associated with it:

That might require migration filenames to indicate if they contained DDL commands or not.

What is gained by rolling back in these cases?

Or alternatively, what is lost by assuming that migration files always contain DDL commands and always rolling back on failure regardless of the database type and the contents of the files?

@kerryland
Copy link

kerryland commented Apr 25, 2017 via email

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