Fix remaining cross-joins in old migrations that have already run
What does this MR do and why?
TL;DR this MR is just deleting code that is in old migrations that we don't need anymore and is causing problems with some tests
Cross-joins are described at https://docs.gitlab.com/ee/development/database/multiple_databases.html#removing-cross-database-transactions . Basically these are single queries that span the Main and the CI database (now that we have 2 separate databases). They are blocked by a middleware we run in our tests which had to have a special allowlist for old migrations that we won't actually need to run ever again. This MR removes the code and the allowlist entries.
As part of #340017 (closed) we want to empty out spec/support/database/cross-join-allowlist.yml
and remove all remaining uses of allow_cross_joins_across_databases
. The only things leftover were old migrations so this MR handles all of those.
Details about the migrations and why we're allowed to remove them
All of these migrations were added or last referenced across the following MRs which were all 14.x
and therefore it is safe to now remove them. Per https://docs.gitlab.com/ee/update/#upgrading-to-a-new-major-version it is enforced that all clients upgrade to the latest minor release before upgrading major releases (in this case 15.x
) and they must also ensure all background migrations have completed before they upgrade which means that making these a no-op now in 15.x
is fine.
We also have some documentation for making migrations a no-op in https://docs.gitlab.com/ee/development/database/deleting_migrations.html which is OK so long as there are no schema changes.
The one migration in this MR that has schema changes is db/post_migrate/20210907211557_finalize_ci_builds_bigint_conversion.rb
. In this case since it also has already been run I just removed the offending code that created cross-joins which was only the LOCK
statements. These lock statements were only used as a performance improvement to reduce the likelihood of lock timeouts. Since, again, we can expect this will have already before this point we don't need to worry about this peformance optimization.
The MRs that added these migrations originally are:
-
!65201 (merged) => added
20210907211557_finalize_ci_builds_bigint_conversion
-
!71944 (merged) => added
20211007155221_schedule_populate_status_column_of_security_scans.rb
-
!79344 (merged) => finalized
BackfillCiNamespaceMirrors
andBackfillCiProjectMirrors
I also decided to just remove any specs for the migrations (even the no-op ones) as there is no value in testing a no-op migration. Technically the migrations still get executed in CI anyway so it doesn't really matter whether or not we have an empty no-op spec.
Screenshots or screen recordings
How to set up and validate locally
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Closes #340017 (closed)