workflow_index.update_time NOT NULL constraint breaking upgrade to 3.21.5 #1
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Pull Request type
NOTE: Please remember to run
./gradlew spotlessApply
to fix any format violations.Changes in this PR
Address items raised in issue 237 conductor-oss#237, these issues were:
To fix these issues this PR make the following changes:
Replaces the v13 flyway migration script with V13.1 that adds update_time with a default of 'epoch' if the column doesn't exist, and sets the default of 'epoch' if the column does exist. Adding a default in postgres is inexpensive, as it does not update existing rows. The default means the constraint can be added without running the back-fill and stops the issues when have old and new versions running at the same time.
Fix the PostgresIndexDao.indexWorkflow to also set the update_time during an UPDATE. This will ensure updateTime accurately reflects the most recent write.
Address the issues with the back-fill script logic, to ensure it doesn't try to insert NULL values and correctly formats the timestamp.
Make the back-fill script for update_time optional, so that expensive data migrations do not need to be run at start up.
Alternatives considered
I considered removing the not null constraint. However, this meant more complications dealing with null values in the consuming code. So given a default is inexpensive to add this seemed preferable.
I also considered dropping the back-fill script entirely, as it doesn't appear to impact any logic. However, I decided to add the ability to disable data migrations, for expensive scripts. Flyway community edition does not support https://flywaydb.org/blog/skipExecutingMigrations so it is impossible to defer scripts from being run during the migration if using that edition.