Add migration to update the primary key and unique constraints on the kudos table

Description

This is the one prophesied to take down the site. Migration:

  • Change the primary key type from INT to BIGINT.

  • Add unique index for commentable_id + commentable_type + user_id. This will remove duplicate kudos for the same work from the same user.

  • Add unique index for commentable_id + commentable_type + ip_address. This will remove duplicate kudos for the same work from the same guest IP.

The migration will be different depending on the environment:

  • Add a usual Rails migration, to be used in dev and CI environments.

  • Add a comment with the pt-online-schema-change command to be run on staging and beta (similar to ).

How to test:

  • (Anyone) Test first.

  • (Server admin) Test the migration:

    1. Run the migration.

    2. Copy the commands that it prints to a file.

    3. Run the pt-online-schema-change command, but not any of the SQL commands that follow it.

    4. Create a brand new kudos, and note the ID in the database.

    5. Create another kudos on the same work, from the same user (by using the Rails console and skipping validation, .save(validate: false)). This is a duplicate of the kudos from the previous step.

    6. Run the SQL commands. (RENAME TABLE, DROP TRIGGER, and DROP TABLE)

    7. Check whether the kudos that you created is still there. Check that it has no duplicates (the duplicates should have been eaten by the pt-online-schema-change migration).

    8. From the Rails console: try to insert a duplicate kudos, bypassing validations, and make sure that it generates an error.

  • (Server admin) Test that the migration can run up and down and up again.

  • (Anyone) Finally, in the post-migration world:

    • Check that leaving logged in kudos still works.

    • Check that leaving logged in kudos a second time on the same work will get you the "left kudos here " message.

    • Check that you can no longer leave logged in kudos after changing default pseuds.

Activity

Show:
james_
March 7, 2020, 12:37 PM

 

 

Looks good to me

redsummernight
March 7, 2020, 6:37 PM
Edited

Post-migration:

  • With JS enabled:

    • I can leave logged in kudos ("Thank you for leaving kudos!")

    • I cannot leave logged in kudos as the same user on the same work again ("You have already left kudos here. :)"), even if I change default pseuds (same error message).

    • I can leave guest kudos only once from the same IP address. I can leave another guest kudos (but still only once) from a second IP address.

  • With JS disabled:

    • I can leave logged in kudos ("Thank you for leaving kudos!")

    • I cannot leave logged in kudos as the same user on the same work again ("User ^You have already left kudos here. :)"), even if I change default pseuds (same error message, which looks weird because of ).

Thanks Capi for testing guest kudos.

Looks good.

AO3 Paula
March 12, 2020, 2:26 AM

Finally, in the post-migration world:

  • Check that leaving logged in kudos still works.

  • Check that leaving logged in kudos a second time on the same work will get you the "left kudos here " message.

  • Check that you can no longer leave logged in kudos after changing default pseuds.

    Checked them, we’re good to go! \o/

 

redsummernight
March 12, 2020, 2:32 AM

Just for fun, I also tested orphaning kudos:

  • Created a new account.

  • https://test.archiveofourown.org/works/434904 had 9142 kudos (9133 from guests). Left a kudos. The work displayed a count of 9143.

  • Deleted the account.

  • Cache-busted the work from the Rails console.

  • The deleted user no longer showed up in the kudos section. The work then had a kudos count of 9143, but the guest count is still 9133. So my kudos was still around, just not displayed () and not counted as from a guest.

Still good.

redsummernight
March 20, 2020, 4:29 AM
Edited

Deploy notes:

  • The migration failed very early on and we had to tweak the pt-online-schema-change parameters: https://github.com/otwcode/otwarchive/pull/3768.

  • The migration is divided into 3 major steps: copying rows, renaming tables, dropping triggers syncing old and new tables, dropping unused old table.

  • Migration take 1 (Mar 16): copying rows finished, but we accidentally dropped the triggers before renaming tables, so the tables were no longer in sync for a swap. An unused "new" table was left over.

  • Migration take 2 (Mar 16-17): finished copying rows, swapping tables, dropping triggers. We successfully dropped the unused "new" table from take 1. However, dropping the unused old table from take 2 caused downtime. We had to restart the database cluster.

:

Makes sense that dropping the unused table would go smoothly – it never entered the buffer pool, so getting it out of the buffer pool is probably pretty quick and clean. But that does make me think that just waiting a few weeks before deletion might be sufficient to delete the second table smoothly.

Ongoing discussion: we want to wait for some time before dropping the remaining table. If the table is no longer in the buffer pool and the adaptive hash index, dropping it will be a lightweight operation (no need for the deletion to be replicated). TBD (SHOW ENGINE INNODB STATUS) if the table is still cached.

Done

Assignee

ticking instant

Reporter

redsummernight

Roadmap

Misc

Priority

Medium

Affects versions

Fix versions

Components

BackEnd

Difficulty

Medium

Milestone

Internal 0.9
Configure