From 2aa0b877bae07c8a73d02d4ffd99a42aa706531b Mon Sep 17 00:00:00 2001 From: Marius Bobin <mbobin@gitlab.com> Date: Fri, 27 Oct 2023 18:36:41 +0000 Subject: [PATCH] Enable partitioned query analyzers for ci_builds --- .../fix_security_scan_statuses.rb | 2 +- .../fix_security_scan_statuses_spec.rb | 2 +- ...k_to_vulnerabilities_state_transition_spec.rb | 2 +- .../purge_stale_security_scans_spec.rb | 3 ++- .../create_vulnerability_links_spec.rb | 2 +- .../ci/partitioning_routing_analyzer.rb | 2 +- ...ve_backfilled_job_artifacts_expire_at_spec.rb | 2 +- .../reestablished_connection_stack_spec.rb | 2 +- .../gitlab_schemas_metrics_spec.rb | 6 +++--- .../gitlab_schemas_validate_connection_spec.rb | 16 ++++++++-------- ...chedule_fixing_security_scan_statuses_spec.rb | 11 ++++++----- .../database/partitioning_routing_analyzer.rb | 7 +++++++ .../helpers/migrations_helpers_spec.rb | 4 ++-- 13 files changed, 35 insertions(+), 26 deletions(-) create mode 100644 spec/support/database/partitioning_routing_analyzer.rb diff --git a/ee/lib/ee/gitlab/background_migration/fix_security_scan_statuses.rb b/ee/lib/ee/gitlab/background_migration/fix_security_scan_statuses.rb index 30a68a091c1af..72784361aa318 100644 --- a/ee/lib/ee/gitlab/background_migration/fix_security_scan_statuses.rb +++ b/ee/lib/ee/gitlab/background_migration/fix_security_scan_statuses.rb @@ -53,7 +53,7 @@ def artifacts_available? end class CiBuild < ::Ci::ApplicationRecord # rubocop:disable Style/Documentation - self.table_name = 'ci_builds' + self.table_name = 'p_ci_builds' self.inheritance_column = :_type_disabled self.primary_key = :id diff --git a/ee/spec/lib/ee/gitlab/background_migration/fix_security_scan_statuses_spec.rb b/ee/spec/lib/ee/gitlab/background_migration/fix_security_scan_statuses_spec.rb index b8f24ebebad94..be625df9b4c0d 100644 --- a/ee/spec/lib/ee/gitlab/background_migration/fix_security_scan_statuses_spec.rb +++ b/ee/spec/lib/ee/gitlab/background_migration/fix_security_scan_statuses_spec.rb @@ -6,7 +6,7 @@ let(:namespaces) { table(:namespaces) } let(:projects) { table(:projects) } let(:pipelines) { table(:ci_pipelines, database: :ci) } - let(:builds) { table(:ci_builds, database: :ci) { |model| model.primary_key = :id } } + let(:builds) { table(:p_ci_builds, database: :ci) { |model| model.primary_key = :id } } let(:security_scans) { table(:security_scans) } let(:namespace) { namespaces.create!(name: "foo", path: "bar") } diff --git a/ee/spec/lib/ee/gitlab/background_migration/migrate_vulnerabilities_feedback_to_vulnerabilities_state_transition_spec.rb b/ee/spec/lib/ee/gitlab/background_migration/migrate_vulnerabilities_feedback_to_vulnerabilities_state_transition_spec.rb index c2a1afaea65ee..1eaf6b56f8208 100644 --- a/ee/spec/lib/ee/gitlab/background_migration/migrate_vulnerabilities_feedback_to_vulnerabilities_state_transition_spec.rb +++ b/ee/spec/lib/ee/gitlab/background_migration/migrate_vulnerabilities_feedback_to_vulnerabilities_state_transition_spec.rb @@ -37,7 +37,7 @@ let(:vulnerability_state_transitions) { table(:vulnerability_state_transitions) } let(:security_scans) { table(:security_scans) } let(:security_findings) { table(:security_findings) } - let(:ci_builds) { table(:ci_builds, database: :ci) { |model| model.primary_key = :id } } + let(:ci_builds) { table(:p_ci_builds, database: :ci) { |model| model.primary_key = :id } } let(:ci_job_artifacts) { table(:ci_job_artifacts, database: :ci) } let(:ci_pipelines) { table(:ci_pipelines, database: :ci) } diff --git a/ee/spec/lib/ee/gitlab/background_migration/purge_stale_security_scans_spec.rb b/ee/spec/lib/ee/gitlab/background_migration/purge_stale_security_scans_spec.rb index 725eaff4fc863..1584ab91aec07 100644 --- a/ee/spec/lib/ee/gitlab/background_migration/purge_stale_security_scans_spec.rb +++ b/ee/spec/lib/ee/gitlab/background_migration/purge_stale_security_scans_spec.rb @@ -3,7 +3,8 @@ require 'spec_helper' RSpec.describe Gitlab::BackgroundMigration::PurgeStaleSecurityScans, - :suppress_gitlab_schemas_validate_connection, schema: 20220407163559 do + :suppress_partitioning_routing_analyzer, :suppress_gitlab_schemas_validate_connection, + schema: 20220407163559 do let(:namespaces) { table(:namespaces) } let(:projects) { table(:projects) } let(:pipelines) { table(:ci_pipelines) } diff --git a/ee/spec/lib/gitlab/background_migration/create_vulnerability_links_spec.rb b/ee/spec/lib/gitlab/background_migration/create_vulnerability_links_spec.rb index e61c4c390c717..1b406bfddaeaa 100644 --- a/ee/spec/lib/gitlab/background_migration/create_vulnerability_links_spec.rb +++ b/ee/spec/lib/gitlab/background_migration/create_vulnerability_links_spec.rb @@ -31,7 +31,7 @@ let(:vulnerability_feedback) { table(:vulnerability_feedback) } let(:security_scans) { table(:security_scans) } let(:security_findings) { table(:security_findings) } - let(:ci_builds) { table(:ci_builds, database: :ci) { |model| model.primary_key = :id } } + let(:ci_builds) { table(:p_ci_builds, database: :ci) { |model| model.primary_key = :id } } let(:ci_pipelines) { table(:ci_pipelines, database: :ci) } let(:ci_job_artifacts) { table(:ci_job_artifacts, database: :ci) } diff --git a/lib/gitlab/database/query_analyzers/ci/partitioning_routing_analyzer.rb b/lib/gitlab/database/query_analyzers/ci/partitioning_routing_analyzer.rb index eb55ebc761910..c2f94b7b0e68c 100644 --- a/lib/gitlab/database/query_analyzers/ci/partitioning_routing_analyzer.rb +++ b/lib/gitlab/database/query_analyzers/ci/partitioning_routing_analyzer.rb @@ -8,7 +8,7 @@ module Ci class PartitioningRoutingAnalyzer < Database::QueryAnalyzers::Base RoutingTableNotUsedError = Class.new(QueryAnalyzerError) - ENABLED_TABLES = %w[ci_builds_metadata].freeze + ENABLED_TABLES = %w[ci_builds ci_builds_metadata].freeze class << self def enabled? diff --git a/spec/lib/gitlab/background_migration/remove_backfilled_job_artifacts_expire_at_spec.rb b/spec/lib/gitlab/background_migration/remove_backfilled_job_artifacts_expire_at_spec.rb index af8b5240e4098..4c989ba9cef5e 100644 --- a/spec/lib/gitlab/background_migration/remove_backfilled_job_artifacts_expire_at_spec.rb +++ b/spec/lib/gitlab/background_migration/remove_backfilled_job_artifacts_expire_at_spec.rb @@ -7,7 +7,7 @@ describe '#perform' do let(:job_artifact) { table(:ci_job_artifacts, database: :ci) } - let(:jobs) { table(:ci_builds, database: :ci) { |model| model.primary_key = :id } } + let(:jobs) { table(:p_ci_builds, database: :ci) { |model| model.primary_key = :id } } let(:test_worker) do described_class.new( diff --git a/spec/lib/gitlab/database/migrations/reestablished_connection_stack_spec.rb b/spec/lib/gitlab/database/migrations/reestablished_connection_stack_spec.rb index c6327de98d14b..6e943307ae64a 100644 --- a/spec/lib/gitlab/database/migrations/reestablished_connection_stack_spec.rb +++ b/spec/lib/gitlab/database/migrations/reestablished_connection_stack_spec.rb @@ -31,7 +31,7 @@ # establish connection ApplicationRecord.connection.select_one("SELECT 1 FROM projects LIMIT 1") - Ci::ApplicationRecord.connection.select_one("SELECT 1 FROM ci_builds LIMIT 1") + Ci::ApplicationRecord.connection.select_one("SELECT 1 FROM p_ci_builds LIMIT 1") end expect(new_handler).not_to eq(original_handler), "is reconnected" diff --git a/spec/lib/gitlab/database/query_analyzers/gitlab_schemas_metrics_spec.rb b/spec/lib/gitlab/database/query_analyzers/gitlab_schemas_metrics_spec.rb index f325060e5926d..1909e134e66fd 100644 --- a/spec/lib/gitlab/database/query_analyzers/gitlab_schemas_metrics_spec.rb +++ b/spec/lib/gitlab/database/query_analyzers/gitlab_schemas_metrics_spec.rb @@ -35,7 +35,7 @@ }, "for query accessing gitlab_ci and gitlab_main" => { model: ApplicationRecord, - sql: "SELECT 1 FROM projects LEFT JOIN ci_builds ON ci_builds.project_id=projects.id", + sql: "SELECT 1 FROM projects LEFT JOIN p_ci_builds ON p_ci_builds.project_id=projects.id", expectations: { gitlab_schemas: "gitlab_ci,gitlab_main_cell", db_config_name: "main" @@ -43,7 +43,7 @@ }, "for query accessing gitlab_ci and gitlab_main the gitlab_schemas is always ordered" => { model: ApplicationRecord, - sql: "SELECT 1 FROM ci_builds LEFT JOIN projects ON ci_builds.project_id=projects.id", + sql: "SELECT 1 FROM p_ci_builds LEFT JOIN projects ON p_ci_builds.project_id=projects.id", expectations: { gitlab_schemas: "gitlab_ci,gitlab_main_cell", db_config_name: "main" @@ -51,7 +51,7 @@ }, "for query accessing CI database" => { model: Ci::ApplicationRecord, - sql: "SELECT 1 FROM ci_builds", + sql: "SELECT 1 FROM p_ci_builds", expectations: { gitlab_schemas: "gitlab_ci", db_config_name: "ci" diff --git a/spec/lib/gitlab/database/query_analyzers/gitlab_schemas_validate_connection_spec.rb b/spec/lib/gitlab/database/query_analyzers/gitlab_schemas_validate_connection_spec.rb index e3ff5ab477905..0664508fa8db1 100644 --- a/spec/lib/gitlab/database/query_analyzers/gitlab_schemas_validate_connection_spec.rb +++ b/spec/lib/gitlab/database/query_analyzers/gitlab_schemas_validate_connection_spec.rb @@ -26,14 +26,14 @@ }, "for query accessing gitlab_ci and gitlab_main" => { model: ApplicationRecord, - sql: "SELECT 1 FROM projects LEFT JOIN ci_builds ON ci_builds.project_id=projects.id", - expect_error: /The query tried to access \["projects", "ci_builds"\]/, + sql: "SELECT 1 FROM projects LEFT JOIN p_ci_builds ON p_ci_builds.project_id=projects.id", + expect_error: /The query tried to access \["projects", "p_ci_builds"\]/, setup: -> (_) { skip_if_shared_database(:ci) } }, "for query accessing gitlab_ci and gitlab_main the gitlab_schemas is always ordered" => { model: ApplicationRecord, - sql: "SELECT 1 FROM ci_builds LEFT JOIN projects ON ci_builds.project_id=projects.id", - expect_error: /The query tried to access \["ci_builds", "projects"\]/, + sql: "SELECT 1 FROM p_ci_builds LEFT JOIN projects ON p_ci_builds.project_id=projects.id", + expect_error: /The query tried to access \["p_ci_builds", "projects"\]/, setup: -> (_) { skip_if_shared_database(:ci) } }, "for query accessing main table from CI database" => { @@ -44,13 +44,13 @@ }, "for query accessing CI database" => { model: Ci::ApplicationRecord, - sql: "SELECT 1 FROM ci_builds", + sql: "SELECT 1 FROM p_ci_builds", expect_error: nil }, "for query accessing CI table from main database" => { model: ::ApplicationRecord, - sql: "SELECT 1 FROM ci_builds", - expect_error: /The query tried to access \["ci_builds"\]/, + sql: "SELECT 1 FROM p_ci_builds", + expect_error: /The query tried to access \["p_ci_builds"\]/, setup: -> (_) { skip_if_shared_database(:ci) } }, "for query accessing unknown gitlab_schema" => { @@ -89,7 +89,7 @@ it "throws an error when trying to access a table that belongs to the gitlab_ci schema from the main database" do expect do - ApplicationRecord.connection.execute("select * from ci_builds limit 1") + ApplicationRecord.connection.execute("select * from p_ci_builds limit 1") end.to raise_error(Gitlab::Database::QueryAnalyzers::GitlabSchemasValidateConnection::CrossSchemaAccessError) end end diff --git a/spec/migrations/schedule_fixing_security_scan_statuses_spec.rb b/spec/migrations/schedule_fixing_security_scan_statuses_spec.rb index 56d30e716765f..f43f58d3be222 100644 --- a/spec/migrations/schedule_fixing_security_scan_statuses_spec.rb +++ b/spec/migrations/schedule_fixing_security_scan_statuses_spec.rb @@ -4,20 +4,21 @@ require_migration! RSpec.describe ScheduleFixingSecurityScanStatuses, - :suppress_gitlab_schemas_validate_connection, feature_category: :vulnerability_management do + :suppress_gitlab_schemas_validate_connection, :suppress_partitioning_routing_analyzer, + feature_category: :vulnerability_management do let!(:namespaces) { table(:namespaces) } let!(:projects) { table(:projects) } - let!(:pipelines) { table(:ci_pipelines) } - let!(:builds) { table(:ci_builds) } + let!(:pipelines) { table(:ci_pipelines, database: :ci) } + let!(:builds) { table(:ci_builds, database: :ci) { |model| model.primary_key = :id } } let!(:security_scans) { table(:security_scans) } let!(:namespace) { namespaces.create!(name: "foo", path: "bar") } let!(:project) { projects.create!(namespace_id: namespace.id, project_namespace_id: namespace.id) } let!(:pipeline) do - pipelines.create!(project_id: project.id, ref: 'master', sha: 'adf43c3a', status: 'success', partition_id: 1) + pipelines.create!(project_id: project.id, ref: 'master', sha: 'adf43c3a', status: 'success', partition_id: 100) end - let!(:ci_build) { builds.create!(commit_id: pipeline.id, retried: false, type: 'Ci::Build', partition_id: 1) } + let!(:ci_build) { builds.create!(commit_id: pipeline.id, retried: false, type: 'Ci::Build', partition_id: 100) } let!(:security_scan_1) { security_scans.create!(build_id: ci_build.id, scan_type: 1, created_at: 91.days.ago) } let!(:security_scan_2) { security_scans.create!(build_id: ci_build.id, scan_type: 2) } diff --git a/spec/support/database/partitioning_routing_analyzer.rb b/spec/support/database/partitioning_routing_analyzer.rb new file mode 100644 index 0000000000000..b1edd81738677 --- /dev/null +++ b/spec/support/database/partitioning_routing_analyzer.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +RSpec.configure do |config| + config.around(:each, :suppress_partitioning_routing_analyzer) do |example| + Gitlab::Database::QueryAnalyzers::Ci::PartitioningRoutingAnalyzer.with_suppressed(&example) + end +end diff --git a/spec/support_specs/helpers/migrations_helpers_spec.rb b/spec/support_specs/helpers/migrations_helpers_spec.rb index 2af161513508c..725caef7a633b 100644 --- a/spec/support_specs/helpers/migrations_helpers_spec.rb +++ b/spec/support_specs/helpers/migrations_helpers_spec.rb @@ -55,7 +55,7 @@ end it 'create a class based on the CI base model' do - klass = helper.table(:ci_builds, database: :ci) + klass = helper.table(:p_ci_builds, database: :ci) { |model| model.primary_key = :id } expect(klass.connection_specification_name).to eq('Ci::ApplicationRecord') end end @@ -66,7 +66,7 @@ end it 'creates a class based on main base model' do - klass = helper.table(:ci_builds, database: :ci) + klass = helper.table(:p_ci_builds, database: :ci) { |model| model.primary_key = :id } expect(klass.connection_specification_name).to eq('ActiveRecord::Base') end end -- GitLab