Commit cb6188ad authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre Committed by Michael Kozono

Rename Geo::FileRegistry model to Geo::UploadRegistry

The file_registry table only tracks uploads. This commit
renames the model to make the model's intent more clear.

This commit also removes some uncessary joins.
parent 179b4f81
......@@ -101,15 +101,16 @@ it's successful, we replace the main repo with the newly cloned one.
### Uploads replication
File uploads are also being replicated to the **secondary** node. To
track the state of syncing, the `Geo::FileRegistry` model is used.
track the state of syncing, the `Geo::UploadRegistry` model is used.
#### File Registry
#### Upload Registry
Similar to the [Project Registry](#project-registry), there is a
`Geo::FileRegistry` model that tracks the synced uploads.
`Geo::UploadRegistry` model that tracks the synced uploads.
CI Job Artifacts are synced in a similar way as uploads or LFS
objects, but they are tracked by `Geo::JobArtifactRegistry` model.
CI Job Artifacts and LFS objects are synced in a similar way as uploads,
but they are tracked by `Geo::JobArtifactRegistry`, and `Geo::LfsObjectRegistry`
models respectively.
#### File Download Dispatch worker
......
......@@ -5,7 +5,7 @@ module Geo
# Counts all existing registries independent
# of any change on filters / selective sync
def count_registry
Geo::FileRegistry.attachments.count
Geo::UploadRegistry.count
end
def count_syncable
......@@ -13,17 +13,17 @@ module Geo
end
def count_synced
registries_for_attachments.merge(Geo::FileRegistry.synced).count
registries_for_attachments.merge(Geo::UploadRegistry.synced).count
end
def count_failed
registries_for_attachments.merge(Geo::FileRegistry.failed).count
registries_for_attachments.merge(Geo::UploadRegistry.failed).count
end
def count_synced_missing_on_primary
registries_for_attachments
.merge(Geo::FileRegistry.synced)
.merge(Geo::FileRegistry.missing_on_primary)
.merge(Geo::UploadRegistry.synced)
.merge(Geo::UploadRegistry.missing_on_primary)
.count
end
......@@ -58,7 +58,6 @@ module Geo
all_attachments
.inner_join_file_registry
.with_files_stored_remotely
.merge(Geo::FileRegistry.attachments)
.id_not_in(except_file_ids)
.limit(batch_size)
end
......@@ -66,8 +65,7 @@ module Geo
# rubocop: disable CodeReuse/ActiveRecord
def find_retryable_failed_registries(batch_size:, except_file_ids: [])
Geo::FileRegistry
.attachments
Geo::UploadRegistry
.failed
.retry_due
.file_id_not_in(except_file_ids)
......@@ -77,8 +75,7 @@ module Geo
# rubocop: disable CodeReuse/ActiveRecord
def find_retryable_synced_missing_on_primary_registries(batch_size:, except_file_ids: [])
Geo::FileRegistry
.attachments
Geo::UploadRegistry
.synced
.missing_on_primary
.retry_due
......@@ -98,7 +95,7 @@ module Geo
end
def registries_for_attachments
attachments.inner_join_file_registry.merge(Geo::FileRegistry.attachments)
attachments.inner_join_file_registry
end
end
end
......@@ -11,13 +11,12 @@ module Geo
self.primary_key = :id
self.table_name = Gitlab::Geo::Fdw.foreign_table_name('uploads')
has_one :registry, class_name: 'Geo::FileRegistry', foreign_key: :file_id
has_one :registry, class_name: 'Geo::UploadRegistry', foreign_key: :file_id
class << self
def for_model(model)
inner_join_file_registry
.where(model_id: model.id, model_type: model.class.name)
.merge(Geo::FileRegistry.uploads)
end
def inner_join_file_registry
......@@ -42,14 +41,14 @@ module Geo
private
def file_registry_table
Geo::FileRegistry.arel_table
Geo::UploadRegistry.arel_table
end
def left_outer_join_file_registry
join_statement =
arel_table
.join(file_registry_table, Arel::Nodes::OuterJoin)
.on(arel_table[:id].eq(file_registry_table[:file_id]).and(file_registry_table[:file_type].in(Gitlab::Geo::Replication::USER_UPLOADS_OBJECT_TYPES)))
.on(arel_table[:id].eq(file_registry_table[:file_id]))
joins(join_statement.join_sources)
end
......
# frozen_string_literal: true
class Geo::FileRegistry < Geo::BaseRegistry
include Geo::Syncable
scope :attachments, -> { where(file_type: Gitlab::Geo::Replication::USER_UPLOADS_OBJECT_TYPES) }
scope :failed, -> { where(success: false).where.not(retry_count: nil) }
scope :fresh, -> { order(created_at: :desc) }
scope :never, -> { where(success: false, retry_count: nil) }
scope :uploads, -> { where(file_type: Gitlab::Geo::Replication::UPLOAD_OBJECT_TYPE) }
self.inheritance_column = 'file_type'
def self.find_sti_class(file_type)
if Gitlab::Geo::Replication.object_type_from_user_uploads?(file_type)
Geo::UploadRegistry
end
end
def self.file_id_in(ids)
where(file_id: ids)
end
def self.file_id_not_in(ids)
where.not(file_id: ids)
end
def self.pluck_file_key
where(nil).pluck(:file_id)
end
def self.with_status(status)
case status
when 'synced', 'never', 'failed'
self.public_send(status) # rubocop: disable GitlabSecurity/PublicSend
else
all
end
end
# Returns a synchronization state based on existing attribute values
#
# It takes into account things like if a successful replication has been done
# if there are pending actions or existing errors
#
# @return [Symbol] :synced, :never, or :failed
def synchronization_state
return :synced if success?
return :never if retry_count.nil?
:failed
end
end
# frozen_string_literal: true
class Geo::UploadRegistry < Geo::FileRegistry
class Geo::UploadRegistry < Geo::BaseRegistry
include Geo::Syncable
self.table_name = 'file_registry'
belongs_to :upload, foreign_key: :file_id
if Rails.gem_version >= Gem::Version.new('6.0')
raise '.type_condition was changed in Rails 6.0, please adapt this code accordingly'
# see https://github.com/rails/rails/commit/6a1a1e66ea7a917942bd8369fa8dbfedce391dab
scope :failed, -> { where(success: false).where.not(retry_count: nil) }
scope :fresh, -> { order(created_at: :desc) }
scope :never, -> { where(success: false, retry_count: nil) }
def self.file_id_in(ids)
where(file_id: ids)
end
def self.type_condition(table = arel_table)
sti_column = arel_attribute(inheritance_column, table)
sti_names = Gitlab::Geo::Replication::USER_UPLOADS_OBJECT_TYPES
def self.file_id_not_in(ids)
where.not(file_id: ids)
end
sti_column.in(sti_names)
def self.pluck_file_key
where(nil).pluck(:file_id)
end
def self.with_search(query)
......@@ -21,6 +29,15 @@ class Geo::UploadRegistry < Geo::FileRegistry
where(file_id: Geo::Fdw::Upload.search(query))
end
def self.with_status(status)
case status
when 'synced', 'never', 'failed'
self.public_send(status) # rubocop: disable GitlabSecurity/PublicSend
else
all
end
end
def file
upload&.path || s_('Removed %{type} with id %{id}') % { type: file_type, id: file_id }
end
......@@ -28,4 +45,17 @@ class Geo::UploadRegistry < Geo::FileRegistry
def project
return upload.model if upload&.model.is_a?(Project)
end
# Returns a synchronization state based on existing attribute values
#
# It takes into account things like if a successful replication has been done
# if there are pending actions or existing errors
#
# @return [Symbol] :synced, :never, or :failed
def synchronization_state
return :synced if success?
return :never if retry_count.nil?
:failed
end
end
......@@ -2,9 +2,9 @@
module Geo
# This class is responsible for:
# * Finding the appropriate Downloader class for a FileRegistry record
# * Finding the appropriate Downloader class for a UploadRegistry record
# * Executing the Downloader
# * Marking the FileRegistry record as synced or needing retry
# * Marking the UploadRegistry record as synced or needing retry
class FileDownloadService < BaseFileService
include Gitlab::Utils::StrongMemoize
......@@ -63,7 +63,7 @@ module Geo
elsif lfs?
Geo::LfsObjectRegistry.find_or_initialize_by(lfs_object_id: object_db_id)
else
Geo::FileRegistry.find_or_initialize_by(file_type: object_type, file_id: object_db_id)
Geo::UploadRegistry.find_or_initialize_by(file_type: object_type, file_id: object_db_id)
end
end
end
......
......@@ -51,7 +51,7 @@ module Geo
elsif lfs?
::Geo::LfsObjectRegistry.find_by(lfs_object_id: object_db_id)
else
::Geo::FileRegistry.find_by(file_type: object_type, file_id: object_db_id)
::Geo::UploadRegistry.find_by(file_type: object_type, file_id: object_db_id)
end
end
end
......
......@@ -16,7 +16,6 @@ module Gitlab
reflect(
query
.joins(fdw_inner_join_uploads)
.merge(::Geo::FileRegistry.uploads)
.where(
fdw_upload_table[:model_id].eq(model.id)
.and(fdw_upload_table[:model_type].eq(model.class.name))
......@@ -28,11 +27,11 @@ module Gitlab
private
def base
::Geo::FileRegistry.select(file_registry_table[Arel.star])
::Geo::UploadRegistry.select(file_registry_table[Arel.star])
end
def file_registry_table
::Geo::FileRegistry.arel_table
::Geo::UploadRegistry.arel_table
end
def fdw_upload_table
......
......@@ -4,7 +4,6 @@ module Gitlab
module Geo
module Replication
USER_UPLOADS_OBJECT_TYPES = %i[attachment avatar file import_export namespace_file personal_file favicon].freeze
UPLOAD_OBJECT_TYPE = :file
FILE_NOT_FOUND_GEO_CODE = 'FILE_NOT_FOUND'.freeze
def self.object_type_from_user_uploads?(object_type)
......
......@@ -7,9 +7,9 @@ describe Admin::Geo::UploadsController, :geo do
set(:admin) { create(:admin) }
set(:secondary) { create(:geo_node) }
set(:synced_registry) { create(:geo_file_registry, :with_file, :attachment, success: true) }
set(:failed_registry) { create(:geo_file_registry, :failed) }
set(:never_registry) { create(:geo_file_registry, :failed, retry_count: nil) }
set(:synced_registry) { create(:geo_upload_registry, :with_file, :attachment, success: true) }
set(:failed_registry) { create(:geo_upload_registry, :failed) }
set(:never_registry) { create(:geo_upload_registry, :failed, retry_count: nil) }
def css_id(registry)
"#upload-#{registry.id}-header"
......@@ -91,10 +91,10 @@ describe Admin::Geo::UploadsController, :geo do
end
describe '#destroy' do
subject { delete :destroy, params: { id: upload_registry } }
subject { delete :destroy, params: { id: registry } }
it_behaves_like 'license required' do
let(:upload_registry) { create(:geo_file_registry) }
let(:registry) { create(:geo_upload_registry) }
end
context 'with a valid license' do
......@@ -103,24 +103,24 @@ describe Admin::Geo::UploadsController, :geo do
end
context 'with an orphaned registry' do
let(:upload_registry) { create(:geo_file_registry, success: true) }
let(:registry) { create(:geo_upload_registry, success: true) }
it 'removes the registry' do
upload_registry.update_column(:file_id, -1)
registry.update_column(:file_id, -1)
expect(subject).to redirect_to(admin_geo_uploads_path)
expect(flash[:notice]).to include('was successfully removed')
expect { Geo::UploadRegistry.find(upload_registry.id) }.to raise_error(ActiveRecord::RecordNotFound)
expect { Geo::UploadRegistry.find(registry.id) }.to raise_error(ActiveRecord::RecordNotFound)
end
end
context 'with a regular registry' do
let(:upload_registry) { create(:geo_file_registry, :avatar, :with_file, success: true) }
let(:registry) { create(:geo_upload_registry, :avatar, :with_file, success: true) }
it 'does not delete the registry and gives an error' do
expect(subject).to redirect_to(admin_geo_uploads_path)
expect(flash[:alert]).to include('Could not remove tracking entry')
expect { Geo::UploadRegistry.find(upload_registry.id) }.not_to raise_error
expect { Geo::UploadRegistry.find(registry.id) }.not_to raise_error
end
end
end
......
# frozen_string_literal: true
FactoryBot.define do
factory :geo_file_registry, class: Geo::FileRegistry do
factory :geo_upload_registry, class: Geo::UploadRegistry do
sequence(:file_id)
file_type { :file }
success { true }
......@@ -14,8 +14,6 @@ FactoryBot.define do
trait(:favicon) { file_type { :favicon } }
trait(:import_export) { file_type { :import_export } }
factory :geo_upload_registry, class: Geo::UploadRegistry
trait :failed do
success { false }
retry_count { 1 }
......
......@@ -38,9 +38,9 @@ describe Geo::AttachmentRegistryFinder, :geo, :geo_fdw do
context 'finds all the things' do
describe '#find_unsynced' do
before do
create(:geo_file_registry, :avatar, file_id: upload_synced_group.id)
create(:geo_file_registry, :avatar, file_id: upload_unsynced_group.id)
create(:geo_file_registry, :avatar, file_id: upload_remote_synced_project.id)
create(:geo_upload_registry, :avatar, file_id: upload_synced_group.id)
create(:geo_upload_registry, :avatar, file_id: upload_unsynced_group.id)
create(:geo_upload_registry, :avatar, file_id: upload_remote_synced_project.id)
end
context 'with object storage sync enabled' do
......@@ -96,8 +96,8 @@ describe Geo::AttachmentRegistryFinder, :geo, :geo_fdw do
describe '#find_migrated_local' do
before do
create(:geo_file_registry, :avatar, file_id: upload_remote_synced_project.id)
create(:geo_file_registry, :avatar, file_id: upload_remote_unsynced_project.id)
create(:geo_upload_registry, :avatar, file_id: upload_remote_synced_project.id)
create(:geo_upload_registry, :avatar, file_id: upload_remote_unsynced_project.id)
end
it 'returns attachments stored remotely and successfully synced locally' do
......@@ -137,14 +137,14 @@ describe Geo::AttachmentRegistryFinder, :geo, :geo_fdw do
context 'counts all the things' do
describe '#count_synced' do
before do
create(:geo_file_registry, :attachment, :failed, file_id: upload_synced_group.id)
create(:geo_file_registry, :attachment, file_id: upload_unsynced_group.id)
create(:geo_file_registry, :attachment, file_id: upload_issuable_synced_nested_project.id)
create(:geo_file_registry, :attachment, file_id: upload_unsynced_project.id)
create(:geo_file_registry, :attachment, file_id: upload_synced_project.id)
create(:geo_file_registry, :attachment, file_id: upload_personal_snippet.id)
create(:geo_file_registry, :attachment, file_id: upload_remote_synced_project.id)
create(:geo_file_registry, :attachment, file_id: upload_remote_unsynced_project.id)
create(:geo_upload_registry, :attachment, :failed, file_id: upload_synced_group.id)
create(:geo_upload_registry, :attachment, file_id: upload_unsynced_group.id)
create(:geo_upload_registry, :attachment, file_id: upload_issuable_synced_nested_project.id)
create(:geo_upload_registry, :attachment, file_id: upload_unsynced_project.id)
create(:geo_upload_registry, :attachment, file_id: upload_synced_project.id)
create(:geo_upload_registry, :attachment, file_id: upload_personal_snippet.id)
create(:geo_upload_registry, :attachment, file_id: upload_remote_synced_project.id)
create(:geo_upload_registry, :attachment, file_id: upload_remote_unsynced_project.id)
end
context 'with object storage sync enabled' do
......@@ -180,14 +180,14 @@ describe Geo::AttachmentRegistryFinder, :geo, :geo_fdw do
describe '#count_failed' do
before do
create(:geo_file_registry, :attachment, file_id: upload_synced_group.id)
create(:geo_file_registry, :attachment, :failed, file_id: upload_unsynced_group.id)
create(:geo_file_registry, :attachment, :failed, file_id: upload_issuable_synced_nested_project.id)
create(:geo_file_registry, :attachment, :failed, file_id: upload_unsynced_project.id)
create(:geo_file_registry, :attachment, :failed, file_id: upload_synced_project.id)
create(:geo_file_registry, :attachment, :failed, file_id: upload_personal_snippet.id)
create(:geo_file_registry, :attachment, :failed, file_id: upload_remote_synced_project.id)
create(:geo_file_registry, :attachment, :failed, file_id: upload_remote_unsynced_project.id)
create(:geo_upload_registry, :attachment, file_id: upload_synced_group.id)
create(:geo_upload_registry, :attachment, :failed, file_id: upload_unsynced_group.id)
create(:geo_upload_registry, :attachment, :failed, file_id: upload_issuable_synced_nested_project.id)
create(:geo_upload_registry, :attachment, :failed, file_id: upload_unsynced_project.id)
create(:geo_upload_registry, :attachment, :failed, file_id: upload_synced_project.id)
create(:geo_upload_registry, :attachment, :failed, file_id: upload_personal_snippet.id)
create(:geo_upload_registry, :attachment, :failed, file_id: upload_remote_synced_project.id)
create(:geo_upload_registry, :attachment, :failed, file_id: upload_remote_unsynced_project.id)
end
context 'with object storage sync enabled' do
......@@ -223,14 +223,14 @@ describe Geo::AttachmentRegistryFinder, :geo, :geo_fdw do
describe '#count_synced_missing_on_primary' do
before do
create(:geo_file_registry, :attachment, :failed, file_id: upload_synced_group.id, missing_on_primary: true)
create(:geo_file_registry, :attachment, file_id: upload_unsynced_group.id, missing_on_primary: true)
create(:geo_file_registry, :attachment, file_id: upload_issuable_synced_nested_project.id, missing_on_primary: true)
create(:geo_file_registry, :attachment, file_id: upload_unsynced_project.id, missing_on_primary: false)
create(:geo_file_registry, :attachment, file_id: upload_synced_project.id, missing_on_primary: true)
create(:geo_file_registry, :attachment, file_id: upload_personal_snippet.id, missing_on_primary: true)
create(:geo_file_registry, :attachment, file_id: upload_remote_synced_project.id, missing_on_primary: true)
create(:geo_file_registry, :attachment, file_id: upload_remote_unsynced_project.id, missing_on_primary: true)
create(:geo_upload_registry, :attachment, :failed, file_id: upload_synced_group.id, missing_on_primary: true)
create(:geo_upload_registry, :attachment, file_id: upload_unsynced_group.id, missing_on_primary: true)
create(:geo_upload_registry, :attachment, file_id: upload_issuable_synced_nested_project.id, missing_on_primary: true)
create(:geo_upload_registry, :attachment, file_id: upload_unsynced_project.id, missing_on_primary: false)
create(:geo_upload_registry, :attachment, file_id: upload_synced_project.id, missing_on_primary: true)
create(:geo_upload_registry, :attachment, file_id: upload_personal_snippet.id, missing_on_primary: true)
create(:geo_upload_registry, :attachment, file_id: upload_remote_synced_project.id, missing_on_primary: true)
create(:geo_upload_registry, :attachment, file_id: upload_remote_unsynced_project.id, missing_on_primary: true)
end
context 'with object storage sync enabled' do
......@@ -298,15 +298,15 @@ describe Geo::AttachmentRegistryFinder, :geo, :geo_fdw do
describe '#count_registry' do
before do
create(:geo_file_registry, :attachment, :failed, file_id: upload_synced_group.id)
create(:geo_file_registry, :attachment, file_id: upload_unsynced_group.id, missing_on_primary: true)
create(:geo_file_registry, :attachment, file_id: upload_issuable_synced_nested_project.id)
create(:geo_file_registry, :attachment, file_id: upload_unsynced_project.id, missing_on_primary: false)
create(:geo_file_registry, :attachment, file_id: upload_synced_project.id)
create(:geo_file_registry, :attachment, file_id: upload_personal_snippet.id)
create(:geo_file_registry, :attachment, file_id: upload_remote_synced_project.id, missing_on_primary: true)
create(:geo_file_registry, :attachment, file_id: upload_remote_unsynced_project.id, missing_on_primary: true)
create(:geo_file_registry, :attachment, file_id: upload_remote_synced_group.id, missing_on_primary: true)
create(:geo_upload_registry, :attachment, :failed, file_id: upload_synced_group.id)
create(:geo_upload_registry, :attachment, file_id: upload_unsynced_group.id, missing_on_primary: true)
create(:geo_upload_registry, :attachment, file_id: upload_issuable_synced_nested_project.id)
create(:geo_upload_registry, :attachment, file_id: upload_unsynced_project.id, missing_on_primary: false)
create(:geo_upload_registry, :attachment, file_id: upload_synced_project.id)
create(:geo_upload_registry, :attachment, file_id: upload_personal_snippet.id)
create(:geo_upload_registry, :attachment, file_id: upload_remote_synced_project.id, missing_on_primary: true)
create(:geo_upload_registry, :attachment, file_id: upload_remote_unsynced_project.id, missing_on_primary: true)
create(:geo_upload_registry, :attachment, file_id: upload_remote_synced_group.id, missing_on_primary: true)
end
context 'with object storage sync enabled' do
......
......@@ -80,7 +80,7 @@ describe Geo::LfsObjectRegistryFinder, :geo_fdw do
create(:geo_lfs_object_registry, lfs_object_id: lfs_object_2.id)
create(:geo_lfs_object_registry, lfs_object_id: lfs_object_3.id)
create(:geo_lfs_object_registry, lfs_object_id: lfs_object_4.id)
create(:geo_file_registry, :avatar)
create(:geo_upload_registry, :avatar)
allow_any_instance_of(LfsObjectsProject).to receive(:update_project_statistics).and_return(nil)
......
......@@ -7,13 +7,13 @@ describe Gitlab::Geo::Fdw::UploadRegistryQueryBuilder, :geo, :geo_fdw do
let(:upload_1) { create(:upload, :issuable_upload, model: project) }
let(:upload_2) { create(:upload, :issuable_upload, model: project) }
let(:upload_3) { create(:upload, :issuable_upload) }
let!(:file_registry_1) { create(:geo_file_registry, file_id: upload_1.id) }
let!(:file_registry_2) { create(:geo_file_registry, :attachment, file_id: upload_2.id) }
let!(:file_registry_3) { create(:geo_file_registry, file_id: upload_3.id) }
let!(:registry_1) { create(:geo_upload_registry, file_id: upload_1.id) }
let!(:registry_2) { create(:geo_upload_registry, :attachment, file_id: upload_2.id) }
let!(:registry_3) { create(:geo_upload_registry, file_id: upload_3.id) }
describe '#for_model' do
it 'returns registries for uploads that belong to the model' do
expect(subject.for_model(project)).to match_ids(file_registry_1)
expect(subject.for_model(project)).to match_ids(registry_1, registry_2)
end
end
end
......@@ -23,13 +23,13 @@ describe Gitlab::Geo::LogCursor::Events::UploadDeletedEvent, :clean_gitlab_redis
let(:upload) { upload_deleted_event.upload }
it 'does not create a tracking database entry' do
expect { subject.process }.not_to change(Geo::FileRegistry, :count)
expect { subject.process }.not_to change(Geo::UploadRegistry, :count)
end
it 'removes the tracking database entry if exist' do
create(:geo_file_registry, :avatar, file_id: upload.id)
create(:geo_upload_registry, :avatar, file_id: upload.id)
expect { subject.process }.to change(Geo::FileRegistry.attachments, :count).by(-1)
expect { subject.process }.to change(Geo::UploadRegistry, :count).by(-1)
end
end
end
......
......@@ -9,17 +9,14 @@ RSpec.describe Geo::Fdw::LfsObject, :geo, type: :model do
end
describe '.missing_file_registry', :geo_fdw do
subject { described_class.missing_file_registry }
it "returns lfs objects that don't have a corresponding registry entry" do
missing_registry_entries = create_list(:lfs_object, 2)
it "returns lfs objects that don't have a corresponding file registry entry" do
lfs_objects = create_list(:lfs_object, 2)
# simulate existing registry entries with the same +id+, but different +file_type+
lfs_objects.each do |lfs|
create(:geo_file_registry, file_id: lfs.id)
create_list(:lfs_object, 2).each do |lfs|
create(:geo_lfs_object_registry, lfs_object_id: lfs.id)
end
expect(subject).to match_ids(lfs_objects)
expect(described_class.missing_file_registry).to match_ids(missing_registry_entries)
end
end
end
# frozen_string_literal: true
require 'spec_helper'
describe Geo::FileRegistry do
set(:failed) { create(:geo_file_registry, :failed) }
set(:synced) { create(:geo_file_registry) }
describe '.failed' do
it 'returns registries in the failed state' do
expect(described_class.failed).to match_ids(failed)
end
end
describe '.synced' do
it 'returns registries in the synced state' do
expect(described_class.synced).to match_ids(synced)
end
end
describe '.retry_due' do
it 'returns registries in the synced state' do
retry_yesterday = create(:geo_file_registry, retry_at: Date.yesterday)
create(:geo_file_registry, retry_at: Date.tomorrow)
expect(described_class.retry_due).to match_ids([failed, synced, retry_yesterday])
end
end
describe '.never' do
it 'returns registries that are never synced' do
never = create(:geo_file_registry, retry_count: nil, success: false)
expect(described_class.never).to match_ids([never])
end
end
describe '.with_status' do
it 'finds the registries with status "synced"' do
expect(described_class).to receive(:synced)
described_class.with_status('synced')
end
it 'finds the registries with status "never"' do
expect(described_class).to receive(:never)
described_class.with_status('never')
end
it 'finds the registries with status "failed"' do
expect(described_class).to receive(:failed)
described_class.with_status('failed')
end
end
describe '#synchronization_state' do
it 'returns :synced for a successful synced registry' do
expect(synced.synchronization_state).to eq(:synced)
end
it 'returns :never for a successful registry never synced' do
never = build(:geo_file_registry, success: false, retry_count: nil)
expect(never.synchronization_state).to eq(:never)
end
it 'returns :failed for a failed registry' do
expect(failed.synchronization_state).to eq(:failed)
end
end
end
......@@ -3,29 +3,61 @@
require 'spec_helper'
describe Geo::UploadRegistry, :geo, :geo_fdw do
let!(:lfs_registry) { create(:geo_lfs_object_registry) }
let!(:attachment_registry) { create(:geo_file_registry, :attachment, :with_file) }
let!(:avatar_registry) { create(:geo_file_registry, :avatar) }
let!(:file_registry) { create(:geo_file_registry, :file) }
let!(:namespace_file_registry) { create(:geo_file_registry, :namespace_file) }
let!(:personal_file_registry) { create(:geo_file_registry, :personal_file) }
let!(:favicon_registry) { create(:geo_file_registry, :favicon) }
let!(:import_export_registry) { create(:geo_file_registry, :import_export) }
it 'finds all upload registries' do
expected = [attachment_registry,
avatar_registry,
file_registry,
namespace_file_registry,
personal_file_registry,
favicon_registry,
import_export_registry]
expect(described_class.all).to match_ids(expected)
end
let!(:failed) { create(:geo_upload_registry, :failed) }
let!(:synced) { create(:geo_upload_registry) }
it 'finds associated Upload record' do
expect(described_class.find(attachment_registry.id).upload).to be_an_instance_of(Upload)
registry = create(:geo_upload_registry, :attachment, :with_file)
expect(described_class.find(registry.id).upload).to be_an_instance_of(Upload)
end
describe '.failed' do
it 'returns registries in the failed state' do
expect(described_class.failed).to match_ids(failed)
end
end
describe '.synced' do
it 'returns registries in the synced state' do
expect(described_class.synced).to match_ids(synced)
end
end
describe '.retry_due' do
it 'returns registries in the synced state' do
retry_yesterday = create(:geo_upload_registry, retry_at: Date.yesterday)
create(:geo_upload_registry, retry_at: Date.tomorrow)
expect(described_class.retry_due).to match_ids([failed, synced, retry_yesterday])
end
end
describe '.never' do
it 'returns registries that are never synced' do
never = create(:geo_upload_registry, retry_count: nil, success: false)
expect(described_class.never).to match_ids([never])
end
end
describe '.with_status' do
it 'finds the registries with status "synced"' do
expect(described_class).to receive(:synced)
described_class.with_status('synced')
end
it 'finds the registries with status "never"' do
expect(described_class).to receive(:never)
described_class.with_status('never')
end
it 'finds the registries with status "failed"' do
expect(described_class).to receive(:failed)
described_class.with_status('failed')
end
end
describe '.with_search' do
......@@ -51,4 +83,20 @@ describe Geo::UploadRegistry, :geo, :geo_fdw do
expect(registry.file).to match(/^Removed avatar with id/)
end
end
describe '#synchronization_state' do
it 'returns :synced for a successful synced registry' do
expect(synced.synchronization_state).to eq(:synced)
end
it 'returns :never for a successful registry never synced' do
never = build(:geo_upload_registry, success: false, retry_count: nil)
expect(never.synchronization_state).to eq(:never)
end
it 'returns :failed for a failed registry' do
expect(failed.synchronization_state).to eq(:failed)
end
end
end
......@@ -135,9 +135,9 @@ describe GeoNodeStatus, :geo, :geo_fdw do
create_list(:user, 3, avatar: fixture_file_upload('spec/fixtures/dk.png', 'image/png'))
uploads = Upload.all.pluck(:id)
create(:geo_file_registry, :avatar, file_id: uploads[0])
create(:geo_file_registry, :avatar, file_id: uploads[1])
create(:geo_file_registry, :avatar, :failed, file_id: uploads[2])
create(:geo_upload_registry, :avatar, file_id: uploads[0])
create(:geo_upload_registry, :avatar, file_id: uploads[1])
create(:geo_upload_registry, :avatar, :failed, file_id: uploads[2])
expect(subject.attachments_synced_count).to eq(2)
end
......@@ -149,7 +149,7 @@ describe GeoNodeStatus, :geo, :geo_fdw do
expect(subject.attachments_synced_count).to eq(0)
upload = Upload.find_by(model: user, uploader: 'AvatarUploader')
create(:geo_file_registry, :avatar, file_id: upload.id)
create(:geo_upload_registry, :avatar, file_id: upload.id)
subject = described_class.current_node_status
......@@ -164,7 +164,7 @@ describe GeoNodeStatus, :geo, :geo_fdw do
expect(subject.attachments_synced_count).to eq(0)
upload = Upload.find_by(model: user, uploader: 'AvatarUploader')
create(:geo_file_registry, :avatar, file_id: upload.id)
create(:geo_upload_registry, :avatar, file_id: upload.id)
subject = described_class.current_node_status
......@@ -178,9 +178,9 @@ describe GeoNodeStatus, :geo, :geo_fdw do
create_list(:user, 3, avatar: fixture_file_upload('spec/fixtures/dk.png', 'image/png'))
uploads = Upload.all.pluck(:id)
create(:geo_file_registry, :avatar, file_id: uploads[0], missing_on_primary: true)
create(:geo_file_registry, :avatar, file_id: uploads[1])
create(:geo_file_registry, :avatar, :failed, file_id: uploads[2])
create(:geo_upload_registry, :avatar, file_id: uploads[0], missing_on_primary: true)
create(:geo_upload_registry, :avatar, file_id: uploads[1])
create(:geo_upload_registry, :avatar, :failed, file_id: uploads[2])
expect(subject.attachments_synced_missing_on_primary_count).to eq(1)
end
......@@ -190,12 +190,12 @@ describe GeoNodeStatus, :geo, :geo_fdw do
it 'counts failed avatars, attachment, personal snippets and files' do
# These two should be ignored
create(:geo_lfs_object_registry, :with_lfs_object, :failed)
create(:geo_file_registry, :with_file)
create(:geo_upload_registry, :with_file)
create(:geo_file_registry, :with_file, :failed, file_type: :personal_file)
create(:geo_file_registry, :with_file, :failed, file_type: :attachment)
create(:geo_file_registry, :avatar, :with_file, :failed)
create(:geo_file_registry, :with_file, :failed)
create(:geo_upload_registry, :with_file, :failed, file_type: :personal_file)
create(:geo_upload_registry, :with_file, :failed, file_type: :attachment)
create(:geo_upload_registry, :avatar, :with_file, :failed)
create(:geo_upload_registry, :with_file, :failed)
expect(subject.attachments_failed_count).to eq(4)
end
......@@ -216,16 +216,16 @@ describe GeoNodeStatus, :geo, :geo_fdw do
end
it 'returns the right percentage with no group restrictions' do
create(:geo_file_registry, :avatar, file_id: upload_1.id)
create(:geo_file_registry, :avatar, file_id: upload_2.id)
create(:geo_upload_registry, :avatar, file_id: upload_1.id)
create(:geo_upload_registry, :avatar, file_id: upload_2.id)
expect(subject.attachments_synced_in_percentage).to be_within(0.0001).of(50)
end
it 'returns the right percentage with group restrictions' do
secondary.update!(selective_sync_type: 'namespaces', namespaces: [group])
create(:geo_file_registry, :avatar, file_id: upload_1.id)
create(:geo_file_registry, :avatar, file_id: upload_2.id)
create(:geo_upload_registry, :avatar, file_id: upload_1.id)
create(:geo_upload_registry, :avatar, file_id: upload_2.id)
expect(subject.attachments_synced_in_percentage).to be_within(0.0001).of(100)
end
......@@ -250,9 +250,9 @@ describe GeoNodeStatus, :geo, :geo_fdw do
describe '#lfs_objects_synced_count' do
it 'counts synced LFS objects' do
# These four should be ignored
create(:geo_file_registry, :failed)
create(:geo_file_registry, :avatar)
create(:geo_file_registry, file_type: :attachment)
create(:geo_upload_registry, :failed)
create(:geo_upload_registry, :avatar)
create(:geo_upload_registry, file_type: :attachment)
create(:geo_lfs_object_registry, :with_lfs_object, :failed)
create(:geo_lfs_object_registry, :with_lfs_object)
......@@ -264,9 +264,9 @@ describe GeoNodeStatus, :geo, :geo_fdw do
describe '#lfs_objects_synced_missing_on_primary_count' do
it 'counts LFS objects marked as synced due to file missing on the primary' do
# These four should be ignored
create(:geo_file_registry, :failed)
create(:geo_file_registry, :avatar, missing_on_primary: true)
create(:geo_file_registry, file_type: :attachment, missing_on_primary: true)
create(:geo_upload_registry, :failed)
create(:geo_upload_registry, :avatar, missing_on_primary: true)
create(:geo_upload_registry, file_type: :attachment, missing_on_primary: true)
create(:geo_lfs_object_registry, :with_lfs_object, :failed)
create(:geo_lfs_object_registry, :with_lfs_object, missing_on_primary: true)
......@@ -278,9 +278,9 @@ describe GeoNodeStatus, :geo, :geo_fdw do
describe '#lfs_objects_failed_count' do
it 'counts failed LFS objects' do
# These four should be ignored
create(:geo_file_registry, :failed)
create(:geo_file_registry, :avatar, :failed)
create(:geo_file_registry, :failed, file_type: :attachment)
create(:geo_upload_registry, :failed)
create(:geo_upload_registry, :avatar, :failed)
create(:geo_upload_registry, :failed, file_type: :attachment)
create(:geo_lfs_object_registry, :with_lfs_object)
create(:geo_lfs_object_registry, :with_lfs_object, :failed)
......@@ -320,7 +320,7 @@ describe GeoNodeStatus, :geo, :geo_fdw do
describe '#job_artifacts_synced_count' do
it 'counts synced job artifacts' do
# These should be ignored
create(:geo_file_registry)
create(:geo_upload_registry)
create(:geo_job_artifact_registry, :with_artifact, success: false)
create(:geo_job_artifact_registry, :with_artifact, success: true)
......@@ -332,7 +332,7 @@ describe GeoNodeStatus, :geo, :geo_fdw do
describe '#job_artifacts_synced_missing_on_primary_count' do
it 'counts job artifacts marked as synced due to file missing on the primary' do
# These should be ignored
create(:geo_file_registry, missing_on_primary: true)
create(:geo_upload_registry, missing_on_primary: true)
create(:geo_job_artifact_registry, :with_artifact, success: true)
create(:geo_job_artifact_registry, :with_artifact, success: true, missing_on_primary: true)
......@@ -344,9 +344,9 @@ describe GeoNodeStatus, :geo, :geo_fdw do
describe '#job_artifacts_failed_count' do
it 'counts failed job artifacts' do
# These should be ignored
create(:geo_file_registry, :failed)
create(:geo_file_registry, :avatar, :failed)
create(:geo_file_registry, :attachment, :failed)
create(:geo_upload_registry, :failed)
create(:geo_upload_registry, :avatar, :failed)
create(:geo_upload_registry, :attachment, :failed)
create(:geo_job_artifact_registry, :with_artifact, success: true)
create(:geo_job_artifact_registry, :with_artifact, success: false)
......
......@@ -64,7 +64,7 @@ describe Geo::FileDownloadService do
context 'with uploads' do
let!(:registry_entry) do
create(:geo_file_registry, :avatar, success: false, file_id: file.id, retry_count: 31)
create(:geo_upload_registry, :avatar, success: false, file_id: file.id, retry_count: 31)
end
let(:file) { create(:upload) }
......@@ -76,7 +76,7 @@ describe Geo::FileDownloadService do
shared_examples_for 'a service that handles orphaned uploads' do |file_type|
let(:download_service) { described_class.new(file_type, file.id) }
let(:registry) { Geo::FileRegistry }
let(:registry) { Geo::UploadRegistry }
before do
stub_exclusive_lease("file_download_service:#{file_type}:#{file.id}",
......@@ -106,7 +106,7 @@ describe Geo::FileDownloadService do
when 'lfs'
Geo::LfsObjectRegistry
else
Geo::FileRegistry
Geo::UploadRegistry
end
end
......@@ -239,7 +239,7 @@ describe Geo::FileDownloadService do
when 'lfs'
create(:geo_lfs_object_registry, success: false, lfs_object_id: file.id, retry_count: 3, retry_at: 1.hour.ago)
else
create(:geo_file_registry, file_type.to_sym, success: false, file_id: file.id, retry_count: 3, retry_at: 1.hour.ago)
create(:geo_upload_registry, file_type.to_sym, success: false, file_id: file.id, retry_count: 3, retry_at: 1.hour.ago)
end
end
......
......@@ -22,10 +22,10 @@ describe Geo::FileRegistryRemovalService do
end
shared_examples 'removes' do
subject(:service) { described_class.new(file_registry.file_type, file_registry.file_id) }
subject(:service) { described_class.new(registry.file_type, registry.file_id) }
before do
stub_exclusive_lease("file_registry_removal_service:#{file_registry.file_type}:#{file_registry.file_id}",
stub_exclusive_lease("file_registry_removal_service:#{registry.file_type}:#{registry.file_id}",
timeout: Geo::FileRegistryRemovalService::LEASE_TIMEOUT)
end
......@@ -38,7 +38,7 @@ describe Geo::FileRegistryRemovalService do
it 'registry when file was deleted successfully' do
expect do
service.execute
end.to change(Geo::FileRegistry, :count).by(-1)
end.to change(Geo::UploadRegistry, :count).by(-1)
end
end
......@@ -140,7 +140,7 @@ describe Geo::FileRegistryRemovalService do
context 'with avatar' do
let!(:upload) { create(:user, :with_avatar).avatar.upload }
let!(:file_registry) { create(:geo_file_registry, :avatar, file_id: upload.id) }
let!(:registry) { create(:geo_upload_registry, :avatar, file_id: upload.id) }
let!(:file_path) { upload.retrieve_uploader.file.path }
it_behaves_like 'removes'
......@@ -157,7 +157,7 @@ describe Geo::FileRegistryRemovalService do
context 'with attachment' do
let!(:upload) { create(:note, :with_attachment).attachment.upload }
let!(:file_registry) { create(:geo_file_registry, :attachment, file_id: upload.id) }
let!(:registry) { create(:geo_upload_registry, :attachment, file_id: upload.id) }
let!(:file_path) { upload.retrieve_uploader.file.path }
it_behaves_like 'removes'
......@@ -174,7 +174,7 @@ describe Geo::FileRegistryRemovalService do
context 'with file' do
let!(:upload) { create(:user, :with_avatar).avatar.upload }
let!(:file_registry) { create(:geo_file_registry, :avatar, file_id: upload.id) }
let!(:registry) { create(:geo_upload_registry, :avatar, file_id: upload.id) }
let!(:file_path) { upload.retrieve_uploader.file.path }
it_behaves_like 'removes'
......@@ -197,7 +197,7 @@ describe Geo::FileRegistryRemovalService do
Upload.find_by(model: group, uploader: NamespaceFileUploader.name)
end
let!(:file_registry) { create(:geo_file_registry, :namespace_file, file_id: upload.id) }
let!(:registry) { create(:geo_upload_registry, :namespace_file, file_id: upload.id) }
let!(:file_path) { upload.retrieve_uploader.file.path }
it_behaves_like 'removes'
......@@ -219,7 +219,7 @@ describe Geo::FileRegistryRemovalService do
PersonalFileUploader.new(snippet).store!(file)
Upload.find_by(model: snippet, uploader: PersonalFileUploader.name)
end
let!(:file_registry) { create(:geo_file_registry, :personal_file, file_id: upload.id) }
let!(:registry) { create(:geo_upload_registry, :personal_file, file_id: upload.id) }
let!(:file_path) { upload.retrieve_uploader.file.path }
it_behaves_like 'removes'
......@@ -241,7 +241,7 @@ describe Geo::FileRegistryRemovalService do
FaviconUploader.new(appearance).store!(file)
Upload.find_by(model: appearance, uploader: FaviconUploader.name)
end
let!(:file_registry) { create(:geo_file_registry, :favicon, file_id: upload.id) }
let!(:registry) { create(:geo_upload_registry, :favicon, file_id: upload.id) }
let!(:file_path) { upload.retrieve_uploader.file.path }
it_behaves_like 'removes'
......
......@@ -11,7 +11,7 @@ describe Geo::FilesExpireService, :geo, :geo_fdw do
describe '#execute' do
let(:file_uploader) { build(:file_uploader, project: project) }
let!(:upload) { Upload.find_by(path: file_uploader.upload_path) }
let!(:file_registry) { create(:geo_file_registry, file_id: upload.id) }
let!(:upload_registry) { create(:geo_upload_registry, file_id: upload.id) }
before do
project.update(path: "#{project.path}_renamed")
......@@ -31,12 +31,12 @@ describe Geo::FilesExpireService, :geo, :geo_fdw do
expect(File.exist?(file_path)).to be_falsey
end
it 'removes file_registry associates with upload' do
expect(file_registry.success).to be_truthy
it 'removes upload_registry associates with upload' do
expect(upload_registry.success).to be_truthy
subject.execute
expect { file_registry.reload }.to raise_error(ActiveRecord::RecordNotFound)
expect { upload_registry.reload }.to raise_error(ActiveRecord::RecordNotFound)
end
end
......
......@@ -52,7 +52,7 @@ describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw do
end
it 'performs Geo::FileDownloadWorker for failed-sync attachments' do
create(:geo_file_registry, :avatar, :failed, file_id: upload.id, bytes: 0)
create(:geo_upload_registry, :avatar, :failed, file_id: upload.id, bytes: 0)
expect(Geo::FileDownloadWorker).to receive(:perform_async)
.with('avatar', upload.id).once.and_return(spy)
......@@ -61,7 +61,7 @@ describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw do
end
it 'does not perform Geo::FileDownloadWorker for synced attachments' do
create(:geo_file_registry, :avatar, file_id: upload.id, bytes: 1234)
create(:geo_upload_registry, :avatar, file_id: upload.id, bytes: 1234)
expect(Geo::FileDownloadWorker).not_to receive(:perform_async)
......@@ -69,7 +69,7 @@ describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw do
end
it 'does not perform Geo::FileDownloadWorker for synced attachments even with 0 bytes downloaded' do
create(:geo_file_registry, :avatar, file_id: upload.id, bytes: 0)
create(:geo_upload_registry, :avatar, file_id: upload.id, bytes: 0)
expect(Geo::FileDownloadWorker).not_to receive(:perform_async)
......@@ -77,7 +77,7 @@ describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw do
end
context 'with a failed file' do
let(:failed_registry) { create(:geo_file_registry, :avatar, :failed, file_id: 999) }
let(:failed_registry) { create(:geo_upload_registry, :avatar, :failed, file_id: 999) }
it 'does not stall backfill' do
unsynced = create(:upload)
......@@ -97,7 +97,7 @@ describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw do
end
it 'does not retry failed files when retry_at is tomorrow' do
failed_registry = create(:geo_file_registry, :avatar, :failed, file_id: 999, retry_at: Date.tomorrow)
failed_registry = create(:geo_upload_registry, :avatar, :failed, file_id: 999, retry_at: Date.tomorrow)
expect(Geo::FileDownloadWorker).not_to receive(:perform_async).with('avatar', failed_registry.file_id)
......@@ -105,7 +105,7 @@ describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw do
end
it 'retries failed files when retry_at is in the past' do
failed_registry = create(:geo_file_registry, :avatar, :failed, file_id: 999, retry_at: Date.yesterday)
failed_registry = create(:geo_upload_registry, :avatar, :failed, file_id: 999, retry_at: Date.yesterday)
expect(Geo::FileDownloadWorker).to receive(:perform_async).with('avatar', failed_registry.file_id)
......@@ -117,7 +117,7 @@ describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw do
let(:synced_upload_with_file_missing_on_primary) { create(:upload) }
before do
Geo::FileRegistry.create!(file_type: :avatar, file_id: synced_upload_with_file_missing_on_primary.id, bytes: 1234, success: true, missing_on_primary: true)
Geo::UploadRegistry.create!(file_type: :avatar, file_id: synced_upload_with_file_missing_on_primary.id, bytes: 1234, success: true, missing_on_primary: true)
end
it 'retries the files if there is spare capacity' do
......@@ -159,7 +159,7 @@ describe Geo::FileDownloadDispatchWorker, :geo, :geo_fdw do
let!(:lfs_object_file_missing_on_primary) { create(:lfs_object, :with_file) }
before do
Geo::FileRegistry.create!(file_type: :lfs, file_id: lfs_object_file_missing_on_primary.id, bytes: 1234, success: true, missing_on_primary: true)
Geo::LfsObjectRegistry.create!(lfs_object_id: lfs_object_file_missing_on_primary.id, bytes: 1234, success: true, missing_on_primary: true)
end
it 'retries the files if there is spare capacity' do
......
......@@ -57,12 +57,12 @@ describe Geo::MigratedLocalFilesCleanUpWorker, :geo, :geo_fdw do
let(:favicon_upload) { create(:upload, :favicon_upload) }
before do
create(:geo_file_registry, :avatar, file_id: avatar_upload.id)
create(:geo_file_registry, :personal_file, file_id: personal_snippet_upload.id)
create(:geo_file_registry, :file, file_id: issuable_upload.id)
create(:geo_file_registry, :namespace_file, file_id: namespace_upload.id)
create(:geo_file_registry, :attachment, file_id: attachment_upload.id)
create(:geo_file_registry, :favicon, file_id: favicon_upload.id)
create(:geo_upload_registry, :avatar, file_id: avatar_upload.id)
create(:geo_upload_registry, :personal_file, file_id: personal_snippet_upload.id)
create(:geo_upload_registry, :file, file_id: issuable_upload.id)
create(:geo_upload_registry, :namespace_file, file_id: namespace_upload.id)
create(:geo_upload_registry, :attachment, file_id: attachment_upload.id)
create(:geo_upload_registry, :favicon, file_id: favicon_upload.id)
end
it 'schedules nothing for attachments stored locally' do
......@@ -160,7 +160,7 @@ describe Geo::MigratedLocalFilesCleanUpWorker, :geo, :geo_fdw do
end
it 'does not perform Geo::FileRegistryRemovalWorker when the backoff time is set' do
create(:geo_file_registry, :avatar)
create(:geo_upload_registry, :avatar)
expect(Rails.cache).to receive(:read).with(cache_key).and_return(true)
......
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