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

Auto-migrate considered harmful #164

Open
davidread opened this issue Dec 9, 2015 · 4 comments
Open

Auto-migrate considered harmful #164

davidread opened this issue Dec 9, 2015 · 4 comments

Comments

@davidread
Copy link

CKAN extensions are often written to create and alter their database tables automatically on run-time. The idea is that when you install or upgrade an extension that uses tables, then you don't have to remember to run a paster command to do the migration - it just happens automatically when you run any paster command or restart the server.

This is seen in:

etc

However data.gov.uk has frequently run into problems with this approach installing them on a live server. When the extension is installed and you restart apache, then the multiple CKAN processes will all try to get a write lock on postgres and then proceed to all create or alter the tables. You either get dead-lock or lots of errors. I've ended up with minutes of downtime, aborted upgrades and fixing table migrations manually while the server remains down. It's particularly bad for ckanext-spatial as it had migration during some upgrades of the code. The only way to upgrade it safely is to stop apache, upgrade code, run an unrelated paster command that does the upgrade and then start apache again.

I think it would be better if migrations were done using a specific paster command, as I have done in ckanext-archiver (paster --plugin=ckanext-archiver archiver init). It means there are 5 steps to the install instead of 4. It is easier to understand than running an unrelated paster command. And you don't have down-time - unplanned or planned.

https://github.com/datagovuk/ckanext-archiver

Can we agree that this is a better way to do it?

@rossjones
Copy link
Contributor

Would this not be better achieved by providing an interface that allows a plugin to specify where it's migrations are stored. That way we could change db upgrade to run those migrations?

@davidread
Copy link
Author

@wardi @amercader Ross said that you weren't keen on changing db migrations from auto to manual, preferring all extensions to be upgraded centrally. Whilst I agree that would be best, can't we at least outlaw these automatic migrations that cause so much grief? Surely manual is better than that? And if someone gets time to do a central solution then bonus.

@wardi
Copy link
Contributor

wardi commented Dec 10, 2015

@davidread 👍

@rossjones
Copy link
Contributor

I've started a PR at ckan/ckan#2785 for migrations in extensions which is waiting on docs, but also need to figure out how to test (it was tested manually so far). Although I've made it so that new extensions automatically get migrations, it might be useful to also document how to add to existing extensions.

I guess this would be a 2.6 thing, so I can make a start on fixing up some of the extensions that do this in the meantime.

torfsen added a commit to stadt-karlsruhe/ckanext-extractor that referenced this issue Jun 14, 2016
Automatic database initialization during CKAN startup can lead to
dead-locks, see ckan/ideas#164.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants