diff --git a/app/models/ci/job_artifact.rb b/app/models/ci/job_artifact.rb index 30374a52c38a3c8873bcb5a9948cff2f0be5f9bc..2a65480ff45ed3e4d08dfb50b282aade7533fcff 100644 --- a/app/models/ci/job_artifact.rb +++ b/app/models/ci/job_artifact.rb @@ -14,6 +14,8 @@ class JobArtifact < Ci::ApplicationRecord include EachBatch include Gitlab::Utils::StrongMemoize + ROUTING_FEATURE_FLAG = :ci_partitioning_use_ci_job_artifacts_routing_table + self.primary_key = :id self.sequence_name = :ci_job_artifacts_id_seq @@ -157,7 +159,10 @@ class JobArtifact < Ci::ApplicationRecord validate :validate_file_format!, unless: :trace?, on: :create update_project_statistics project_statistics_name: :build_artifacts_size - partitionable scope: :job + partitionable scope: :job, through: { + table: :p_ci_job_artifacts, + flag: ROUTING_FEATURE_FLAG + } scope :not_expired, -> { where('expire_at IS NULL OR expire_at > ?', Time.current) } scope :for_sha, ->(sha, project_id) { joins(job: :pipeline).where(ci_pipelines: { sha: sha, project_id: project_id }) } diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index 23c2164135dc59b2b2e099a0f69ae7f410229108..40a7623332418364d10e605896ede946f405abd5 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -769,7 +769,7 @@ def latest_report_artifacts ::Gitlab::SafeRequestStore.fetch("pipeline:#{self.id}:latest_report_artifacts") do ::Ci::JobArtifact.where( id: job_artifacts.all_reports - .select('max(ci_job_artifacts.id) as id') + .select("max(#{Ci::JobArtifact.quoted_table_name}.id) as id") .group(:file_type) ) .preload(:job) diff --git a/config/feature_flags/gitlab_com_derisk/ci_partitioning_use_ci_job_artifacts_routing_table.yml b/config/feature_flags/gitlab_com_derisk/ci_partitioning_use_ci_job_artifacts_routing_table.yml new file mode 100644 index 0000000000000000000000000000000000000000..e85f84643a3ee41ec548b2181f113fa577f7ebab --- /dev/null +++ b/config/feature_flags/gitlab_com_derisk/ci_partitioning_use_ci_job_artifacts_routing_table.yml @@ -0,0 +1,9 @@ +--- +name: ci_partitioning_use_ci_job_artifacts_routing_table +feature_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/440760 +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/144709 +rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/441759 +milestone: '16.10' +group: group::pipeline execution +type: gitlab_com_derisk +default_enabled: false diff --git a/config/gitlab_loose_foreign_keys.yml b/config/gitlab_loose_foreign_keys.yml index c491d45a5f352ff6786f599570239a24e42f69aa..aa5582e3a3532658af9ecab5d53b3fe21713a4c9 100644 --- a/config/gitlab_loose_foreign_keys.yml +++ b/config/gitlab_loose_foreign_keys.yml @@ -35,10 +35,6 @@ ci_group_variables: - table: namespaces column: group_id on_delete: async_delete -ci_job_artifacts: - - table: projects - column: project_id - on_delete: async_delete ci_job_token_group_scope_links: - table: users column: added_by_id @@ -294,6 +290,10 @@ p_ci_builds_metadata: - table: projects column: project_id on_delete: async_delete +p_ci_job_artifacts: + - table: projects + column: project_id + on_delete: async_delete p_ci_runner_machine_builds: - table: ci_runner_machines column: runner_machine_id diff --git a/db/docs/p_ci_job_artifacts.yml b/db/docs/p_ci_job_artifacts.yml index 5bb3dc580c7373097ce5bf99ff57fcd82b1412db..adca9eca071e448d682b05a23907e7a46e4c3799 100644 --- a/db/docs/p_ci_job_artifacts.yml +++ b/db/docs/p_ci_job_artifacts.yml @@ -2,6 +2,7 @@ table_name: p_ci_job_artifacts classes: - Ci::JobArtifact +- Ci::JobArtifact::Partitioned feature_categories: - continuous_integration description: Routing table for ci_job_artifacts diff --git a/spec/db/schema_spec.rb b/spec/db/schema_spec.rb index 898926ec51f7b0a3f9702b7bd82923e71d721b4b..9a0a4587e6ea5b8b032ff8fec64e60e00b4bfef1 100644 --- a/spec/db/schema_spec.rb +++ b/spec/db/schema_spec.rb @@ -49,6 +49,7 @@ chat_names: %w[chat_id team_id user_id], chat_teams: %w[team_id], ci_builds: %w[project_id runner_id user_id erased_by_id trigger_request_id partition_id auto_canceled_by_partition_id], + ci_job_artifacts: %w[partition_id project_id job_id], ci_namespace_monthly_usages: %w[namespace_id], ci_pipeline_artifacts: %w[partition_id], ci_pipeline_chat_data: %w[partition_id], diff --git a/spec/lib/gitlab/database/sharding_key_spec.rb b/spec/lib/gitlab/database/sharding_key_spec.rb index 2a3f2a3155cf56552514ce22d13f56f6bd399eb6..9891914a38cbdcbd2c825684c2959f6a81265d38 100644 --- a/spec/lib/gitlab/database/sharding_key_spec.rb +++ b/spec/lib/gitlab/database/sharding_key_spec.rb @@ -50,7 +50,8 @@ 'value_stream_dashboard_counts.namespace_id', # https://gitlab.com/gitlab-org/gitlab/-/issues/439555 'zoekt_indices.namespace_id', 'zoekt_repositories.project_identifier', - 'ci_namespace_monthly_usages.namespace_id' # https://gitlab.com/gitlab-org/gitlab/-/issues/321400 + 'ci_namespace_monthly_usages.namespace_id', # https://gitlab.com/gitlab-org/gitlab/-/issues/321400 + 'ci_job_artifacts.project_id' ] end diff --git a/spec/models/ci/job_artifact_spec.rb b/spec/models/ci/job_artifact_spec.rb index e65c1e2f577043dd805fc13f1aa835ab3e3b65a7..69ab2692562f3c9f14bf90fc57bd71ed15477ce2 100644 --- a/spec/models/ci/job_artifact_spec.rb +++ b/spec/models/ci/job_artifact_spec.rb @@ -892,4 +892,26 @@ def file_type_limit_failure_message(type, limit_name) it_behaves_like 'returning attributes for object deletion' end end + + describe 'routing table switch' do + context 'with ff disabled' do + before do + stub_feature_flags(ci_partitioning_use_ci_job_artifacts_routing_table: false) + end + + it 'uses the legacy table' do + expect(described_class.table_name).to eq('ci_job_artifacts') + end + end + + context 'with ff enabled' do + before do + stub_feature_flags(ci_partitioning_use_ci_job_artifacts_routing_table: true) + end + + it 'uses the routing table' do + expect(described_class.table_name).to eq('p_ci_job_artifacts') + end + end + end end diff --git a/spec/services/ci/unlock_artifacts_service_spec.rb b/spec/services/ci/unlock_artifacts_service_spec.rb index c149eaf41e5ca44b26f365cfaf7fe7ff9d826ccb..2219ee3bddb7a3493163a95d7550f966877ed9c2 100644 --- a/spec/services/ci/unlock_artifacts_service_spec.rb +++ b/spec/services/ci/unlock_artifacts_service_spec.rb @@ -208,6 +208,7 @@ subject { described_class.new(pipeline.project, pipeline.user).unlock_job_artifacts_query(pipeline_ids) } let(:builds_table) { Ci::Build.quoted_table_name } + let(:job_artifacts_table) { Ci::JobArtifact.quoted_table_name } context 'when given a single pipeline ID' do let(:pipeline_ids) { [older_pipeline.id] } @@ -215,11 +216,11 @@ it 'produces the expected SQL string' do expect(subject.squish).to eq <<~SQL.squish UPDATE - "ci_job_artifacts" + #{job_artifacts_table} SET "locked" = 0 WHERE - "ci_job_artifacts"."job_id" IN + #{job_artifacts_table}."job_id" IN (SELECT #{builds_table}."id" FROM @@ -228,7 +229,7 @@ #{builds_table}."type" = 'Ci::Build' AND #{builds_table}."commit_id" = #{older_pipeline.id}) RETURNING - ("ci_job_artifacts"."id") + (#{job_artifacts_table}."id") SQL end end @@ -239,11 +240,11 @@ it 'produces the expected SQL string' do expect(subject.squish).to eq <<~SQL.squish UPDATE - "ci_job_artifacts" + #{job_artifacts_table} SET "locked" = 0 WHERE - "ci_job_artifacts"."job_id" IN + #{job_artifacts_table}."job_id" IN (SELECT #{builds_table}."id" FROM @@ -252,7 +253,7 @@ #{builds_table}."type" = 'Ci::Build' AND #{builds_table}."commit_id" IN (#{pipeline_ids.join(', ')})) RETURNING - ("ci_job_artifacts"."id") + (#{job_artifacts_table}."id") SQL end end