Commit c160b8f6 authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre

Merge branch 'mk/fix-design-update-failures' into 'master'

Geo: Fix update event bugs

Closes #238137 and #241668

See merge request gitlab-org/gitlab!40643
parents 834fa20a ce174c5a
...@@ -244,10 +244,23 @@ class GeoNode < ApplicationRecord ...@@ -244,10 +244,23 @@ class GeoNode < ApplicationRecord
ContainerRepository.project_id_in(projects) ContainerRepository.project_id_in(projects)
end end
def container_repositories_include?(container_repository_id)
return false unless Geo::ContainerRepositoryRegistry.replication_enabled?
return true unless selective_sync?
container_repositories.where(id: container_repository_id).exists?
end
def designs def designs
projects.with_designs projects.with_designs
end end
def designs_include?(project_id)
return true unless selective_sync?
designs.where(id: project_id).exists?
end
def lfs_objects def lfs_objects
return LfsObject.all unless selective_sync? return LfsObject.all unless selective_sync?
......
---
title: 'Geo: Fix design repository failures with selective sync, and make container repository updates more robust'
merge_request: 40643
author:
type: fixed
...@@ -37,8 +37,14 @@ module Gitlab ...@@ -37,8 +37,14 @@ module Gitlab
yield if healthy_shard_for?(event) yield if healthy_shard_for?(event)
end end
def replicable_project?(project_id) def replicable_project?
strong_memoize(:"replicable_project_#{project_id}") do memoize_and_short_circuit_if_registry_is_persisted(:"replicable_project_#{event.project_id}", registry) do
Gitlab::Geo.current_node.projects_include?(event.project_id)
end
end
def memoize_and_short_circuit_if_registry_is_persisted(memoize_key, registry, &block)
strong_memoize(memoize_key) do
# If a registry exists, then it *should* be replicated. The # If a registry exists, then it *should* be replicated. The
# registry will be removed by the delete event or # registry will be removed by the delete event or
# RegistryConsistencyWorker if it should no longer be replicated. # RegistryConsistencyWorker if it should no longer be replicated.
...@@ -47,7 +53,7 @@ module Gitlab ...@@ -47,7 +53,7 @@ module Gitlab
# for repository updates which are a large proportion of events. # for repository updates which are a large proportion of events.
next true if registry.persisted? next true if registry.persisted?
Gitlab::Geo.current_node.projects_include?(project_id) yield
end end
end end
......
...@@ -8,8 +8,9 @@ module Gitlab ...@@ -8,8 +8,9 @@ module Gitlab
include BaseEvent include BaseEvent
def process def process
if should_sync? if replicable_container_repository?
registry.repository_updated! registry.repository_updated!
registry.save
job_id = ::Geo::ContainerRepositorySyncWorker.perform_async(event.container_repository_id) job_id = ::Geo::ContainerRepositorySyncWorker.perform_async(event.container_repository_id)
end end
...@@ -19,11 +20,21 @@ module Gitlab ...@@ -19,11 +20,21 @@ module Gitlab
private private
def should_sync? def replicable_container_repository?
strong_memoize(:should_sync) do id = event.container_repository_id
::Geo::ContainerRepositoryRegistry.replication_enabled? &&
registry.container_repository && strong_memoize(:"replicable_container_repository_#{id}") do
replicable_project?(registry.container_repository.project_id) next false unless ::Geo::ContainerRepositoryRegistry.replication_enabled?
# If a registry exists, then it *should* be replicated. The
# registry will be removed by the delete event or
# RegistryConsistencyWorker if it should no longer be replicated.
#
# This early exit helps keep processing of update events
# efficient.
next true if registry.persisted?
Gitlab::Geo.current_node.container_repositories_include?(id)
end end
end end
...@@ -38,10 +49,9 @@ module Gitlab ...@@ -38,10 +49,9 @@ module Gitlab
def log_event(job_id) def log_event(job_id)
super( super(
'Docker Repository update', 'Docker Repository update',
container_repository_id: registry.container_repository_id, container_repository_id: event.container_repository_id,
should_sync: should_sync?,
replication_enabled: ::Geo::ContainerRepositoryRegistry.replication_enabled?, replication_enabled: ::Geo::ContainerRepositoryRegistry.replication_enabled?,
replicable_project: replicable_project?(registry.container_repository.project_id), replicable_container_repository: replicable_container_repository?,
project_id: registry.container_repository.project_id, project_id: registry.container_repository.project_id,
job_id: job_id) job_id: job_id)
end end
......
...@@ -9,7 +9,7 @@ module Gitlab ...@@ -9,7 +9,7 @@ module Gitlab
def process def process
job_id = job_id =
if replicable_project?(event.project_id) if replicable_design?
registry.repository_updated! registry.repository_updated!
registry.save registry.save
...@@ -25,6 +25,12 @@ module Gitlab ...@@ -25,6 +25,12 @@ module Gitlab
@registry ||= ::Geo::DesignRegistry.find_or_initialize_by(project_id: event.project_id) # rubocop: disable CodeReuse/ActiveRecord @registry ||= ::Geo::DesignRegistry.find_or_initialize_by(project_id: event.project_id) # rubocop: disable CodeReuse/ActiveRecord
end end
def replicable_design?
memoize_and_short_circuit_if_registry_is_persisted(:"replicable_design_#{event.project_id}", registry) do
Gitlab::Geo.current_node.designs_include?(event.project_id)
end
end
def schedule_job(event) def schedule_job(event)
enqueue_job_if_shard_healthy(event) do enqueue_job_if_shard_healthy(event) do
::Geo::DesignRepositorySyncWorker.perform_async(event.project_id) ::Geo::DesignRepositorySyncWorker.perform_async(event.project_id)
...@@ -36,7 +42,7 @@ module Gitlab ...@@ -36,7 +42,7 @@ module Gitlab
'Design repository update', 'Design repository update',
project_id: event.project_id, project_id: event.project_id,
scheduled_at: Time.now, scheduled_at: Time.now,
replicable_project: replicable_project?(event.project_id), replicable_design: replicable_design?,
job_id: job_id) job_id: job_id)
end end
end end
......
...@@ -8,7 +8,7 @@ module Gitlab ...@@ -8,7 +8,7 @@ module Gitlab
include BaseEvent include BaseEvent
def process def process
if replicable_project?(event.project_id) if replicable_project?
registry.repository_created!(event) registry.repository_created!(event)
job_id = nil job_id = nil
...@@ -31,7 +31,7 @@ module Gitlab ...@@ -31,7 +31,7 @@ module Gitlab
wiki_path: event.wiki_path, wiki_path: event.wiki_path,
resync_repository: registry.resync_repository, resync_repository: registry.resync_repository,
resync_wiki: registry.resync_wiki, resync_wiki: registry.resync_wiki,
replicable_project: replicable_project?(event.project_id), replicable_project: replicable_project?,
job_id: job_id) job_id: job_id)
end end
end end
......
...@@ -8,7 +8,7 @@ module Gitlab ...@@ -8,7 +8,7 @@ module Gitlab
include BaseEvent include BaseEvent
def process def process
if replicable_project?(event.project_id) if replicable_project?
registry.repository_updated!(event.source, scheduled_at) registry.repository_updated!(event.source, scheduled_at)
job_id = enqueue_job_if_shard_healthy(event) do job_id = enqueue_job_if_shard_healthy(event) do
...@@ -33,7 +33,7 @@ module Gitlab ...@@ -33,7 +33,7 @@ module Gitlab
resync_repository: registry.resync_repository, resync_repository: registry.resync_repository,
resync_wiki: registry.resync_wiki, resync_wiki: registry.resync_wiki,
scheduled_at: scheduled_at, scheduled_at: scheduled_at,
replicable_project: replicable_project?(event.project_id), replicable_project: replicable_project?,
job_id: job_id) job_id: job_id)
end end
......
...@@ -30,25 +30,11 @@ RSpec.describe Gitlab::Geo::LogCursor::Events::ContainerRepositoryUpdatedEvent, ...@@ -30,25 +30,11 @@ RSpec.describe Gitlab::Geo::LogCursor::Events::ContainerRepositoryUpdatedEvent,
stub_config(geo: { registry_replication: { enabled: true } }) stub_config(geo: { registry_replication: { enabled: true } })
end end
context "when the container repository's project is not excluded by selective sync" do context "when the container repository is not excluded by selective sync" do
# TODO: Fix https://gitlab.com/gitlab-org/gitlab/-/issues/233514 and it_behaves_like 'event should trigger a sync'
# use this test, and remove the other tests in this context.
# it_behaves_like 'event should trigger a sync'
context 'when a registry does not yet exist' do
it_behaves_like 'event schedules a sync worker'
it_behaves_like 'logs event source info'
end
context 'when a registry exists' do
let!(:registry) { create(registry_factory, *registry_factory_args) }
it_behaves_like 'event schedules a sync worker'
it_behaves_like 'logs event source info'
end
end end
context "when the container repository's project is excluded by selective sync" do context "when the container repository is excluded by selective sync" do
before do before do
stub_current_geo_node(secondary_excludes_all_projects) stub_current_geo_node(secondary_excludes_all_projects)
end end
......
...@@ -30,11 +30,24 @@ RSpec.describe Gitlab::Geo::LogCursor::Events::DesignRepositoryUpdatedEvent, :cl ...@@ -30,11 +30,24 @@ RSpec.describe Gitlab::Geo::LogCursor::Events::DesignRepositoryUpdatedEvent, :cl
allow(Gitlab::ShardHealthCache).to receive(:healthy_shard?).with('default').and_return(true) allow(Gitlab::ShardHealthCache).to receive(:healthy_shard?).with('default').and_return(true)
end end
context "when the container repository's project is not excluded by selective sync" do context 'when the design repository is not excluded by selective sync' do
it_behaves_like 'event should trigger a sync' it_behaves_like 'event should trigger a sync'
context 'when the project is included in selective sync but there is no design' do
before do
node = create(:geo_node, selective_sync_type: 'shards', selective_sync_shards: [project.repository_storage])
stub_current_geo_node(node)
end
context 'when a registry does not yet exist' do
it_behaves_like 'event does not create a registry'
it_behaves_like 'event does not schedule a sync worker'
it_behaves_like 'logs event source info'
end
end
end end
context "when the container repository's project is excluded by selective sync" do context "when the design repository is excluded by selective sync" do
before do before do
stub_current_geo_node(secondary_excludes_all_projects) stub_current_geo_node(secondary_excludes_all_projects)
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