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.

Sign up for GitHub

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

Jump to bottom

[Django Upgrade] [ENG-3998] Migration Clean squash + Update RunSQL Indexes #10028

Merged
merged 7 commits into from
Aug 23, 2022
Merged

[Django Upgrade] [ENG-3998] Migration Clean squash + Update RunSQL Indexes #10028

merged 7 commits into from
Aug 23, 2022

Conversation

cslzchen
Copy link
Collaborator

@cslzchen cslzchen commented Aug 17, 2022 •
edited
Loading

Purpose

Squash all migrations before upgrade and add not-auto-genarated postgres indexes.

Deployment Notes

Fortunately, on servers, we don't need to go through the steps that creates this PR. All we need to do is:

  1. If the target server has auto-migration turned on, disable it before merge/deploy this PR
  2. Run migrate --fake to take care of everything

Changes

Here is the step-by-step work flow on how the migrations in this PR are generated.

  1. Check out develop, make sure migrations are up-to-date.
  2. (Optional) Create a bunch of data while running OSF locally. Make sure you have all services running.
  3. Check out feature/django_upgrade
  4. Delete osf migrations
  5. Delete every addon_* migrations
  6. Run showmigrations and verify the deletion
  7. Run makemigrations: migrations will be created for osf and every addon_*
  8. Run showmigrations again to trigger the circular import error between osfand built-in admin
  9. Fix the error by removing the dependency and the migration
  10. Run showmigrations again to verify the fix
  11. Run migrate with --fake on osf to trigger the NotableEmailDomain bug
  12. Fix the error by setting default=0 in the migration
  13. Run migrate with --fake on osf again to fake the init migration
  14. Run migrate with --fake on each addon_* to fake all each add-on's migrations
  15. Run makemigrations to generate a new migration for the removed migration in Step 9
  16. Run migrate with --fake on osf to fake the second OSF migration
  17. Add a third OSF migration that manually updates not-auto-generated indexes
  18. Run migrate with --fake on osf to fake the third OSF migration
  19. (Optional) Play with you OSF locally to confirm everything still works.

About the indexes update

Our initial approach is to rewrite all RunSQL indexes update documented in Migrations Creating Indexes/Constraints but kept run into errors due to out-dated models. Thus, we switched the approach to generate a index diff that we can easily compare and pick the indexes to add.

QA Notes

N/A

Documentation

N/A

Side Effects

N/A

Ticket

https://openscience.atlassian.net/browse/ENG-3998

@cslzchen cslzchen force-pushed the feature/clean_squash branch from fa82935 to c4322af Compare August 17, 2022 20:05
@cslzchen cslzchen changed the title [Django Upgrade] [Reference PR] Clean squash: remove all migrations and make a new 0001 [Django Upgrade] [ENG-TBD] Migration Clean squash + Update RunSQL Indexes Aug 18, 2022
@cslzchen cslzchen force-pushed the feature/clean_squash branch from 376d48a to fcff72a Compare August 18, 2022 18:57
@cslzchen cslzchen changed the title [Django Upgrade] [ENG-TBD] Migration Clean squash + Update RunSQL Indexes [Django Upgrade] [ENG-3998] Migration Clean squash + Update RunSQL Indexes Aug 18, 2022
@cslzchen cslzchen marked this pull request as ready for review August 18, 2022 19:00
@cslzchen cslzchen force-pushed the feature/clean_squash branch 2 times, most recently from 51ae779 to ac1e404 Compare August 22, 2022 18:51
@cslzchen cslzchen force-pushed the feature/clean_squash branch from ac1e404 to 699fc1c Compare August 22, 2022 18:53
@cslzchen cslzchen requested a review from mfraezz August 22, 2022 19:23
cslzchen
cslzchen commented Aug 23, 2022
View reviewed changes
osf/migrations/0003_aggregated_runsql_calls.py
Comment on lines +16 to +27
CREATE UNIQUE INDEX one_quickfiles_per_user ON public.osf_abstractnode USING btree (creator_id, type, is_deleted) WHERE (((type)::text = 'osf.quickfilesnode'::text) AND (is_deleted = false));
CREATE INDEX osf_abstractnode_collection_pub_del_type_index ON public.osf_abstractnode USING btree (is_public, is_deleted, type) WHERE ((is_public = true) AND (is_deleted = false) AND ((type)::text = 'osf.collection'::text));
CREATE INDEX osf_abstractnode_date_modified_ef1e2ad8 ON public.osf_abstractnode USING btree (last_logged);
CREATE INDEX osf_abstractnode_node_pub_del_type_index ON public.osf_abstractnode USING btree (is_public, is_deleted, type) WHERE ((is_public = true) AND (is_deleted = false) AND ((type)::text = 'osf.node'::text));
CREATE INDEX osf_abstractnode_registered_date_index ON public.osf_abstractnode USING btree (registered_date DESC);
CREATE INDEX osf_abstractnode_registration_pub_del_type_index ON public.osf_abstractnode USING btree (is_public, is_deleted, type) WHERE ((is_public = true) AND (is_deleted = false) AND ((type)::text = 'osf.registration'::text));
CREATE INDEX fileversion_date_created_desc ON public.osf_fileversion USING btree (created DESC);
CREATE INDEX fileversion_metadata_sha_arch_vault_index ON public.osf_fileversion USING btree (((metadata -> 'sha256'::text)), ((metadata -> 'archive'::text)), ((metadata -> 'vault'::text)));
CREATE INDEX nodelog__node_id_date_desc ON public.osf_nodelog USING btree (node_id, date DESC);
CREATE INDEX osf_nodelog_should_hide_nid ON public.osf_nodelog USING btree (should_hide, node_id);
CREATE UNIQUE INDEX osf_noderequest_target_creator_non_accepted ON public.osf_noderequest USING btree (target_id, creator_id) WHERE ((machine_state)::text <> 'accepted'::text);
CREATE INDEX lowercase_tag_index ON public.osf_tag USING btree (lower((name)::text), system);
Copy link
Collaborator Author

@cslzchen cslzchen Aug 23, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

More on indexes update

Indexes Comparision

Comments

First, we've verified the extra index between local-develop and prod is an existing "bug". This is trivial and can be neglected for now.

Institution inherits Loggable, but that 0069 migration didn't actually run, any my script didn't manually create the index here

osf_institution
osf_institution_last_logged_9ac963e2
CREATE INDEX osf_institution_last_logged_9ac963e2 ON public.osf_institution USING btree (last_logged)

Second, the one extra index is not introduced by the squash but by one of our post-migrate signal PRs. This probably doesn't matter though we no longer use expires. There maybe some performance hit due to indexing this one but not a blocker for staging3 merge since its already there.

osf_cache_table
osf_cache_table_expires
CREATE INDEX osf_cache_table_expires ON public.osf_cache_table USING btree (expires)

Finally, there is no difference between before-squash and after-squash (with indexes added in this PR).

Reference

@mfraezz mfraezz merged commit 0dc9220 into CenterForOpenScience:feature/django_upgrade Aug 23, 2022
@cslzchen cslzchen deleted the feature/clean_squash branch October 9, 2024 16:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mfraezz mfraezz Awaiting requested review from mfraezz

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@cslzchen @mfraezz