Commit f87b7fe3 authored by Sean McGivern's avatar Sean McGivern

Merge branch 'issue_51789_part_1' into 'master'

Migrate issuable states to integer patch 1 of 2

Closes #51789

See merge request gitlab-org/gitlab-ce!25107
parents 5ddd4f0f f2b7da4b
...@@ -23,6 +23,7 @@ module Issuable ...@@ -23,6 +23,7 @@ module Issuable
include Sortable include Sortable
include CreatedAtFilterable include CreatedAtFilterable
include UpdatedAtFilterable include UpdatedAtFilterable
include IssuableStates
include ClosedAtFilterable include ClosedAtFilterable
# This object is used to gather issuable meta data for displaying # This object is used to gather issuable meta data for displaying
...@@ -143,6 +144,15 @@ module Issuable ...@@ -143,6 +144,15 @@ module Issuable
fuzzy_search(query, [:title]) fuzzy_search(query, [:title])
end end
# Available state values persisted in state_id column using state machine
#
# Override this on subclasses if different states are needed
#
# Check MergeRequest.available_states for example
def available_states
@available_states ||= { opened: 1, closed: 2 }.with_indifferent_access
end
# Searches for records with a matching title or description. # Searches for records with a matching title or description.
# #
# This method uses ILIKE on PostgreSQL and LIKE on MySQL. # This method uses ILIKE on PostgreSQL and LIKE on MySQL.
......
# frozen_string_literal: true
module IssuableStates
extend ActiveSupport::Concern
# The state:string column is being migrated to state_id:integer column
# This is a temporary hook to populate state_id column with new values
# and should be removed after the state column is removed.
# Check https://gitlab.com/gitlab-org/gitlab-ce/issues/51789 for more information
included do
before_save :set_state_id
end
def set_state_id
return if state.nil? || state.empty?
# Needed to prevent breaking some migration specs that
# rollback database to a point where state_id does not exist.
# We can use this guard clause for now since this file will
# be removed in the next release.
return unless self.has_attribute?(:state_id)
self.state_id = self.class.available_states[state]
end
end
...@@ -210,6 +210,10 @@ class MergeRequest < ApplicationRecord ...@@ -210,6 +210,10 @@ class MergeRequest < ApplicationRecord
'!' '!'
end end
def self.available_states
@available_states ||= super.merge(merged: 3, locked: 4)
end
# Returns the top 100 target branches # Returns the top 100 target branches
# #
# The returned value is a Array containing branch names # The returned value is a Array containing branch names
......
# frozen_string_literal: true
class AddStateIdToIssuables < ActiveRecord::Migration[5.0]
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
def up
add_column :issues, :state_id, :integer, limit: 2
add_column :merge_requests, :state_id, :integer, limit: 2
end
def down
remove_column :issues, :state_id
remove_column :merge_requests, :state_id
end
end
class ScheduleSyncIssuablesStateId < ActiveRecord::Migration[5.0]
# This migration schedules the sync of state_id for issues and merge requests
# which are converting the state column from string to integer.
# For more information check: https://gitlab.com/gitlab-org/gitlab-ce/issues/51789
include Gitlab::Database::MigrationHelpers
DOWNTIME = false
# 2019-02-12 gitlab.com issuable numbers
# issues count: 13587305
# merge requests count: 18925274
#
# Using 5000 as batch size and 115 seconds interval will give:
# 2718 jobs for issues - taking ~86 hours
# 3786 jobs for merge requests - taking ~120 hours
#
BATCH_SIZE = 5000
DELAY_INTERVAL = 120.seconds.to_i
ISSUES_MIGRATION = 'SyncIssuesStateId'.freeze
MERGE_REQUESTS_MIGRATION = 'SyncMergeRequestsStateId'.freeze
disable_ddl_transaction!
class Issue < ActiveRecord::Base
include EachBatch
self.table_name = 'issues'
end
class MergeRequest < ActiveRecord::Base
include EachBatch
self.table_name = 'merge_requests'
end
def up
queue_background_migration_jobs_by_range_at_intervals(
Issue.all,
ISSUES_MIGRATION,
DELAY_INTERVAL,
batch_size: BATCH_SIZE
)
queue_background_migration_jobs_by_range_at_intervals(
MergeRequest.all,
MERGE_REQUESTS_MIGRATION,
DELAY_INTERVAL,
batch_size: BATCH_SIZE
)
end
def down
# No op
end
end
...@@ -1069,6 +1069,7 @@ ActiveRecord::Schema.define(version: 20190325165127) do ...@@ -1069,6 +1069,7 @@ ActiveRecord::Schema.define(version: 20190325165127) do
t.boolean "discussion_locked" t.boolean "discussion_locked"
t.datetime_with_timezone "closed_at" t.datetime_with_timezone "closed_at"
t.integer "closed_by_id" t.integer "closed_by_id"
t.integer "state_id", limit: 2
t.index ["author_id"], name: "index_issues_on_author_id", using: :btree t.index ["author_id"], name: "index_issues_on_author_id", using: :btree
t.index ["closed_by_id"], name: "index_issues_on_closed_by_id", using: :btree t.index ["closed_by_id"], name: "index_issues_on_closed_by_id", using: :btree
t.index ["confidential"], name: "index_issues_on_confidential", using: :btree t.index ["confidential"], name: "index_issues_on_confidential", using: :btree
...@@ -1317,6 +1318,7 @@ ActiveRecord::Schema.define(version: 20190325165127) do ...@@ -1317,6 +1318,7 @@ ActiveRecord::Schema.define(version: 20190325165127) do
t.string "rebase_commit_sha" t.string "rebase_commit_sha"
t.boolean "squash", default: false, null: false t.boolean "squash", default: false, null: false
t.boolean "allow_maintainer_to_push" t.boolean "allow_maintainer_to_push"
t.integer "state_id", limit: 2
t.index ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree t.index ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree
t.index ["author_id"], name: "index_merge_requests_on_author_id", using: :btree t.index ["author_id"], name: "index_merge_requests_on_author_id", using: :btree
t.index ["created_at"], name: "index_merge_requests_on_created_at", using: :btree t.index ["created_at"], name: "index_merge_requests_on_created_at", using: :btree
......
# frozen_string_literal: true
# rubocop:disable Style/Documentation
module Gitlab
module BackgroundMigration
class SyncIssuesStateId
def perform(start_id, end_id)
ActiveRecord::Base.connection.execute <<~SQL
UPDATE issues
SET state_id =
CASE state
WHEN 'opened' THEN 1
WHEN 'closed' THEN 2
END
WHERE state_id IS NULL
AND id BETWEEN #{start_id} AND #{end_id}
SQL
end
end
end
end
# frozen_string_literal: true
# rubocop:disable Style/Documentation
module Gitlab
module BackgroundMigration
class SyncMergeRequestsStateId
def perform(start_id, end_id)
ActiveRecord::Base.connection.execute <<~SQL
UPDATE merge_requests
SET state_id =
CASE state
WHEN 'opened' THEN 1
WHEN 'closed' THEN 2
WHEN 'merged' THEN 3
WHEN 'locked' THEN 4
END
WHERE state_id IS NULL
AND id BETWEEN #{start_id} AND #{end_id}
SQL
end
end
end
end
...@@ -25,12 +25,12 @@ describe 'Database schema' do ...@@ -25,12 +25,12 @@ describe 'Database schema' do
events: %w[target_id], events: %w[target_id],
forked_project_links: %w[forked_from_project_id], forked_project_links: %w[forked_from_project_id],
identities: %w[user_id], identities: %w[user_id],
issues: %w[last_edited_by_id], issues: %w[last_edited_by_id state_id],
keys: %w[user_id], keys: %w[user_id],
label_links: %w[target_id], label_links: %w[target_id],
lfs_objects_projects: %w[lfs_object_id project_id], lfs_objects_projects: %w[lfs_object_id project_id],
members: %w[source_id created_by_id], members: %w[source_id created_by_id],
merge_requests: %w[last_edited_by_id], merge_requests: %w[last_edited_by_id state_id],
namespaces: %w[owner_id parent_id], namespaces: %w[owner_id parent_id],
notes: %w[author_id commit_id noteable_id updated_by_id resolved_by_id discussion_id], notes: %w[author_id commit_id noteable_id updated_by_id resolved_by_id discussion_id],
notification_settings: %w[source_id], notification_settings: %w[source_id],
......
...@@ -11,6 +11,7 @@ Issue: ...@@ -11,6 +11,7 @@ Issue:
- branch_name - branch_name
- description - description
- state - state
- state_id
- iid - iid
- updated_by_id - updated_by_id
- confidential - confidential
...@@ -158,6 +159,7 @@ MergeRequest: ...@@ -158,6 +159,7 @@ MergeRequest:
- created_at - created_at
- updated_at - updated_at
- state - state
- state_id
- merge_status - merge_status
- target_project_id - target_project_id
- iid - iid
......
# frozen_string_literal: true
require 'spec_helper'
require Rails.root.join('db', 'post_migrate', '20190214112022_schedule_sync_issuables_state_id.rb')
describe ScheduleSyncIssuablesStateId, :migration do
let(:namespaces) { table(:namespaces) }
let(:projects) { table(:projects) }
let(:merge_requests) { table(:merge_requests) }
let(:issues) { table(:issues) }
let(:migration) { described_class.new }
let(:group) { namespaces.create!(name: 'gitlab', path: 'gitlab') }
let(:project) { projects.create!(namespace_id: group.id) }
shared_examples 'scheduling migrations' do
before do
Sidekiq::Worker.clear_all
stub_const("#{described_class.name}::BATCH_SIZE", 2)
end
it 'correctly schedules issuable sync background migration' do
Sidekiq::Testing.fake! do
Timecop.freeze do
migrate!
expect(migration).to be_scheduled_delayed_migration(120.seconds, resource_1.id, resource_2.id)
expect(migration).to be_scheduled_delayed_migration(240.seconds, resource_3.id, resource_4.id)
expect(BackgroundMigrationWorker.jobs.size).to eq(2)
end
end
end
end
describe '#up' do
context 'issues' do
it 'migrates state column to integer' do
opened_issue = issues.create!(description: 'first', state: 'opened')
closed_issue = issues.create!(description: 'second', state: 'closed')
invalid_state_issue = issues.create!(description: 'fourth', state: 'not valid')
migrate!
expect(opened_issue.reload.state_id).to eq(Issue.available_states[:opened])
expect(closed_issue.reload.state_id).to eq(Issue.available_states[:closed])
expect(invalid_state_issue.reload.state_id).to be_nil
end
it_behaves_like 'scheduling migrations' do
let(:migration) { described_class::ISSUES_MIGRATION }
let!(:resource_1) { issues.create!(description: 'first', state: 'opened') }
let!(:resource_2) { issues.create!(description: 'second', state: 'closed') }
let!(:resource_3) { issues.create!(description: 'third', state: 'closed') }
let!(:resource_4) { issues.create!(description: 'fourth', state: 'closed') }
end
end
context 'merge requests' do
it 'migrates state column to integer' do
opened_merge_request = merge_requests.create!(state: 'opened', target_project_id: project.id, target_branch: 'feature1', source_branch: 'master')
closed_merge_request = merge_requests.create!(state: 'closed', target_project_id: project.id, target_branch: 'feature2', source_branch: 'master')
merged_merge_request = merge_requests.create!(state: 'merged', target_project_id: project.id, target_branch: 'feature3', source_branch: 'master')
locked_merge_request = merge_requests.create!(state: 'locked', target_project_id: project.id, target_branch: 'feature4', source_branch: 'master')
migrate!
expect(opened_merge_request.reload.state_id).to eq(MergeRequest.available_states[:opened])
expect(closed_merge_request.reload.state_id).to eq(MergeRequest.available_states[:closed])
expect(merged_merge_request.reload.state_id).to eq(MergeRequest.available_states[:merged])
expect(locked_merge_request.reload.state_id).to eq(MergeRequest.available_states[:locked])
end
it_behaves_like 'scheduling migrations' do
let(:migration) { described_class::MERGE_REQUESTS_MIGRATION }
let!(:resource_1) { merge_requests.create!(state: 'opened', target_project_id: project.id, target_branch: 'feature1', source_branch: 'master') }
let!(:resource_2) { merge_requests.create!(state: 'closed', target_project_id: project.id, target_branch: 'feature2', source_branch: 'master') }
let!(:resource_3) { merge_requests.create!(state: 'merged', target_project_id: project.id, target_branch: 'feature3', source_branch: 'master') }
let!(:resource_4) { merge_requests.create!(state: 'locked', target_project_id: project.id, target_branch: 'feature4', source_branch: 'master') }
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
# This spec checks if state_id column of issues and merge requests
# are being synced on every save.
# It can be removed in the next release. Check https://gitlab.com/gitlab-org/gitlab-ce/issues/51789 for more information.
describe IssuableStates do
[Issue, MergeRequest].each do |klass|
it "syncs state_id column when #{klass.model_name.human} gets created" do
klass.available_states.each do |state, state_id|
issuable = build(klass.model_name.param_key, state: state.to_s)
issuable.save!
expect(issuable.state_id).to eq(state_id)
end
end
it "syncs state_id column when #{klass.model_name.human} gets updated" do
klass.available_states.each do |state, state_id|
issuable = create(klass.model_name.param_key, state: state.to_s)
issuable.update(state: state)
expect(issuable.state_id).to eq(state_id)
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment