Commit 618875c5 authored by Stan Hu's avatar Stan Hu Committed by Toon Claes

Fix duplicate disk path in Backfill ProjectRepos

On GitLab.com, we saw numerous duplicate disk entry inserts because
the migration was not taking the routes table into account. We now
implement this in the migration to be consistent.

Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/56061
parent 6b2f81f6
...@@ -83,7 +83,7 @@ module Gitlab ...@@ -83,7 +83,7 @@ module Gitlab
extend ActiveSupport::Concern extend ActiveSupport::Concern
def full_path def full_path
@full_path ||= build_full_path route&.path || build_full_path
end end
def build_full_path def build_full_path
...@@ -99,6 +99,9 @@ module Gitlab ...@@ -99,6 +99,9 @@ module Gitlab
end end
end end
class Route < ActiveRecord::Base
end
# Namespace model. # Namespace model.
class Namespace < ActiveRecord::Base class Namespace < ActiveRecord::Base
self.table_name = 'namespaces' self.table_name = 'namespaces'
...@@ -110,6 +113,10 @@ module Gitlab ...@@ -110,6 +113,10 @@ module Gitlab
has_many :projects, inverse_of: :parent has_many :projects, inverse_of: :parent
has_many :namespaces, inverse_of: :parent has_many :namespaces, inverse_of: :parent
def route
Route.find_by(source_type: 'Namespace', source_id: self.id)
end
end end
# ProjectRegistry model # ProjectRegistry model
...@@ -165,6 +172,10 @@ module Gitlab ...@@ -165,6 +172,10 @@ module Gitlab
end end
end end
def route
Route.find_by(source_type: 'Project', source_id: self.id)
end
def storage def storage
@storage ||= @storage ||=
if hashed_storage? if hashed_storage?
......
...@@ -34,6 +34,7 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do ...@@ -34,6 +34,7 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do
let!(:project_hashed_storage_2) { create(:project, name: 'bar', path: 'bar', namespace: group, storage_version: 2) } let!(:project_hashed_storage_2) { create(:project, name: 'bar', path: 'bar', namespace: group, storage_version: 2) }
let!(:project_legacy_storage_3) { create(:project, name: 'baz', path: 'baz', namespace: group, storage_version: 0) } let!(:project_legacy_storage_3) { create(:project, name: 'baz', path: 'baz', namespace: group, storage_version: 0) }
let!(:project_legacy_storage_4) { create(:project, name: 'zoo', path: 'zoo', namespace: group, storage_version: nil) } let!(:project_legacy_storage_4) { create(:project, name: 'zoo', path: 'zoo', namespace: group, storage_version: nil) }
let!(:project_legacy_storage_5) { create(:project, name: 'test', path: 'test', namespace: group, storage_version: nil) }
describe '.on_hashed_storage' do describe '.on_hashed_storage' do
it 'finds projects with repository on hashed storage' do it 'finds projects with repository on hashed storage' do
...@@ -47,7 +48,7 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do ...@@ -47,7 +48,7 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do
it 'finds projects with repository on legacy storage' do it 'finds projects with repository on legacy storage' do
projects = described_class.on_legacy_storage.pluck(:id) projects = described_class.on_legacy_storage.pluck(:id)
expect(projects).to match_array([project_legacy_storage_3.id, project_legacy_storage_4.id]) expect(projects).to match_array([project_legacy_storage_3.id, project_legacy_storage_4.id, project_legacy_storage_5.id])
end end
end end
...@@ -58,7 +59,7 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do ...@@ -58,7 +59,7 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do
projects = described_class.without_project_repository.pluck(:id) projects = described_class.without_project_repository.pluck(:id)
expect(projects).to contain_exactly(project_hashed_storage_2.id, project_legacy_storage_4.id) expect(projects).to contain_exactly(project_hashed_storage_2.id, project_legacy_storage_4.id, project_legacy_storage_5.id)
end end
end end
...@@ -78,12 +79,21 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do ...@@ -78,12 +79,21 @@ describe Gitlab::BackgroundMigration::BackfillProjectRepositories do
expect(project.disk_path).to eq(project_legacy_storage_3.disk_path) expect(project.disk_path).to eq(project_legacy_storage_3.disk_path)
end end
it 'returns the correct disk_path using the route entry' do
project_legacy_storage_5.route.update(path: 'zoo/new-test')
project = described_class.find(project_legacy_storage_5.id)
expect(project.disk_path).to eq('zoo/new-test')
end
it 'raises OrphanedNamespaceError when any parent namespace does not exist' do it 'raises OrphanedNamespaceError when any parent namespace does not exist' do
subgroup = create(:group, parent: group) subgroup = create(:group, parent: group)
project_orphaned_namespace = create(:project, name: 'baz', path: 'baz', namespace: subgroup, storage_version: nil) project_orphaned_namespace = create(:project, name: 'baz', path: 'baz', namespace: subgroup, storage_version: nil)
subgroup.update_column(:parent_id, Namespace.maximum(:id).succ) subgroup.update_column(:parent_id, Namespace.maximum(:id).succ)
project = described_class.find(project_orphaned_namespace.id) project = described_class.find(project_orphaned_namespace.id)
project.route.destroy
subgroup.route.destroy
expect { project.disk_path } expect { project.disk_path }
.to raise_error(Gitlab::BackgroundMigration::BackfillProjectRepositories::OrphanedNamespaceError) .to raise_error(Gitlab::BackgroundMigration::BackfillProjectRepositories::OrphanedNamespaceError)
......
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