Add temporary partial index to speed up the migration
What are the relevant issue numbers?
Closes #32469 (closed)
Merge request reports
Activity
@yorickpeterse Please review, thanks. I also added
IF NOT EXISTSandIF EXISTSin case it's interrupted in the middle, messing things up.@timothyandrew This is not really a regression, but as you know this is an update to the slow migration to speed it up. So I think we should get this in %9.2. Since we already ran the migration, this is more for the customers.
/cc @selfup
Edited by username-removed-423915@timothyandrew We did that manually on production (not the same code, but the same concept/similar SQL), and I am not sure if this should be considered tested enough.
@godfat: Let's get this signed-off on before we pick to
9-2-stable, according to https://gitlab.com/gitlab-org/gitlab-ce/blob/master/PROCESS.md#after-the-7th:- a release manager: @timothyandrew
- the VP of Engineering: @stanhu
- an Engineering Lead: @rymai
Edited by Stan Hu@timothyandrew This is not even reviewed/merged yet :) @yorickpeterse should also sign because it's raised by him.
assigned to @godfat
@timothyandrew Not including this in 9.2.0 means that any instance with a lot of CI builds can take a very long time to migrate. On GitLab.com for example it took around 1,5 hours of which most was done without the index (we created it by the time progress was around 80%).
added 1 commit
- 1aa67247 - Add temporary partial index to speed up the migration
assigned to @yorickpeterse
added 1 commit
- 1aa67247 - Add temporary partial index to speed up the migration
assigned to @rymai
-
@yorickpeterse: Good enough for me! I'd still like to get the other two sign-offs before this is picked (after it's merged first, of course!).
/cc @selfup
@godfat Thanks, looks good to me!
@timothyandrew Ok for me.
mentioned in commit 27eab8a4
@rymai Thanks!
@timothyandrew @selfup Stan signed it too https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11534#note_30103035 Let's pick this if possible. If we can't make it, I think that's fine, we could do it in 9.2.1.
added Next Patch Release label
mentioned in merge request !11619 (closed)
19 # This is slow update as it does single-row query 20 # This is designed to be run as idle, or a post deployment migration 21 is_retried = Arel.sql("((#{latest_id}) != ci_builds.id)") 22 23 update_column_in_batches(:ci_builds, :retried, is_retried) do |table, query| 24 query.where(table[:retried].eq(nil)) 25 end 24 26 end 25 27 end 26 28 27 29 def down 28 30 end 31 32 def with_temporary_partial_index 33 if Gitlab::Database.postgresql? 34 execute 'CREATE INDEX CONCURRENTLY IF NOT EXISTS index_for_ci_builds_retried_migration ON ci_builds (id) WHERE retried IS NULL;' Yes and we'll need to follow up in !11620 (closed) as you pointed out.
@godfat This MR doesn't apply cleanly onto
9-2-stable. Could you please fix the conflicts in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11619?@ClemMakesApps As explained in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/11619#note_30427123 we no longer need to pick this.
