patka / cassandra-migration

Schema migration library for Cassandra
MIT License
152 stars 47 forks source link

Use execution config for table creation #62

Closed ikstewa closed 2 years ago

ikstewa commented 2 years ago

Use the execution profile name and consistency level configured when creating tables. This is necessary as some instances will fail with a timeout on the table creations. Adding the execution profile allows the table creation execution to be configurable.

patka commented 2 years ago

Hi Ian,

thanks for the pull request and that you implemented this. I just committed a MigrationConfiguration class because I don't think it makes sense to continue to introduce new constructors for every new setting that is introduced. Could you adapt your PR to this? I can also do the implementation but I would like you to have the credit for the implementation as you submitted it. Or is there a way I can adapt this PR before merging?

Have a nice day Patrick

ikstewa commented 2 years ago

Will do.

There were a few other locations that weren't using the profile or consistency level. I wasn't certain if we should move all these cases over or just the table creation. @patka Which of these do you think should also change?

patka commented 2 years ago

Hi Ian,

That is a good question. Let me have a closer look at these methods and see which ones should be adapted. I tend to say all of them but I would like to double check if there is a good reason for any of them not to be executed with a certain profile. Once you have adapted the PR and can merge it and do the final changes to these methods then. I think we anyway need to introduce new private execution methods.

Thanks, Patrick

ikstewa commented 2 years ago

@patka Thanks Patrick!

Looks like you already got it updated to use the profile and consistency level for the table creation. I can create another PR for the other queries. I'm not certain if we want to allow the user to set the consistency for the version or leader election queries as I feel those should always be quorum.

Would leader election work if it had a consistency level other than quorum?

ikstewa commented 2 years ago

The execution profile name wasn't being set in the constructor from the configuration. Updated to do so and ensure all statements use the profile/consistency level: https://github.com/patka/cassandra-migration/pull/63

patka commented 2 years ago

@ikstewa Thanks a lot Ian, for the second PR!

Personally I would not execute any of the statements without Quorum consistency as you can easily get into a situation where you execute two updates that depend on each other and that could then not see themselves. At least for leader election, I think it makes sense to keep the Quorum. In general the way I handle it is, as long as nobody asks, I don't want to provide options to get yourself into a mess. And also because it keeps the API simpler and easier to maintain.