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

Poll migration is not idempotent #7964

Merged
merged 1 commit into from Aug 8, 2019

Conversation

@eduardopoleo
Copy link
Contributor

commented Aug 2, 2019

The poll migration script (for 2.3.0) is not idempotent due to database constrains on the
poll related objects, namely:

polls: index_polls_on_post_id_and_name (post_id,name) UNIQUE
poll_options: index_poll_options_on_poll_id_and_digest  (poll_id,digest) UNIQUE
poll_votes:  index_poll_votes_on_poll_id_and_poll_option_id_and_user_id  (poll_id,poll_option_id,user_id) UNIQUE

This change skips a particular poll migration if it's already found on
the db. We also wrap the poll migration in a transaction so that if the
script happens to fail mid-way while migrating a poll we rollback the
poll and all it's related records, thus preventing undesirable partial
states.

@discoursebot

This comment has been minimized.

Copy link

commented Aug 2, 2019

Thanks for contributing this pull request! Could you please sign our CLA so we can review it? http://www.discourse.org/cla

@SamSaffron

This comment has been minimized.

Copy link
Member

commented Aug 5, 2019

This is very confusing to me... all migrations run in a transaction so we should not need to layer more transactions here.

Eduardo Poleo
Poll migration is not idempotent
The migration script is not idempotent due to database constrains on the
poll related objects, namely:

polls: index_polls_on_post_id_and_name (post_id,name) UNIQUE
poll_options: index_poll_options_on_poll_id_and_digest  (poll_id,digest) UNIQUE
poll_votes:  index_poll_votes_on_poll_id_and_poll_option_id_and_user_id  (poll_id,poll_option_id,user_id) UNIQUE

This change skips a particular poll migration if it's already found on
the db.

@eduardopoleo eduardopoleo force-pushed the eduardopoleo:idempotent_poll_migration branch from f4694d8 to fd5b269 Aug 6, 2019

@eduardopoleo

This comment has been minimized.

Copy link
Contributor Author

commented Aug 6, 2019

Just updated. Thanks for looking into this.

@ZogStriP ZogStriP merged commit b500ef7 into discourse:master Aug 8, 2019

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details
@ZogStriP

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

Thanks 👍

@discoursebot

This comment has been minimized.

Copy link

commented Sep 11, 2019

This pull request has been mentioned on Discourse Meta. There might be relevant details there:

https://meta.discourse.org/t/zero-downtime-upgrades/120673/2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.