diff options
25 files changed, 303 insertions, 216 deletions
diff --git a/.gitlab/ci/rails.gitlab-ci.yml b/.gitlab/ci/rails.gitlab-ci.yml index 95780116800..0049917ce81 100644 --- a/.gitlab/ci/rails.gitlab-ci.yml +++ b/.gitlab/ci/rails.gitlab-ci.yml @@ -147,6 +147,12 @@ rspec unit pg12 single-db: - .single-db-rspec - .rails:rules:single-db +rspec unit pg12 single-db-ci-connection: + extends: + - rspec unit pg12 + - .single-db-ci-connection-rspec + - .rails:rules:single-db-ci-connection + rspec unit pg12 praefect: extends: - rspec unit pg12 @@ -171,6 +177,12 @@ rspec integration pg12 single-db: - .single-db-rspec - .rails:rules:single-db +rspec integration pg12 single-db-ci-connection: + extends: + - rspec integration pg12 + - .single-db-ci-connection-rspec + - .rails:rules:single-db-ci-connection + rspec integration pg12 praefect: extends: - rspec integration pg12 @@ -197,6 +209,12 @@ rspec system pg12 single-db: - .single-db-rspec - .rails:rules:single-db +rspec system pg12 single-db-ci-connection: + extends: + - rspec system pg12 + - .single-db-ci-connection-rspec + - .rails:rules:single-db-ci-connection + rspec system pg12 praefect: extends: - rspec system pg12 @@ -289,6 +307,12 @@ rspec:coverage: - rspec unit pg12 single-db - rspec integration pg12 single-db - rspec system pg12 single-db + # FOSS/EE single-db-ci-connection jobs + - rspec migration pg12 single-db-ci-connection + - rspec background_migration pg12 single-db-ci-connection + - rspec unit pg12 single-db-ci-connection + - rspec integration pg12 single-db-ci-connection + - rspec system pg12 single-db-ci-connection # EE jobs - rspec-ee migration pg12 - rspec-ee background_migration pg12 @@ -307,24 +331,35 @@ rspec:coverage: - rspec-ee unit pg12 single-db - rspec-ee integration pg12 single-db - rspec-ee system pg12 single-db + # EE single-db-ci-connection jobs + - rspec-ee migration pg12 single-db-ci-connection + - rspec-ee background_migration pg12 single-db-ci-connection + - rspec-ee unit pg12 single-db-ci-connection + - rspec-ee integration pg12 single-db-ci-connection + - rspec-ee system pg12 single-db-ci-connection # Memory jobs - memory-on-boot # As-if-FOSS jobs - rspec migration pg12-as-if-foss - rspec migration pg12-as-if-foss predictive - rspec migration pg12-as-if-foss single-db + - rspec migration pg12-as-if-foss single-db-ci-connection - rspec background_migration pg12-as-if-foss - rspec background_migration pg12-as-if-foss predictive - rspec background_migration pg12-as-if-foss single-db + - rspec background_migration pg12-as-if-foss single-db-ci-connection - rspec unit pg12-as-if-foss - rspec unit pg12-as-if-foss predictive - rspec unit pg12-as-if-foss single-db + - rspec unit pg12-as-if-foss single-db-ci-connection - rspec integration pg12-as-if-foss - rspec integration pg12-as-if-foss predictive - rspec integration pg12-as-if-foss single-db + - rspec integration pg12-as-if-foss single-db-ci-connection - rspec system pg12-as-if-foss - rspec system pg12-as-if-foss predictive - rspec system pg12-as-if-foss single-db + - rspec system pg12-as-if-foss single-db-ci-connection script: - run_timed_command "bundle exec scripts/merge-simplecov" coverage: '/LOC \((\d+\.\d+%)\) covered.$/' @@ -470,6 +505,12 @@ rspec unit pg12-as-if-foss single-db: - .single-db-rspec - .rails:rules:single-db +rspec unit pg12-as-if-foss single-db-ci-connection: + extends: + - rspec unit pg12-as-if-foss + - .single-db-ci-connection-rspec + - .rails:rules:single-db-ci-connection + rspec integration pg12-as-if-foss: extends: - .rspec-base-pg12-as-if-foss @@ -488,6 +529,12 @@ rspec integration pg12-as-if-foss single-db: - .single-db-rspec - .rails:rules:single-db +rspec integration pg12-as-if-foss single-db-ci-connection: + extends: + - rspec integration pg12-as-if-foss + - .single-db-ci-connection-rspec + - .rails:rules:single-db-ci-connection + rspec system pg12-as-if-foss: extends: - .rspec-base-pg12-as-if-foss @@ -506,6 +553,12 @@ rspec system pg12-as-if-foss single-db: - .single-db-rspec - .rails:rules:single-db +rspec system pg12-as-if-foss single-db-ci-connection: + extends: + - rspec system pg12-as-if-foss + - .single-db-ci-connection-rspec + - .rails:rules:single-db-ci-connection + rspec-ee migration pg12: extends: - .rspec-ee-base-pg12 @@ -591,6 +644,12 @@ rspec-ee unit pg12 single-db: - .single-db-rspec - .rails:rules:single-db +rspec-ee unit pg12 single-db-ci-connection: + extends: + - rspec-ee unit pg12 + - .single-db-ci-connection-rspec + - .rails:rules:single-db-ci-connection + rspec-ee integration pg12: extends: - .rspec-ee-base-pg12 @@ -614,6 +673,12 @@ rspec-ee integration pg12 single-db: - .single-db-rspec - .rails:rules:single-db +rspec-ee integration pg12 single-db-ci-connection: + extends: + - rspec-ee integration pg12 + - .single-db-ci-connection-rspec + - .rails:rules:single-db-ci-connection + rspec-ee system pg12: extends: - .rspec-ee-base-pg12 @@ -636,6 +701,12 @@ rspec-ee system pg12 single-db: - rspec-ee system pg12 - .single-db-rspec - .rails:rules:single-db + +rspec-ee system pg12 single-db-ci-connection: + extends: + - rspec-ee system pg12 + - .single-db-ci-connection-rspec + - .rails:rules:single-db-ci-connection # EE: default refs (MRs, default branch, schedules) jobs # ################################################## diff --git a/.rubocop_todo/layout/line_length.yml b/.rubocop_todo/layout/line_length.yml index f6750d917ec..382b9d29ca8 100644 --- a/.rubocop_todo/layout/line_length.yml +++ b/.rubocop_todo/layout/line_length.yml @@ -5532,7 +5532,6 @@ Layout/LineLength: - 'spec/workers/authorized_project_update/project_recalculate_per_user_worker_spec.rb' - 'spec/workers/authorized_project_update/user_refresh_from_replica_worker_spec.rb' - 'spec/workers/auto_devops/disable_worker_spec.rb' - - 'spec/workers/background_migration/ci_database_worker_spec.rb' - 'spec/workers/build_success_worker_spec.rb' - 'spec/workers/bulk_import_worker_spec.rb' - 'spec/workers/bulk_imports/export_request_worker_spec.rb' diff --git a/.rubocop_todo/lint/unused_block_argument.yml b/.rubocop_todo/lint/unused_block_argument.yml index b9013f03bfa..aa2f0169006 100644 --- a/.rubocop_todo/lint/unused_block_argument.yml +++ b/.rubocop_todo/lint/unused_block_argument.yml @@ -202,7 +202,6 @@ Lint/UnusedBlockArgument: - 'lib/api/helpers/snippets_helpers.rb' - 'lib/api/search.rb' - 'lib/atlassian/jira_connect/serializers/repository_entity.rb' - - 'lib/backup/database.rb' - 'lib/banzai/filter/autolink_filter.rb' - 'lib/banzai/filter/emoji_filter.rb' - 'lib/banzai/filter/inline_metrics_redactor_filter.rb' @@ -260,7 +259,6 @@ Lint/UnusedBlockArgument: - 'lib/tasks/contracts/pipeline_schedules.rake' - 'lib/tasks/contracts/pipelines.rake' - 'lib/tasks/frontend.rake' - - 'lib/tasks/gitlab/background_migrations.rake' - 'lib/tasks/gitlab/bulk_add_permission.rake' - 'lib/tasks/gitlab/db.rake' - 'lib/tasks/gitlab/external_diffs.rake' @@ -332,7 +330,6 @@ Lint/UnusedBlockArgument: - 'spec/graphql/types/base_object_spec.rb' - 'spec/initializers/secret_token_spec.rb' - 'spec/lib/api/helpers/pagination_strategies_spec.rb' - - 'spec/lib/backup/database_spec.rb' - 'spec/lib/banzai/filter/audio_link_filter_spec.rb' - 'spec/lib/banzai/filter/video_link_filter_spec.rb' - 'spec/lib/feature_spec.rb' diff --git a/app/services/users/unblock_service.rb b/app/services/users/unblock_service.rb index 1302395662f..d80f65b5757 100644 --- a/app/services/users/unblock_service.rb +++ b/app/services/users/unblock_service.rb @@ -27,3 +27,5 @@ module Users end end end + +Users::UnblockService.prepend_mod_with('Users::UnblockService') diff --git a/app/views/doorkeeper/applications/edit.html.haml b/app/views/doorkeeper/applications/edit.html.haml index c48e2cd4db0..f1c3ea6aab1 100644 --- a/app/views/doorkeeper/applications/edit.html.haml +++ b/app/views/doorkeeper/applications/edit.html.haml @@ -1,5 +1,4 @@ - page_title _("Edit"), @application.name, _("Applications") -- @content_class = "limit-container-width" unless fluid_layout %h1.page-title.gl-font-size-h-display= _('Edit application') = render 'shared/doorkeeper/applications/form', url: doorkeeper_submit_path(@application) diff --git a/app/views/doorkeeper/applications/show.html.haml b/app/views/doorkeeper/applications/show.html.haml index d087d85a94e..5fc1384f6ee 100644 --- a/app/views/doorkeeper/applications/show.html.haml +++ b/app/views/doorkeeper/applications/show.html.haml @@ -1,7 +1,6 @@ - add_to_breadcrumbs _("Applications"), oauth_applications_path - breadcrumb_title @application.name - page_title @application.name, _("Applications") -- @content_class = "limit-container-width" unless fluid_layout %h1.page-title.gl-font-size-h-display = _("Application: %{name}") % { name: @application.name } diff --git a/doc/development/database/avoiding_downtime_in_migrations.md b/doc/development/database/avoiding_downtime_in_migrations.md index 8e1eeee7a42..25310554c24 100644 --- a/doc/development/database/avoiding_downtime_in_migrations.md +++ b/doc/development/database/avoiding_downtime_in_migrations.md @@ -605,7 +605,7 @@ See example [merge request](https://gitlab.com/gitlab-org/gitlab/-/merge_request ### Remove the trigger and old `integer` columns (release N + 2) Using post-deployment migration and the provided `cleanup_conversion_of_integer_to_bigint` helper, -drop the database trigger and the old `integer` columns ([see an example](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69714)). +drop the database trigger and the old `integer` columns ([see an example](https://gitlab.com/gitlab-org/gitlab/-/merge_requests/70351)). ### Remove ignore rules (release N + 3) diff --git a/lib/gitlab/database.rb b/lib/gitlab/database.rb index 6c4f8bb169f..dbd24684208 100644 --- a/lib/gitlab/database.rb +++ b/lib/gitlab/database.rb @@ -128,6 +128,7 @@ module Gitlab Gitlab::Runtime.max_threads + headroom end + # Database configured. Returns true even if the database is shared def self.has_config?(database_name) ActiveRecord::Base.configurations .configs_for(env_name: Rails.env, name: database_name.to_s, include_replicas: true) diff --git a/lib/tasks/gitlab/background_migrations.rake b/lib/tasks/gitlab/background_migrations.rake index e0699d5eb41..eca51c345d1 100644 --- a/lib/tasks/gitlab/background_migrations.rake +++ b/lib/tasks/gitlab/background_migrations.rake @@ -46,20 +46,22 @@ namespace :gitlab do end desc 'Display the status of batched background migrations' - task status: :environment do |_, args| - Gitlab::Database.database_base_models.each do |name, model| - display_migration_status(name, model.connection) + task status: :environment do |_, _args| + Gitlab::Database.database_base_models.each do |database_name, model| + next unless Gitlab::Database.has_database?(database_name) + + display_migration_status(database_name, model.connection) end end namespace :status do - ActiveRecord::Tasks::DatabaseTasks.for_each(databases) do |name| - next if name.to_s == 'geo' + ActiveRecord::Tasks::DatabaseTasks.for_each(databases) do |database_name| + next if database_name.to_s == 'geo' - desc "Gitlab | DB | Display the status of batched background migrations on #{name} database" - task name => :environment do |_, args| - model = Gitlab::Database.database_base_models[name] - display_migration_status(name, model.connection) + desc "Gitlab | DB | Display the status of batched background migrations on #{database_name} database" + task database_name => :environment do |_, _args| + model = Gitlab::Database.database_base_models[database_name] + display_migration_status(database_name, model.connection) end end end diff --git a/spec/lib/gitlab/database/migration_helpers/automatic_lock_writes_on_tables_spec.rb b/spec/lib/gitlab/database/migration_helpers/automatic_lock_writes_on_tables_spec.rb index be9346e3829..22274ab924d 100644 --- a/spec/lib/gitlab/database/migration_helpers/automatic_lock_writes_on_tables_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers/automatic_lock_writes_on_tables_spec.rb @@ -86,7 +86,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers::AutomaticLockWritesOnTables, let(:create_gitlab_shared_table_migration_class) { create_table_migration(gitlab_shared_table_name) } before do - skip_if_multiple_databases_are_setup(:ci) + skip_if_database_exists(:ci) end it 'does not lock any newly created tables' do @@ -106,7 +106,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers::AutomaticLockWritesOnTables, context 'when multiple databases' do before do - skip_if_multiple_databases_not_setup(:ci) + skip_if_shared_database(:ci) end let(:migration_class) { create_table_migration(table_name, skip_automatic_lock_on_writes) } @@ -238,7 +238,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers::AutomaticLockWritesOnTables, context 'when renaming a table' do before do - skip_if_multiple_databases_not_setup(:ci) + skip_if_shared_database(:ci) create_table_migration(old_table_name).migrate(:up) # create the table first before renaming it end @@ -277,7 +277,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers::AutomaticLockWritesOnTables, let(:config_model) { Gitlab::Database.database_base_models[:main] } before do - skip_if_multiple_databases_are_setup(:ci) + skip_if_database_exists(:ci) end it 'does not lock any newly created tables' do @@ -305,7 +305,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers::AutomaticLockWritesOnTables, context 'when multiple databases' do before do - skip_if_multiple_databases_not_setup(:ci) + skip_if_shared_database(:ci) migration_class.connection.execute("CREATE TABLE #{table_name}()") migration_class.migrate(:up) end diff --git a/spec/lib/gitlab/database/migration_helpers/restrict_gitlab_schema_spec.rb b/spec/lib/gitlab/database/migration_helpers/restrict_gitlab_schema_spec.rb index 714fbab5aff..635661c91f8 100644 --- a/spec/lib/gitlab/database/migration_helpers/restrict_gitlab_schema_spec.rb +++ b/spec/lib/gitlab/database/migration_helpers/restrict_gitlab_schema_spec.rb @@ -506,7 +506,7 @@ RSpec.describe Gitlab::Database::MigrationHelpers::RestrictGitlabSchema, query_a def down; end end, query_matcher: /FROM ci_builds/, - setup: -> (_) { skip_if_multiple_databases_not_setup }, + setup: -> (_) { skip_if_shared_database(:ci) }, expected: { no_gitlab_schema: { main: :cross_schema_error, diff --git a/spec/lib/gitlab/database/migrations/runner_spec.rb b/spec/lib/gitlab/database/migrations/runner_spec.rb index 66eb5a5de51..7c71076e8f3 100644 --- a/spec/lib/gitlab/database/migrations/runner_spec.rb +++ b/spec/lib/gitlab/database/migrations/runner_spec.rb @@ -65,7 +65,7 @@ RSpec.describe Gitlab::Database::Migrations::Runner, :reestablished_active_recor end before do - skip_if_multiple_databases_not_setup unless database == :main + skip_if_shared_database(database) stub_const('Gitlab::Database::Migrations::Runner::BASE_RESULT_DIR', base_result_dir) allow(ActiveRecord::Migrator).to receive(:new) do |dir, _all_migrations, _schema_migration_class, version_to_migrate| diff --git a/spec/lib/gitlab/database/partitioning_spec.rb b/spec/lib/gitlab/database/partitioning_spec.rb index 4c0fde46b2f..4aa9d5f6df0 100644 --- a/spec/lib/gitlab/database/partitioning_spec.rb +++ b/spec/lib/gitlab/database/partitioning_spec.rb @@ -2,11 +2,11 @@ require 'spec_helper' -RSpec.describe Gitlab::Database::Partitioning do +RSpec.describe Gitlab::Database::Partitioning, feature_category: :database do include Database::PartitioningHelpers include Database::TableSchemaHelpers - let(:connection) { ApplicationRecord.connection } + let(:main_connection) { ApplicationRecord.connection } around do |example| previously_registered_models = described_class.registered_models.dup @@ -84,7 +84,7 @@ RSpec.describe Gitlab::Database::Partitioning do before do table_names.each do |table_name| - connection.execute(<<~SQL) + execute_on_each_database(<<~SQL) CREATE TABLE #{table_name} ( id serial not null, created_at timestamptz not null, @@ -101,32 +101,12 @@ RSpec.describe Gitlab::Database::Partitioning do end context 'with multiple databases' do - before do - table_names.each do |table_name| - ci_connection.execute("DROP TABLE IF EXISTS #{table_name}") - - ci_connection.execute(<<~SQL) - CREATE TABLE #{table_name} ( - id serial not null, - created_at timestamptz not null, - PRIMARY KEY (id, created_at)) - PARTITION BY RANGE (created_at); - SQL - end - end - - after do - table_names.each do |table_name| - ci_connection.execute("DROP TABLE IF EXISTS #{table_name}") - end - end - it 'creates partitions in each database' do - skip_if_multiple_databases_not_setup(:ci) + skip_if_shared_database(:ci) expect { described_class.sync_partitions(models) } - .to change { find_partitions(table_names.first, conn: connection).size }.from(0) - .and change { find_partitions(table_names.last, conn: connection).size }.from(0) + .to change { find_partitions(table_names.first, conn: main_connection).size }.from(0) + .and change { find_partitions(table_names.last, conn: main_connection).size }.from(0) .and change { find_partitions(table_names.first, conn: ci_connection).size }.from(0) .and change { find_partitions(table_names.last, conn: ci_connection).size }.from(0) end @@ -161,10 +141,12 @@ RSpec.describe Gitlab::Database::Partitioning do end before do + skip_if_shared_database(:ci) + (table_names + ['partitioning_test3']).each do |table_name| - ci_connection.execute("DROP TABLE IF EXISTS #{table_name}") + execute_on_each_database("DROP TABLE IF EXISTS #{table_name}") - ci_connection.execute(<<~SQL) + execute_on_each_database(<<~SQL) CREATE TABLE #{table_name} ( id serial not null, created_at timestamptz not null, @@ -181,14 +163,12 @@ RSpec.describe Gitlab::Database::Partitioning do end it 'manages partitions for models for the given database', :aggregate_failures do - skip_if_multiple_databases_not_setup(:ci) - expect { described_class.sync_partitions([models.first, ci_model], only_on: 'ci') } .to change { find_partitions(ci_model.table_name, conn: ci_connection).size }.from(0) - expect(find_partitions(models.first.table_name).size).to eq(0) + expect(find_partitions(models.first.table_name, conn: main_connection).size).to eq(0) expect(find_partitions(models.first.table_name, conn: ci_connection).size).to eq(0) - expect(find_partitions(ci_model.table_name).size).to eq(0) + expect(find_partitions(ci_model.table_name, conn: main_connection).size).to eq(0) end end end 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 d31be6cb883..35ba7134df9 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 @@ -28,19 +28,19 @@ RSpec.describe Gitlab::Database::QueryAnalyzers::GitlabSchemasValidateConnection 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"\]/, - setup: -> (_) { skip_if_multiple_databases_not_setup(:ci) } + 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"\]/, - setup: -> (_) { skip_if_multiple_databases_not_setup(:ci) } + setup: -> (_) { skip_if_shared_database(:ci) } }, "for query accessing main table from CI database" => { model: Ci::ApplicationRecord, sql: "SELECT 1 FROM projects", expect_error: /The query tried to access \["projects"\]/, - setup: -> (_) { skip_if_multiple_databases_not_setup(:ci) } + setup: -> (_) { skip_if_shared_database(:ci) } }, "for query accessing CI database" => { model: Ci::ApplicationRecord, @@ -51,13 +51,13 @@ RSpec.describe Gitlab::Database::QueryAnalyzers::GitlabSchemasValidateConnection model: ::ApplicationRecord, sql: "SELECT 1 FROM ci_builds", expect_error: /The query tried to access \["ci_builds"\]/, - setup: -> (_) { skip_if_multiple_databases_not_setup(:ci) } + setup: -> (_) { skip_if_shared_database(:ci) } }, "for query accessing unknown gitlab_schema" => { model: ::ApplicationRecord, sql: "SELECT 1 FROM new_table", expect_error: /The query tried to access \["new_table"\] \(of undefined_new_table\)/, - setup: -> (_) { skip_if_multiple_databases_not_setup(:ci) } + setup: -> (_) { skip_if_shared_database(:ci) } } } end @@ -77,7 +77,7 @@ RSpec.describe Gitlab::Database::QueryAnalyzers::GitlabSchemasValidateConnection context "when analyzer is enabled for tests", :query_analyzers do before do - skip_if_multiple_databases_not_setup(:ci) + skip_if_shared_database(:ci) end it "throws an error when trying to access a table that belongs to the gitlab_main schema from the ci database" do diff --git a/spec/lib/gitlab/database/tables_locker_spec.rb b/spec/lib/gitlab/database/tables_locker_spec.rb index d0ba5c742c0..ef634f85b20 100644 --- a/spec/lib/gitlab/database/tables_locker_spec.rb +++ b/spec/lib/gitlab/database/tables_locker_spec.rb @@ -166,7 +166,7 @@ RSpec.describe Gitlab::Database::TablesLocker, :suppress_gitlab_schemas_validate context 'when running on single database' do before do - skip_if_multiple_databases_are_setup(:ci) + skip_if_database_exists(:ci) end describe '#lock_writes' do @@ -203,7 +203,7 @@ RSpec.describe Gitlab::Database::TablesLocker, :suppress_gitlab_schemas_validate context 'when running on multiple databases' do before do - skip_if_multiple_databases_not_setup(:ci) + skip_if_shared_database(:ci) end describe '#lock_writes' do diff --git a/spec/lib/gitlab/database/tables_truncate_spec.rb b/spec/lib/gitlab/database/tables_truncate_spec.rb index 3bb2f4e982c..66216d16ce8 100644 --- a/spec/lib/gitlab/database/tables_truncate_spec.rb +++ b/spec/lib/gitlab/database/tables_truncate_spec.rb @@ -48,7 +48,7 @@ RSpec.describe Gitlab::Database::TablesTruncate, :reestablished_active_record_ba end before do - skip_if_multiple_databases_not_setup(:ci) + skip_if_shared_database(:ci) # Creating some test tables on the main database main_tables_sql = <<~SQL @@ -79,8 +79,7 @@ RSpec.describe Gitlab::Database::TablesTruncate, :reestablished_active_record_ba ALTER TABLE _test_gitlab_hook_logs DETACH PARTITION gitlab_partitions_dynamic._test_gitlab_hook_logs_202201; SQL - main_connection.execute(main_tables_sql) - ci_connection.execute(main_tables_sql) + execute_on_each_database(main_tables_sql) ci_tables_sql = <<~SQL CREATE TABLE _test_gitlab_ci_items (id serial NOT NULL PRIMARY KEY); @@ -92,15 +91,13 @@ RSpec.describe Gitlab::Database::TablesTruncate, :reestablished_active_record_ba ); SQL - main_connection.execute(ci_tables_sql) - ci_connection.execute(ci_tables_sql) + execute_on_each_database(ci_tables_sql) internal_tables_sql = <<~SQL CREATE TABLE _test_gitlab_shared_items (id serial NOT NULL PRIMARY KEY); SQL - main_connection.execute(internal_tables_sql) - ci_connection.execute(internal_tables_sql) + execute_on_each_database(internal_tables_sql) # Filling the tables 5.times do |i| @@ -314,8 +311,7 @@ RSpec.describe Gitlab::Database::TablesTruncate, :reestablished_active_record_ba context 'when running with multiple shared databases' do before do skip_if_multiple_databases_not_setup(:ci) - ci_db_config = Ci::ApplicationRecord.connection_db_config - allow(::Gitlab::Database).to receive(:db_config_share_with).with(ci_db_config).and_return('main') + skip_if_database_exists(:ci) end it 'raises an error when truncating the main database that it is a single database setup' do diff --git a/spec/support/helpers/database/multiple_databases_helpers.rb b/spec/support/helpers/database/multiple_databases_helpers.rb index bd06895bb9a..3c9a5762c47 100644 --- a/spec/support/helpers/database/multiple_databases_helpers.rb +++ b/spec/support/helpers/database/multiple_databases_helpers.rb @@ -16,6 +16,16 @@ module Database skip "Skipping because database #{database_name} exists" if database_exists?(database_name) end + def execute_on_each_database(query, databases: %I[main ci]) + databases = databases.select { |database_name| database_exists?(database_name) } + + Gitlab::Database::EachDatabase.each_database_connection(only: databases, include_shared: false) do |connection, _| + next unless Gitlab::Database.gitlab_schemas_for_connection(connection).include?(:gitlab_shared) + + connection.execute(query) + end + end + def skip_if_multiple_databases_not_setup(*databases) unless (databases - EXTRA_DBS).empty? raise "Unsupported database in #{databases}. It must be one of #{EXTRA_DBS}." diff --git a/spec/support/shared_examples/workers/batched_background_migration_worker_shared_examples.rb b/spec/support/shared_examples/workers/batched_background_migration_worker_shared_examples.rb index 8ec955940c0..06877aee565 100644 --- a/spec/support/shared_examples/workers/batched_background_migration_worker_shared_examples.rb +++ b/spec/support/shared_examples/workers/batched_background_migration_worker_shared_examples.rb @@ -88,9 +88,9 @@ RSpec.shared_examples 'it runs batched background migration jobs' do |tracking_d end end - context 'when the feature flag is disabled' do + context 'when the tracking database is shared' do before do - stub_feature_flags(execute_batched_migrations_on_schedule: false) + skip_if_database_exists(tracking_database) end it 'does nothing' do @@ -101,22 +101,17 @@ RSpec.shared_examples 'it runs batched background migration jobs' do |tracking_d end end - context 'when the feature flag is enabled' do - let(:base_model) { Gitlab::Database.database_base_models[tracking_database] } - + context 'when the tracking database is not shared' do before do - stub_feature_flags(execute_batched_migrations_on_schedule: true) - - allow(Gitlab::Database::BackgroundMigration::BatchedMigration).to receive(:active_migration) - .with(connection: base_model.connection) - .and_return(nil) + skip_if_shared_database(tracking_database) end - context 'when database config is shared' do - it 'does nothing' do - expect(Gitlab::Database).to receive(:db_config_share_with) - .with(base_model.connection_db_config).and_return('main') + context 'when the feature flag is disabled' do + before do + stub_feature_flags(execute_batched_migrations_on_schedule: false) + end + it 'does nothing' do expect(worker).not_to receive(:active_migration) expect(worker).not_to receive(:run_active_migration) @@ -124,123 +119,146 @@ RSpec.shared_examples 'it runs batched background migration jobs' do |tracking_d end end - context 'when no active migrations exist' do - context 'when parallel execution is disabled' do - before do - stub_feature_flags(batched_migrations_parallel_execution: false) - end + context 'when the feature flag is enabled' do + let(:base_model) { Gitlab::Database.database_base_models[tracking_database] } + let(:connection) { base_model.connection } + + before do + stub_feature_flags(execute_batched_migrations_on_schedule: true) + allow(Gitlab::Database::BackgroundMigration::BatchedMigration).to receive(:active_migration) + .with(connection: connection) + .and_return(nil) + end + + context 'when database config is shared' do it 'does nothing' do + expect(Gitlab::Database).to receive(:db_config_share_with) + .with(base_model.connection_db_config).and_return('main') + + expect(worker).not_to receive(:active_migration) expect(worker).not_to receive(:run_active_migration) worker.perform end end - context 'when parallel execution is enabled' do - before do - stub_feature_flags(batched_migrations_parallel_execution: true) - end + context 'when no active migrations exist' do + context 'when parallel execution is disabled' do + before do + stub_feature_flags(batched_migrations_parallel_execution: false) + end - it 'does nothing' do - expect(worker).not_to receive(:queue_migrations_for_execution) + it 'does nothing' do + expect(worker).not_to receive(:run_active_migration) - worker.perform + worker.perform + end end - end - end - context 'when active migrations exist' do - let(:job_interval) { 5.minutes } - let(:lease_timeout) { 15.minutes } - let(:lease_key) { described_class.name.demodulize.underscore } - let(:migration_id) { 123 } - let(:migration) do - build( - :batched_background_migration, :active, - id: migration_id, interval: job_interval, table_name: table_name - ) - end + context 'when parallel execution is enabled' do + before do + stub_feature_flags(batched_migrations_parallel_execution: true) + end - let(:execution_worker_class) do - case tracking_database - when :main - Database::BatchedBackgroundMigration::MainExecutionWorker - when :ci - Database::BatchedBackgroundMigration::CiExecutionWorker + it 'does nothing' do + expect(worker).not_to receive(:queue_migrations_for_execution) + + worker.perform + end end end - before do - allow(Gitlab::Database::BackgroundMigration::BatchedMigration).to receive(:active_migration) - .with(connection: base_model.connection) - .and_return(migration) - end + context 'when active migrations exist' do + let(:job_interval) { 5.minutes } + let(:lease_timeout) { 15.minutes } + let(:lease_key) { described_class.name.demodulize.underscore } + let(:migration_id) { 123 } + let(:migration) do + build( + :batched_background_migration, :active, + id: migration_id, interval: job_interval, table_name: table_name + ) + end + + let(:execution_worker_class) do + case tracking_database + when :main + Database::BatchedBackgroundMigration::MainExecutionWorker + when :ci + Database::BatchedBackgroundMigration::CiExecutionWorker + end + end - context 'when parallel execution is disabled' do before do - stub_feature_flags(batched_migrations_parallel_execution: false) + allow(Gitlab::Database::BackgroundMigration::BatchedMigration).to receive(:active_migration) + .with(connection: connection) + .and_return(migration) end - let(:execution_worker) { instance_double(execution_worker_class) } + context 'when parallel execution is disabled' do + before do + stub_feature_flags(batched_migrations_parallel_execution: false) + end - context 'when the calculated timeout is less than the minimum allowed' do - let(:minimum_timeout) { described_class::MINIMUM_LEASE_TIMEOUT } - let(:job_interval) { 2.minutes } + let(:execution_worker) { instance_double(execution_worker_class) } - it 'sets the lease timeout to the minimum value' do - expect_to_obtain_exclusive_lease(lease_key, timeout: minimum_timeout) + context 'when the calculated timeout is less than the minimum allowed' do + let(:minimum_timeout) { described_class::MINIMUM_LEASE_TIMEOUT } + let(:job_interval) { 2.minutes } - expect(execution_worker_class).to receive(:new).and_return(execution_worker) - expect(execution_worker).to receive(:perform_work).with(tracking_database, migration_id) + it 'sets the lease timeout to the minimum value' do + expect_to_obtain_exclusive_lease(lease_key, timeout: minimum_timeout) - expect(worker).to receive(:run_active_migration).and_call_original + expect(execution_worker_class).to receive(:new).and_return(execution_worker) + expect(execution_worker).to receive(:perform_work).with(tracking_database, migration_id) - worker.perform - end - end + expect(worker).to receive(:run_active_migration).and_call_original - it 'always cleans up the exclusive lease' do - lease = stub_exclusive_lease_taken(lease_key, timeout: lease_timeout) + worker.perform + end + end - expect(lease).to receive(:try_obtain).and_return(true) + it 'always cleans up the exclusive lease' do + lease = stub_exclusive_lease_taken(lease_key, timeout: lease_timeout) - expect(worker).to receive(:run_active_migration).and_raise(RuntimeError, 'I broke') - expect(lease).to receive(:cancel) + expect(lease).to receive(:try_obtain).and_return(true) - expect { worker.perform }.to raise_error(RuntimeError, 'I broke') - end + expect(worker).to receive(:run_active_migration).and_raise(RuntimeError, 'I broke') + expect(lease).to receive(:cancel) - it 'delegetes the execution to ExecutionWorker' do - base_model = Gitlab::Database.database_base_models[tracking_database] + expect { worker.perform }.to raise_error(RuntimeError, 'I broke') + end - expect(Gitlab::Database::SharedModel).to receive(:using_connection).with(base_model.connection).and_yield - expect(execution_worker_class).to receive(:new).and_return(execution_worker) - expect(execution_worker).to receive(:perform_work).with(tracking_database, migration_id) + it 'delegetes the execution to ExecutionWorker' do + expect(Gitlab::Database::SharedModel).to receive(:using_connection).with(connection).and_yield + expect(execution_worker_class).to receive(:new).and_return(execution_worker) + expect(execution_worker).to receive(:perform_work).with(tracking_database, migration_id) - worker.perform + worker.perform + end end - end - context 'when parallel execution is enabled' do - before do - stub_feature_flags(batched_migrations_parallel_execution: true) - end + context 'when parallel execution is enabled' do + before do + stub_feature_flags(batched_migrations_parallel_execution: true) + end - it 'delegetes the execution to ExecutionWorker' do - expect(Gitlab::Database::BackgroundMigration::BatchedMigration) - .to receive(:active_migrations_distinct_on_table).with( - connection: base_model.connection, - limit: execution_worker_class.max_running_jobs - ).and_return([migration]) + it 'delegetes the execution to ExecutionWorker' do + expect(Gitlab::Database::BackgroundMigration::BatchedMigration) + .to receive(:active_migrations_distinct_on_table).with( + connection: base_model.connection, + limit: execution_worker_class.max_running_jobs + ).and_return([migration]) - expected_arguments = [ - [tracking_database.to_s, migration_id] - ] + expected_arguments = [ + [tracking_database.to_s, migration_id] + ] - expect(execution_worker_class).to receive(:perform_with_capacity).with(expected_arguments) + expect(execution_worker_class).to receive(:perform_with_capacity).with(expected_arguments) - worker.perform + worker.perform + end end end end @@ -248,7 +266,7 @@ RSpec.shared_examples 'it runs batched background migration jobs' do |tracking_d end end - describe 'executing an entire migration', :freeze_time, if: Gitlab::Database.has_config?(tracking_database) do + describe 'executing an entire migration', :freeze_time, if: Gitlab::Database.has_database?(tracking_database) do include Gitlab::Database::DynamicModelHelpers include Database::DatabaseHelpers diff --git a/spec/tasks/dev_rake_spec.rb b/spec/tasks/dev_rake_spec.rb index ef047b383a6..82c9bb4faa2 100644 --- a/spec/tasks/dev_rake_spec.rb +++ b/spec/tasks/dev_rake_spec.rb @@ -121,7 +121,7 @@ RSpec.describe 'dev rake tasks' do context 'when a database is not found' do before do - skip_if_multiple_databases_not_setup + skip_if_shared_database(:ci) end it 'continues to next connection' do @@ -135,7 +135,7 @@ RSpec.describe 'dev rake tasks' do context 'multiple databases' do before do - skip_if_multiple_databases_not_setup(:ci) + skip_if_shared_database(:ci) end context 'with a valid database' do diff --git a/spec/tasks/gitlab/background_migrations_rake_spec.rb b/spec/tasks/gitlab/background_migrations_rake_spec.rb index 876b56d1208..04be713e0d4 100644 --- a/spec/tasks/gitlab/background_migrations_rake_spec.rb +++ b/spec/tasks/gitlab/background_migrations_rake_spec.rb @@ -2,7 +2,8 @@ require 'rake_helper' -RSpec.describe 'gitlab:background_migrations namespace rake tasks', :suppress_gitlab_schemas_validate_connection do +RSpec.describe 'gitlab:background_migrations namespace rake tasks', :suppress_gitlab_schemas_validate_connection, + feature_category: :database do before do Rake.application.rake_require 'tasks/gitlab/background_migrations' end @@ -62,7 +63,7 @@ RSpec.describe 'gitlab:background_migrations namespace rake tasks', :suppress_gi let(:databases) { [Gitlab::Database::MAIN_DATABASE_NAME, ci_database_name] } before do - skip_if_multiple_databases_not_setup(:ci) + skip_if_shared_database(:ci) allow(Gitlab::Database).to receive(:database_base_models).and_return(base_models) end @@ -114,12 +115,6 @@ RSpec.describe 'gitlab:background_migrations namespace rake tasks', :suppress_gi let(:connection) { double(:connection) } let(:base_models) { { 'main' => model }.with_indifferent_access } - around do |example| - Gitlab::Database::SharedModel.using_connection(model.connection) do - example.run - end - end - it 'outputs the status of background migrations' do allow(Gitlab::Database).to receive(:database_base_models).and_return(base_models) @@ -130,15 +125,33 @@ RSpec.describe 'gitlab:background_migrations namespace rake tasks', :suppress_gi OUTPUT end - context 'when multiple database feature is enabled' do + context 'when running the rake task against one database in multiple databases setup' do before do - skip_if_multiple_databases_not_setup + skip_if_shared_database(:ci) end - context 'with a single database' do - subject(:status_task) { run_rake_task("gitlab:background_migrations:status:#{main_database_name}") } + subject(:status_task) { run_rake_task("gitlab:background_migrations:status:#{main_database_name}") } - it 'outputs the status of background migrations' do + it 'outputs the status of background migrations' do + expect { status_task }.to output(<<~OUTPUT).to_stdout + Database: #{main_database_name} + finished | #{migration1.job_class_name},#{migration1.table_name},#{migration1.column_name},[["id1","id2"]] + failed | #{migration2.job_class_name},#{migration2.table_name},#{migration2.column_name},[] + OUTPUT + end + end + + context 'when multiple databases are configured' do + before do + skip_if_multiple_databases_not_setup(:ci) + end + + context 'with two connections sharing the same database' do + before do + skip_if_database_exists(:ci) + end + + it 'skips the shared database' do expect { status_task }.to output(<<~OUTPUT).to_stdout Database: #{main_database_name} finished | #{migration1.job_class_name},#{migration1.table_name},#{migration1.column_name},[["id1","id2"]] @@ -153,6 +166,10 @@ RSpec.describe 'gitlab:background_migrations namespace rake tasks', :suppress_gi end context 'with multiple databases' do + before do + skip_if_shared_database(:ci) + end + subject(:status_task) { run_rake_task('gitlab:background_migrations:status') } let(:base_models) { { main: main_model, ci: ci_model } } @@ -161,6 +178,8 @@ RSpec.describe 'gitlab:background_migrations namespace rake tasks', :suppress_gi it 'outputs the status for each database' do allow(Gitlab::Database).to receive(:database_base_models).and_return(base_models) + allow(Gitlab::Database).to receive(:has_database?).with(:main).and_return(true) + allow(Gitlab::Database).to receive(:has_database?).with(:ci).and_return(true) expect(Gitlab::Database::SharedModel).to receive(:using_connection).with(main_model.connection).and_yield expect(Gitlab::Database::BackgroundMigration::BatchedMigration).to receive(:find_each).and_yield(migration1) diff --git a/spec/tasks/gitlab/db/decomposition/rollback/bump_ci_sequences_rake_spec.rb b/spec/tasks/gitlab/db/decomposition/rollback/bump_ci_sequences_rake_spec.rb index 0682a4b39cf..11c3b494f0d 100644 --- a/spec/tasks/gitlab/db/decomposition/rollback/bump_ci_sequences_rake_spec.rb +++ b/spec/tasks/gitlab/db/decomposition/rollback/bump_ci_sequences_rake_spec.rb @@ -86,7 +86,7 @@ RSpec.describe 'gitlab:db:decomposition:rollback:bump_ci_sequences', :silence_st context 'when multiple databases' do before do - skip_if_multiple_databases_not_setup(:ci) + skip_if_shared_database(:ci) end it 'does not change ci sequences on the ci database' do diff --git a/spec/tasks/gitlab/db/truncate_legacy_tables_rake_spec.rb b/spec/tasks/gitlab/db/truncate_legacy_tables_rake_spec.rb index 6e245b6f227..cd35af24fe8 100644 --- a/spec/tasks/gitlab/db/truncate_legacy_tables_rake_spec.rb +++ b/spec/tasks/gitlab/db/truncate_legacy_tables_rake_spec.rb @@ -20,19 +20,16 @@ RSpec.describe 'gitlab:db:truncate_legacy_tables', :silence_stdout, :reestablish end before do - skip_if_multiple_databases_not_setup(:ci) - - # Filling the table on both databases main and ci - Gitlab::Database.database_base_models.each_value do |base_model| - base_model.connection.execute(<<~SQL) - CREATE TABLE #{test_gitlab_main_table} (id integer NOT NULL); - INSERT INTO #{test_gitlab_main_table} VALUES(generate_series(1, 50)); - SQL - base_model.connection.execute(<<~SQL) - CREATE TABLE #{test_gitlab_ci_table} (id integer NOT NULL); - INSERT INTO #{test_gitlab_ci_table} VALUES(generate_series(1, 50)); - SQL - end + skip_if_shared_database(:ci) + + execute_on_each_database(<<~SQL) + CREATE TABLE #{test_gitlab_main_table} (id integer NOT NULL); + INSERT INTO #{test_gitlab_main_table} VALUES(generate_series(1, 50)); + SQL + execute_on_each_database(<<~SQL) + CREATE TABLE #{test_gitlab_ci_table} (id integer NOT NULL); + INSERT INTO #{test_gitlab_ci_table} VALUES(generate_series(1, 50)); + SQL allow(Gitlab::Database::GitlabSchema).to receive(:tables_to_schema).and_return( { diff --git a/spec/tasks/gitlab/db_rake_spec.rb b/spec/tasks/gitlab/db_rake_spec.rb index 3b842accaaa..45d0c050949 100644 --- a/spec/tasks/gitlab/db_rake_spec.rb +++ b/spec/tasks/gitlab/db_rake_spec.rb @@ -25,7 +25,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor let(:main_model) { ApplicationRecord } before do - skip_if_multiple_databases_are_setup + skip_if_database_exists(:ci) end it 'marks the migration complete on the given database' do @@ -43,7 +43,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor let(:base_models) { { 'main' => main_model, 'ci' => ci_model } } before do - skip_unless_ci_uses_database_tasks + skip_if_shared_database(:ci) allow(Gitlab::Database).to receive(:database_base_models_with_gitlab_shared).and_return(base_models) end @@ -130,7 +130,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor let(:main_config) { double(:config, name: 'main') } before do - skip_if_multiple_databases_are_setup + skip_if_database_exists(:ci) end context 'when geo is not configured' do @@ -259,7 +259,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor let(:ci_config) { double(:config, name: 'ci') } before do - skip_unless_ci_uses_database_tasks + skip_if_shared_database(:ci) allow(Gitlab::Database).to receive(:database_base_models_with_gitlab_shared).and_return(base_models) end @@ -615,7 +615,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor let(:base_models) { { 'main' => main_model, 'ci' => ci_model } } before do - skip_unless_ci_uses_database_tasks + skip_if_shared_database(:ci) allow(Gitlab::Database).to receive(:database_base_models_with_gitlab_shared).and_return(base_models) @@ -687,7 +687,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor context 'with multiple databases' do before do - skip_unless_ci_uses_database_tasks + skip_if_shared_database(:ci) end context 'when running the multi-database variant' do @@ -722,7 +722,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor describe 'reindex' do context 'with a single database' do before do - skip_if_multiple_databases_are_setup + skip_if_shared_database(:ci) end it 'delegates to Gitlab::Database::Reindexing' do @@ -758,7 +758,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor context 'when the single database task is used' do before do - skip_unless_ci_uses_database_tasks + skip_if_shared_database(:ci) end it 'delegates to Gitlab::Database::Reindexing with a specific database' do @@ -810,7 +810,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor describe 'execute_async_index_operations' do before do - skip_if_multiple_databases_not_setup + skip_if_shared_database(:ci) end it 'delegates ci task to Gitlab::Database::AsyncIndexes' do @@ -884,7 +884,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor describe 'validate_async_constraints' do before do - skip_if_multiple_databases_not_setup + skip_if_shared_database(:ci) end it 'delegates ci task to Gitlab::Database::AsyncConstraints' do @@ -1123,7 +1123,7 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor context 'with multiple databases', :reestablished_active_record_base do before do - skip_unless_ci_uses_database_tasks + skip_if_shared_database(:ci) end describe 'db:schema:dump against a single database' do @@ -1205,14 +1205,6 @@ RSpec.describe 'gitlab:db namespace rake task', :silence_stdout, feature_categor run_rake_task(test_task_name) end - def skip_unless_ci_uses_database_tasks - skip "Skipping because database tasks won't run against the ci database" unless ci_database_tasks? - end - - def ci_database_tasks? - !!ActiveRecord::Base.configurations.configs_for(env_name: Rails.env, name: 'ci')&.database_tasks? - end - def skip_unless_geo_configured skip 'Skipping because the geo database is not configured' unless geo_configured? end diff --git a/spec/workers/background_migration/ci_database_worker_spec.rb b/spec/workers/background_migration/ci_database_worker_spec.rb index 1048a06bb12..3f2977a0aaa 100644 --- a/spec/workers/background_migration/ci_database_worker_spec.rb +++ b/spec/workers/background_migration/ci_database_worker_spec.rb @@ -2,6 +2,10 @@ require 'spec_helper' -RSpec.describe BackgroundMigration::CiDatabaseWorker, :clean_gitlab_redis_shared_state, if: Gitlab::Database.has_config?(:ci), feature_category: :database do +RSpec.describe BackgroundMigration::CiDatabaseWorker, :clean_gitlab_redis_shared_state, feature_category: :database do + before do + skip_if_shared_database(:ci) + end + it_behaves_like 'it runs background migration jobs', 'ci' end diff --git a/spec/workers/database/batched_background_migration/ci_database_worker_spec.rb b/spec/workers/database/batched_background_migration/ci_database_worker_spec.rb index 91ba6e5a20a..782f949eacf 100644 --- a/spec/workers/database/batched_background_migration/ci_database_worker_spec.rb +++ b/spec/workers/database/batched_background_migration/ci_database_worker_spec.rb @@ -2,6 +2,7 @@ require 'spec_helper' -RSpec.describe Database::BatchedBackgroundMigration::CiDatabaseWorker, :clean_gitlab_redis_shared_state, feature_category: :database do +RSpec.describe Database::BatchedBackgroundMigration::CiDatabaseWorker, :clean_gitlab_redis_shared_state, + feature_category: :database do it_behaves_like 'it runs batched background migration jobs', :ci, :ci_builds end |