search

yogthos / migratus

MIGRATE ALL THE THINGS!
647 stars 95 forks source link

tx rollback doesn't unreserve the migrations table #132

Open leblowl opened 6 years ago

leblowl commented 6 years ago

Testing locally with an H2 db, I found that if one of my migrations failed, then migratus would not unreserve the schema migrations table. This led to a weird state for me, because when I tried to re-run the migration, it told me something like "table reserved", but when I tried to rollback it pretty much left me without any constructive message - it looked like it was rolling back, but it really didn't do anything. I think the messaging can be brought up in a separate issue, but I just wanted to mention it as it relates to finding this bug. So I think the issue is with tx vs no-tx. When the migration has tx enabled, the entire migration code is wrapped in a transaction, including the mark-unreserved call:

https://github.com/yogthos/migratus/blob/19c0fa9740dcf23335bd1169f594cc7fb477561d/src/migratus/database.clj#L230-L234 https://github.com/yogthos/migratus/blob/19c0fa9740dcf23335bd1169f594cc7fb477561d/src/migratus/database.clj#L54-L72

I added some tests that should hilight this behavior. Thought, I'm not really sure how to best structure tests like this that share the same structure but slightly different data...maybe you have a suggestion.

leblowl commented 6 years ago

Also not sure how best to share code like this, open to suggestions. Thanks

yogthos commented 6 years ago

I think the issue here is that h2 has quirky support for transactions. So, not sure how much can be done to address that within migratus.

leblowl commented 6 years ago

ok, makes sense. H2 is the only DB I have tested. I can double check it with another store like PostgreSQL

nakiya commented 6 years ago

Same happens on MySql too

eoinmcq commented 6 years ago

[Edit: having thought about this I am not convinced it is the same problem as the reserved id would never have successfully been written due to an error in the transaction and it being rolled back - I will leave the comment here in case people have read it. Thanks!)

Same happens on Postgres.

The unreserve is being done within the same transaction block which failed so will always fail with a transaction aborted. (In the case of migrate-up* all being done in one transaction block obviously.)

The manifestation of the problem I have/saw was the the SQL error code was coming back as 25P02 (transaction aborted) because the exception in the finally block was superseding the exception in the catch block so no matter what the original error was it would always get overwritten.

In my understanding of this (I am very new to Clojure...) the unreserve can never work in failed cases because the transaction block it is trying to run in will always be in a failed state before the unreserve is ran.