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

Collapse database migrations #3524

Merged
merged 17 commits into from
May 31, 2019
Merged

Conversation

ocket8888
Copy link
Contributor

@ocket8888 ocket8888 commented Apr 25, 2019

What does this PR (Pull Request) do?

Collapses all previous migrations into seeds.sql and/or create_tables.sql (as appropriate). It does leave all migrations written since 3.0.x was released.

  • This PR is not related to any Issue

Which Traffic Control components are affected by this PR?

  • Documentation
  • Traffic Ops (DB)

What is the best way to verify this PR?

Building CDN-in-a-Box seems appropriate. That's what I did. Also, try dumping an older schema and comparing it with this. The only (real) difference should be the new latitude/longitude constraints.

If this is a bug fix, what versions of Traffic Ops are affected?

This isn't a bug fix, but on the topic of TO versions: this is a breaking change to the database, meaning rollbacks to migrations that previously existed will no longer be possible.

The following criteria are ALL met by this PR

  • This PR includes tests OR I have explained why tests are unnecessary
  • This PR includes documentation OR I have explained why documentation is unnecessary
  • This PR includes an update to CHANGELOG.md OR such an update is not necessary
  • This PR includes any and all required license headers
  • This PR ensures that database migration sequence is correct OR this PR does not include a database migration
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@asfgit
Copy link
Contributor

asfgit commented Apr 25, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3586/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Apr 25, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3587/
Test PASSed.

@mitchell852 mitchell852 added Traffic Ops related to Traffic Ops improvement The functionality exists but it could be improved in some way. labels Apr 25, 2019
@limited
Copy link
Contributor

limited commented Apr 26, 2019

What happens if someone is not on the latest minor version before migrating to this version?
I think it is an unreasonable expectation that a user install every version we have released in order as part of an upgrade.

@ocket8888
Copy link
Contributor Author

You should only have to run an upgrade once to get from any arbitrary minor version to the latest. Unless you mean trying to allow people to go from e.g. v1.x -> v4.x skipping everything in between? That's never been possible afaik.

@asfgit
Copy link
Contributor

asfgit commented Apr 29, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3595/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Apr 29, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3601/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented Apr 29, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3603/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented May 8, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3637/
Test PASSed.

@dangogh
Copy link
Member

dangogh commented May 8, 2019

just resolved the CHANGELOG.md conflict straight from GH

Copy link
Member

@dangogh dangogh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are a number of things that changed in addition to collapsing the migrations. This PR should be limited to only creating the db as if it had gone through the migrations and nothing more.

Any other fixes to tables or data should be addressed in a separate PR. The extra constraint migration should be alone in a separate PR.

@ocket8888
Copy link
Contributor Author

Traffic Ops will fail to install without any migrations - any change that collapses migrations must also do one of:

  • Replace Goose with something that won't crash if it has nothing to do
  • Change admin.go to silently check for migrations before running them, and not attempt to do so if none are found
  • Modify postinstall to continue even if admin crashes (bad idea IMO)
  • Add at least one database migration.

@ocket8888
Copy link
Contributor Author

ocket8888 commented May 9, 2019

Instead of introducing a new migration, this now merely leaves in the migrations introduced since the release of 3.0.x as the only remaining migrations after collapse.

@asfgit
Copy link
Contributor

asfgit commented May 9, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3642/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented May 9, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3643/
Test FAILed.

@asfgit
Copy link
Contributor

asfgit commented May 9, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3644/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented May 20, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3693/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented May 20, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3695/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented May 21, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3698/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented May 21, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3699/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented May 28, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3736/
Test PASSed.

Copy link
Member

@dangogh dangogh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Realized that I hadn't run the patch step on previous one, so some diffs I noted before are no longer there.

Couple of things still different that should be fixed.. Still want to run some more data thru it..

traffic_ops/app/db/create_tables.sql Outdated Show resolved Hide resolved
traffic_ops/app/db/create_tables.sql Outdated Show resolved Hide resolved
@dangogh dangogh self-requested a review May 31, 2019 14:33
Copy link
Member

@dangogh dangogh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

still have the last_edited_by error and conflict in CHANGELOG.md that need to be addressed.

@asfgit
Copy link
Contributor

asfgit commented May 31, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3769/
Test PASSed.

@asfgit
Copy link
Contributor

asfgit commented May 31, 2019

Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/trafficcontrol-PR/3774/
Test PASSed.

@dangogh dangogh merged commit 41cc65a into apache:master May 31, 2019
@ocket8888 ocket8888 deleted the the-great-migration branch June 1, 2019 02:35
@dangogh dangogh mentioned this pull request Jun 3, 2019
7 tasks
@mitchell852 mitchell852 added the database relating to setup/installation/structure of the Traffic Ops database label Jun 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
database relating to setup/installation/structure of the Traffic Ops database improvement The functionality exists but it could be improved in some way. Traffic Ops related to Traffic Ops
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants