sky-uk / cqlmigrate

Cassandra schema migration library
BSD 3-Clause "New" or "Revised" License
47 stars 29 forks source link

issue:35 make cluster health check optional #36

Closed lukaszgielecinski closed 7 years ago

lukaszgielecinski commented 7 years ago

At the moment cqlmigrate requires that all nodes in all datacentres are up before it will run. We would like to make this check optional so that it will still run if a node is down so that the migration will be attempted with the supplied consistency level - which we will plan to set to LOCAL_QUORUM for read and EACH_QUORUM for write for our migrations.

chbatey commented 7 years ago

Consistency isn't relevant for schema changes.

I am hesitant to allow schema changes when nodes are down. It really isn't a good idea and it'll allow people to shoot them selves in the foot.

lukaszgielecinski commented 7 years ago

Having reviewed what we've done we now understand the problem a bit better and agree that we need a new approach. Part of the problem we're seeing is that we are currenly always running cqlmigrate at startup so that if there are new changes they are applied as part of our startup logic, rather than specifically running it from the command line. For most of our startups we don't expect any changes at all so its causing use problems when nodes are down and we need to restart our processes.

We see that consistency level makes no difference to how DDL statements are applied although for our use case we also potentially apply data updates.

The change we propose is to only require that all nodes are up if cqlmigrate needs to make any changes at all i.e. creating the keyspace, creating the schema migration table, updating schema_migration table and any application specific migrations that have not been applied (determined by querying schema_migrations table)

At the point we determine that a change is needed, the ClusterHealth.check() would be invoked and if this fails the cqlmigrate would abort.

chbatey commented 7 years ago

Yeah that makes a lot of sense otherwise we'll need all nodes up to restart an app if it uses cqlmigrate on start up.

lukaszgielecinski commented 7 years ago

New approach introduced here: https://github.com/sky-uk/cqlmigrate/pull/37