From 8b5553daa43d48fdef42f0f2a3f700580dea770b Mon Sep 17 00:00:00 2001 From: Mayra Cabrera Date: Wed, 30 Jan 2019 09:39:48 -0600 Subject: [PATCH] Use a single sql statement for ADO query Since cluster_projects table does not have a lot of records, currently it has 11,638, it seems better to use a single sql statement to update all the records --- ...e_auto_dev_ops_domain_to_cluster_domain.rb | 79 ++++++------------- ...o_dev_ops_domain_to_cluster_domain_spec.rb | 10 ++- 2 files changed, 33 insertions(+), 56 deletions(-) diff --git a/db/migrate/20190129165720_migrate_auto_dev_ops_domain_to_cluster_domain.rb b/db/migrate/20190129165720_migrate_auto_dev_ops_domain_to_cluster_domain.rb index 2d3e9acaa62..392e64eeade 100644 --- a/db/migrate/20190129165720_migrate_auto_dev_ops_domain_to_cluster_domain.rb +++ b/db/migrate/20190129165720_migrate_auto_dev_ops_domain_to_cluster_domain.rb @@ -6,12 +6,7 @@ class MigrateAutoDevOpsDomainToClusterDomain < ActiveRecord::Migration[5.0] DOWNTIME = false def up - domains_info = connection.exec_query(project_auto_devops_query).rows - domains_info.each_slice(1_000) do |batch| - update_clusters_query = build_clusters_query(Hash[*batch.flatten]) - - connection.exec_query(update_clusters_query) - end + execute(update_clusters_domain_query) end def down @@ -20,59 +15,35 @@ class MigrateAutoDevOpsDomainToClusterDomain < ActiveRecord::Migration[5.0] private - def project_auto_devops_table - @project_auto_devops_table ||= ProjectAutoDevops.arel_table + def update_clusters_domain_query + if Gitlab::Database.mysql? + mysql_query + else + postgresql_query + end end - def cluster_projects_table - @cluster_projects_table ||= Clusters::Project.arel_table - end - - # Fetches ProjectAutoDevops records with: - # - A domain set - # - With a Clusters::Project related to Project - # - # Returns an array of arrays like: - # => [ - # [177, "104.198.38.135.nip.io"], - # [178, "35.232.213.111.nip.io"], - # ... - # ] - # Where the first element is the Cluster ID and - # the second element is the domain. - def project_auto_devops_query - project_auto_devops_table.join(cluster_projects_table, Arel::Nodes::OuterJoin) - .on(project_auto_devops_table[:project_id].eq(cluster_projects_table[:project_id])) - .where(project_auto_devops_table[:domain].not_eq(nil).and(project_auto_devops_table[:domain].not_eq(''))) - .project(cluster_projects_table[:cluster_id], project_auto_devops_table[:domain]) - .to_sql - end - - # Returns an SQL UPDATE query using a CASE statement - # to update multiple cluster rows with different values. - # - # Example: - # UPDATE clusters - # SET domain = (CASE - # WHEN id = 177 then '104.198.38.135.nip.io' - # WHEN id = 178 then '35.232.213.111.nip.io' - # WHEN id = 179 then '35.232.168.149.nip.io' - # WHEN id = 180 then '35.224.116.88.nip.io' - # END) - # WHERE id IN (177,178,179,180); - def build_clusters_query(cluster_domains_info) + def mysql_query <<~HEREDOC - UPDATE clusters - SET domain = (CASE - #{cluster_when_statements(cluster_domains_info)} - END) - WHERE id IN (#{cluster_domains_info.keys.join(",")}); + UPDATE clusters, project_auto_devops, cluster_projects + SET + clusters.domain = project_auto_devops.domain + WHERE + cluster_projects.cluster_id = clusters.id + AND project_auto_devops.project_id = cluster_projects.project_id + AND project_auto_devops.domain != '' HEREDOC end - def cluster_when_statements(cluster_domains_info) - cluster_domains_info.map do |cluster_id, domain| - "WHEN id = #{cluster_id} then '#{domain}'" - end.join("\n") + def postgresql_query + <<~HEREDOC + UPDATE clusters + SET domain = project_auto_devops.domain + FROM cluster_projects, project_auto_devops + WHERE + cluster_projects.cluster_id = clusters.id + AND project_auto_devops.project_id = cluster_projects.project_id + AND project_auto_devops.domain != '' + HEREDOC end end diff --git a/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb b/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb index 09013ee4bd0..c7fd27588a5 100644 --- a/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb +++ b/spec/migrations/migrate_auto_dev_ops_domain_to_cluster_domain_spec.rb @@ -46,12 +46,14 @@ describe MigrateAutoDevOpsDomainToClusterDomain, :migration do let(:domain) { 'example-domain.com' } before do - setup_cluster_projects_with_domain(quantity: 20, domain: nil) + setup_cluster_projects_with_domain(quantity: 25, domain: nil) end it 'should only update specific cluster projects' do migrate! + expect(clusters_with_domain.count).to eq(20) + project_auto_devops_with_domain.each do |project_auto_devops| cluster_project = Clusters::Project.find_by(project_id: project_auto_devops.project_id) cluster = Clusters::Cluster.find(cluster_project.cluster_id) @@ -59,6 +61,8 @@ describe MigrateAutoDevOpsDomainToClusterDomain, :migration do expect(cluster.domain).to be_present end + expect(clusters_without_domain.count).to eq(25) + project_auto_devops_without_domain.each do |project_auto_devops| cluster_project = Clusters::Project.find_by(project_id: project_auto_devops.project_id) cluster = Clusters::Cluster.find(cluster_project.cluster_id) @@ -74,10 +78,12 @@ describe MigrateAutoDevOpsDomainToClusterDomain, :migration do cluster_projects = cluster_projects_table.last(quantity) cluster_projects.each do |cluster_project| + specific_domain = "#{cluster_project.id}-#{domain}" if domain + project_auto_devops_table.create( project_id: cluster_project.project_id, enabled: true, - domain: domain + domain: specific_domain ) end end