Fix caching large snippet HTML content on MySQL databases

What does this MR do?

Increases the size limit of the snippets.content_html column on MySQL databases

Are there points in the code the reviewer needs to double check?

Other _html columns? This is the only column with an uncached counterpart that has had its MySQL limit increased, so I think this is a complete fix.

This migration will run O(N) with the number of snippet rows. MySQL users are getting a raw deal here, but it doesn't seem that there's a good option: https://serverfault.com/questions/6771/altering-a-column-length-on-a-large-innodb-table-with-minimal-downtime

Why was this MR needed?

This bug breaks caching large snippets on MySQL systems.

Screenshots (if relevant)

Does this MR meet the acceptance criteria?

• Changelog entry added, if necessary
• Tests
  • All builds are passing
• Conform by the merge request performance guides
• Conform by the style guides
• Branch has no merge conflicts with master (if it does - rebase it please)
• Squashed related commits together

What are the relevant issue numbers?

Closes #31647 (closed)

marked the checklist item Changelog entry added, if necessary as completed

added 1 commit

• c2162fd3 - Fix caching large snippet HTML content on MySQL databases

Compare with previous version

assigned to @yorickpeterse

@yorickpeterse WDYT? This is modelled on existing "add MySQL limit" migrations, e.g., https://gitlab.com/gitlab-org/gitlab-ce/blob/master/db/migrate/20151020173516_ci_limits_to_mysql.rb , but I wonder if there's a Better Way.

added 1 commit

• 8dd7f725 - Fix caching large snippet HTML content on MySQL databases

Compare with previous version

added database label

changed milestone to %9.2

added 1 commit

• bec0b779 - Fix caching large snippet HTML content on MySQL databases

Compare with previous version

@nick.thomas Since the limit is not included in db/schema.rb, what do we need to do to ensure this migration is applied to new databases? It seems that in the past we have used db/migrate/limits_to_mysql.rb for this.

added 1 commit

• 703dd891 - Fix caching large snippet HTML content on MySQL databases

Compare with previous version

@yorickpeterse @nick.thomas Yep, we should add this one to db/migrate/limits_to_mysql.rb.

added 1 commit

• e6127a44 - Fix caching large snippet HTML content on MySQL databases

Compare with previous version

resolved all discussions

@DouweM @yorickpeterse the CI column limits were never added. Is that a bug we need to address too?

    change_column :ci_builds, :trace, :text, limit: 1073741823
    change_column :ci_commits, :push_data, :text, limit: 16777215

This hack probably needs documenting somewhere.

added 1 commit

• 85488221 - Fix caching large snippet HTML content on MySQL databases

Compare with previous version

@nick.thomas Yes, I think that needs to be addressed as well.

OK, blindly adding it (and the ci_limits too, I presume) to limits_to_mysql doesn't work, as it then gets executed as part of the 2015 migration of the same name.

The add mysql limits rake task can be modified to run more than one migration; I'll copy the two-files-per-migration hack of the original and ci and markdown cache limits to that rake task.

added 2 commits

• da470cb9 - Fix caching large snippet HTML content on MySQL databases
• c53f62d3 - Ensure CI limits are applied to new databases

Compare with previous version

added 131 commits

• c53f62d3...6201f4c2 - 129 commits from branch gitlab-org:master
• 645089c6 - Fix caching large snippet HTML content on MySQL databases
• 3f0df4ec - Ensure CI limits are applied to new databases

Compare with previous version

added 1 commit

• d1dfaad5 - Ensure CI limits are applied to new databases

Compare with previous version

added 1 commit

• 853e0da1 - Ensure CI limits are applied to new databases

Compare with previous version

resolved all discussions

added 1 commit

• eb6c088d - Fix caching large snippet HTML content on MySQL databases

Compare with previous version

CI limits are causing problems, e.g. https://gitlab.com/nick.thomas/gitlab-ce/builds/15506816 - I've taken it out for now. I guess the ci_commits table has been renamed (to ci_pipelines?) but it's not entirely obvious what to do about it..

Yes it was renamed to ci_pipelines. Why was it still using the old name?

It's a migration, so it gets run in-order when an older schema comes up, as well as at the very end due to the rake task.

I'm starting to think the code shouldn't be shared between the two paths, but I want us to at least fix the current problems in master with in this MR. We can consider fixing this other long-standing issue separately.

added 1 commit

• 1a168dc7 - Fix caching large snippet HTML content on MySQL databases

Compare with previous version

Having files not in the <version>_<name>.rb form in db/migrate breaks rake downtime_check: https://gitlab.com/nick.thomas/gitlab-ce/builds/15510236

Just pushed a fix

@rymai this is ready for merge, I think.

There are outstanding issue with the whole mysql limits framework, but we can handle those in a separate MR.

assigned to @rymai

resolved all discussions

approved this merge request

merged

mentioned in commit 8983ade2