diff --git a/db/docs/batched_background_migrations/backfill_issues_dates_with_work_item_dates_sources.yml b/db/docs/batched_background_migrations/backfill_issues_dates_with_work_item_dates_sources.yml new file mode 100644 index 0000000000000000000000000000000000000000..79e8cf068a527e47ed2a33c3d4b759560b792c6e --- /dev/null +++ b/db/docs/batched_background_migrations/backfill_issues_dates_with_work_item_dates_sources.yml @@ -0,0 +1,8 @@ +--- +migration_job_name: BackfillIssuesDatesWithWorkItemDatesSources +description: Backfill issues dates with data from work_item_dates_sources table +feature_category: product_planning +introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/157993 +milestone: '17.2' +queued_migration_version: 20240702212015 +finalize_after: '2024-08-05' diff --git a/db/migrate/20240701153843_add_work_items_dates_sources_sync_to_issues_trigger.rb b/db/migrate/20240701153843_add_work_items_dates_sources_sync_to_issues_trigger.rb new file mode 100644 index 0000000000000000000000000000000000000000..6e6b8703f824f568815e692b7ce0200bffb8775e --- /dev/null +++ b/db/migrate/20240701153843_add_work_items_dates_sources_sync_to_issues_trigger.rb @@ -0,0 +1,42 @@ +# frozen_string_literal: true + +class AddWorkItemsDatesSourcesSyncToIssuesTrigger < Gitlab::Database::Migration[2.2] + milestone '17.2' + + include Gitlab::Database::SchemaHelpers + + enable_lock_retries! + + WORK_ITEM_DATES_SOURCE_TABLE_NAME = 'work_item_dates_sources' + + TRIGGER_FUNCTION_NAME = 'sync_issues_dates_with_work_item_dates_sources' + TRIGGER_NAME = "trigger_#{TRIGGER_FUNCTION_NAME}" + + def up + create_trigger_function(TRIGGER_FUNCTION_NAME, replace: true) do + <<~SQL + UPDATE + issues + SET + start_date = NEW.start_date, + due_date = NEW.due_date + WHERE + issues.id = NEW.issue_id; + + RETURN NULL; + SQL + end + + create_trigger( + WORK_ITEM_DATES_SOURCE_TABLE_NAME, + TRIGGER_NAME, + TRIGGER_FUNCTION_NAME, + fires: "AFTER INSERT OR UPDATE OF start_date, due_date" + ) + end + + def down + drop_trigger(WORK_ITEM_DATES_SOURCE_TABLE_NAME, TRIGGER_NAME) + drop_function(TRIGGER_FUNCTION_NAME) + end +end diff --git a/db/post_migrate/20240702212015_queue_backfill_issues_dates_with_work_item_dates_sources.rb b/db/post_migrate/20240702212015_queue_backfill_issues_dates_with_work_item_dates_sources.rb new file mode 100644 index 0000000000000000000000000000000000000000..82379aad76a1274fa4d54431336f053c0d654b9c --- /dev/null +++ b/db/post_migrate/20240702212015_queue_backfill_issues_dates_with_work_item_dates_sources.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class QueueBackfillIssuesDatesWithWorkItemDatesSources < Gitlab::Database::Migration[2.2] + milestone '17.2' + + restrict_gitlab_migration gitlab_schema: :gitlab_main + + MIGRATION = "BackfillIssuesDatesWithWorkItemDatesSources" + DELAY_INTERVAL = 2.minutes + BATCH_SIZE = 100 + + def up + queue_batched_background_migration( + MIGRATION, + :work_item_dates_sources, + :issue_id, + job_interval: DELAY_INTERVAL, + batch_size: BATCH_SIZE + ) + end + + def down + delete_batched_background_migration(MIGRATION, :work_item_dates_sources, :issue_id, []) + end +end diff --git a/db/schema_migrations/20240701153843 b/db/schema_migrations/20240701153843 new file mode 100644 index 0000000000000000000000000000000000000000..43cadadd7139821d0affcb2648ae26c7dc326620 --- /dev/null +++ b/db/schema_migrations/20240701153843 @@ -0,0 +1 @@ +a86234cca3400e87012ea211d8445e114520d06f7fc79c4dcb2baf7e59415ff5 \ No newline at end of file diff --git a/db/schema_migrations/20240702212015 b/db/schema_migrations/20240702212015 new file mode 100644 index 0000000000000000000000000000000000000000..e0dec1e55a75ff5c878b5a4046e150cb530de485 --- /dev/null +++ b/db/schema_migrations/20240702212015 @@ -0,0 +1 @@ +e818fec9c1bb9cb7b0f9e71a7d970da48bf10ec9bc26f384abfccfd90a39a7d0 \ No newline at end of file diff --git a/db/structure.sql b/db/structure.sql index d4a539057d02a3ad572254034f2ebb4dcc26e86f..3355dfaaa1ca8e336aa0e65296cc2d865e9eb76a 100644 --- a/db/structure.sql +++ b/db/structure.sql @@ -613,6 +613,23 @@ RETURN NULL; END $$; +CREATE FUNCTION sync_issues_dates_with_work_item_dates_sources() RETURNS trigger + LANGUAGE plpgsql + AS $$ +BEGIN +UPDATE + issues +SET + start_date = NEW.start_date, + due_date = NEW.due_date +WHERE + issues.id = NEW.issue_id; + +RETURN NULL; + +END +$$; + CREATE FUNCTION table_sync_function_0992e728d3() RETURNS trigger LANGUAGE plpgsql AS $$ @@ -31735,6 +31752,8 @@ CREATE TRIGGER trigger_projects_parent_id_on_insert AFTER INSERT ON projects FOR CREATE TRIGGER trigger_projects_parent_id_on_update AFTER UPDATE ON projects FOR EACH ROW WHEN ((old.namespace_id IS DISTINCT FROM new.namespace_id)) EXECUTE FUNCTION insert_projects_sync_event(); +CREATE TRIGGER trigger_sync_issues_dates_with_work_item_dates_sources AFTER INSERT OR UPDATE OF start_date, due_date ON work_item_dates_sources FOR EACH ROW EXECUTE FUNCTION sync_issues_dates_with_work_item_dates_sources(); + CREATE TRIGGER trigger_update_details_on_namespace_insert AFTER INSERT ON namespaces FOR EACH ROW WHEN (((new.type)::text <> 'Project'::text)) EXECUTE FUNCTION update_namespace_details_from_namespaces(); CREATE TRIGGER trigger_update_details_on_namespace_update AFTER UPDATE ON namespaces FOR EACH ROW WHEN ((((new.type)::text <> 'Project'::text) AND (((old.description)::text IS DISTINCT FROM (new.description)::text) OR (old.description_html IS DISTINCT FROM new.description_html) OR (old.cached_markdown_version IS DISTINCT FROM new.cached_markdown_version)))) EXECUTE FUNCTION update_namespace_details_from_namespaces(); diff --git a/lib/gitlab/background_migration/backfill_issues_dates_with_work_item_dates_sources.rb b/lib/gitlab/background_migration/backfill_issues_dates_with_work_item_dates_sources.rb new file mode 100644 index 0000000000000000000000000000000000000000..c38db53c1e101d4f1c989839dfa3ab60e9fdc86a --- /dev/null +++ b/lib/gitlab/background_migration/backfill_issues_dates_with_work_item_dates_sources.rb @@ -0,0 +1,32 @@ +# frozen_string_literal: true + +module Gitlab + module BackgroundMigration + class BackfillIssuesDatesWithWorkItemDatesSources < BatchedMigrationJob + operation_name :backfill_issues_dates_with_work_item_dates_source + feature_category :team_planning + + def perform + each_sub_batch do |sub_batch| + inner_scope = sub_batch.select(:start_date, :due_date, :issue_id) + + define_batchable_model('issues', connection: ApplicationRecord.connection).connection.execute <<~SQL + WITH work_item_dates_sources_date_values AS MATERIALIZED ( + #{inner_scope.to_sql} + ) + UPDATE issues + SET + start_date = work_item_dates_sources_date_values.start_date, + due_date = work_item_dates_sources_date_values.due_date + FROM + work_item_dates_sources_date_values + WHERE + work_item_dates_sources_date_values.issue_id = issues.id + AND work_item_dates_sources_date_values.start_date IS DISTINCT FROM issues.start_date + AND work_item_dates_sources_date_values.due_date IS DISTINCT FROM issues.due_date + SQL + end + end + end + end +end diff --git a/spec/lib/gitlab/background_migration/backfill_issues_dates_with_work_item_dates_sources_spec.rb b/spec/lib/gitlab/background_migration/backfill_issues_dates_with_work_item_dates_sources_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..fdb347e64c82e2afb8478cf5dc45f16e257c5ec9 --- /dev/null +++ b/spec/lib/gitlab/background_migration/backfill_issues_dates_with_work_item_dates_sources_spec.rb @@ -0,0 +1,81 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Gitlab::BackgroundMigration::BackfillIssuesDatesWithWorkItemDatesSources, + feature_category: :team_planning do + let!(:namespace) { table(:namespaces).create!(name: 'my test group1', path: 'my-test-group1') } + let!(:author) { table(:users).create!(username: 'tester', projects_limit: 100) } + let!(:epic_type_id) { table(:work_item_types).find_by(base_type: 7).id } + + let!(:issue_1) { work_items(iid: 1) } + let!(:issue_2) { work_items(iid: 2) } + let!(:unassociated_issue) { work_items(iid: 3) } + + let!(:date_source_1) do + dates_source( + work_item: issue_1, + start_date: 1.day.ago, + due_date: 1.day.from_now + ) + end + + let!(:date_source_2) do + dates_source( + work_item: issue_2, + start_date: 2.days.ago, + due_date: 2.days.from_now + ) + end + + subject(:migration) do + described_class.new( + start_id: WorkItems::DatesSource.minimum(:issue_id), + end_id: WorkItems::DatesSource.maximum(:issue_id), + batch_table: :work_item_dates_sources, + batch_column: :issue_id, + job_arguments: [nil], + sub_batch_size: 2, + pause_ms: 2, + connection: ::ApplicationRecord.connection + ) + end + + it 'backfills data correctly' do + # Because we now also have a database trigger to ensure the work_item_dates_sources + # dates are synced with work_items, we have to reload the objects in memory before we + # can update them to ensure their start_date/due_date are nil + issue_1.reload.update!(start_date: nil, due_date: nil) + issue_2.reload.update!(start_date: nil, due_date: nil) + + expect { migration.perform } + .to change { issue_1.reload.start_date }.from(nil).to(date_source_1.start_date) + .and change { issue_1.reload.due_date }.from(nil).to(date_source_1.due_date) + .and change { issue_2.reload.start_date }.from(nil).to(date_source_2.start_date) + .and change { issue_2.reload.due_date }.from(nil).to(date_source_2.due_date) + .and not_change { unassociated_issue.reload.start_date } + .and not_change { unassociated_issue.reload.due_date } + end + + private + + def work_items(iid:) + table(:issues).create!( + iid: iid, + title: "Issue #{iid}", + lock_version: 1, + namespace_id: namespace.id, + author_id: author.id, + work_item_type_id: epic_type_id + ) + end + + def dates_source(work_item:, start_date: nil, due_date: nil) + table(:work_item_dates_sources).create!( + issue_id: work_item.id, + namespace_id: work_item.namespace_id, + start_date: start_date, + due_date: due_date + ) + end + end diff --git a/spec/migrations/20240702212015_queue_backfill_issues_dates_with_work_item_dates_sources_spec.rb b/spec/migrations/20240702212015_queue_backfill_issues_dates_with_work_item_dates_sources_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..fe3f8586587a68f270c5a8629bd06e16b4ca0727 --- /dev/null +++ b/spec/migrations/20240702212015_queue_backfill_issues_dates_with_work_item_dates_sources_spec.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +require 'spec_helper' +require_migration! + +RSpec.describe QueueBackfillIssuesDatesWithWorkItemDatesSources, feature_category: :team_planning do + let!(:batched_migration) { described_class::MIGRATION } + + it 'schedules a new batched migration' do + reversible_migration do |migration| + migration.before -> { + expect(batched_migration).not_to have_scheduled_batched_migration + } + + migration.after -> { + expect(batched_migration).to have_scheduled_batched_migration( + table_name: :work_item_dates_sources, + column_name: :issue_id, + interval: described_class::DELAY_INTERVAL, + batch_size: described_class::BATCH_SIZE + ) + } + end + end +end diff --git a/spec/models/work_items/dates_source_spec.rb b/spec/models/work_items/dates_source_spec.rb index d75500cab16143ae6737d97ad1d53140c5ee5f70..4cedf3ebf0321907b1b00f7ddc8176fb2bda5862 100644 --- a/spec/models/work_items/dates_source_spec.rb +++ b/spec/models/work_items/dates_source_spec.rb @@ -3,7 +3,7 @@ require 'spec_helper' RSpec.describe WorkItems::DatesSource, feature_category: :portfolio_management do - describe 'ssociations' do + describe 'associations' do it { is_expected.to belong_to(:namespace).inverse_of(:work_items_dates_source) } it { is_expected.to belong_to(:work_item).with_foreign_key('issue_id').inverse_of(:dates_source) } it { is_expected.to belong_to(:due_date_sourcing_work_item).class_name('WorkItem') } @@ -16,8 +16,43 @@ work_item = create(:work_item) date_source = described_class.new(work_item: work_item) - date_source.valid? + expect(date_source).to be_valid expect(date_source.namespace).to eq(work_item.namespace) end + + context 'on database triggers' do + let_it_be_with_reload(:work_item) { create(:work_item) } + + context 'on create' do + it 'ensures to keep the issues table start_date and due_date columns updated' do + date_source = described_class.new( + work_item: work_item, + start_date: 1.day.ago, + due_date: 1.day.from_now + ) + + expect { date_source.save! } + .to change { work_item.reload.start_date }.from(nil).to(date_source.start_date) + .and change { work_item.reload.due_date }.from(nil).to(date_source.due_date) + end + end + + context 'on update' do + it 'ensures to keep the issues table start_date and due_date columns updated' do + start_date = 2.days.ago.to_date + due_date = 2.days.from_now.to_date + + date_source = described_class.create!( + work_item: work_item, + start_date: start_date - 1.day, + due_date: due_date + 1.day + ) + + expect { date_source.update!(start_date: start_date, due_date: due_date) } + .to change { work_item.reload.start_date }.to(start_date) + .and change { work_item.reload.due_date }.to(due_date) + end + end + end end diff --git a/spec/support/matchers/background_migrations_matchers.rb b/spec/support/matchers/background_migrations_matchers.rb index 97993b158c807f98d2c4c561a7aad59d6c8a50b4..328506b4bd55557cfcaf4ed18093c0f435b9b8a5 100644 --- a/spec/support/matchers/background_migrations_matchers.rb +++ b/spec/support/matchers/background_migrations_matchers.rb @@ -89,7 +89,10 @@ def same_arrays?(arg, expected) Gitlab::Database::BackgroundMigration::BatchedMigration .where(job_class_name: migration) - expect(batched_migrations.count).to be(0) + expect(batched_migrations.count).to( + be(0), + "#{migration} should not be scheduled, found #{batched_migrations.count} times" + ) end end