Commit fc1df0d3 authored by Ash McKenzie's avatar Ash McKenzie Committed by Nick Thomas

Shard name vs. Shard updates, general tech debt cleanup

parent d5b77d24
......@@ -39,8 +39,9 @@ module Geo
registry.force_to_redownload_wiki = false
if registry.changed? || registry.last_wiki_synced_at.nil? || registry.last_wiki_successful_sync_at.nil?
registry.last_wiki_synced_at = DateTime.now
registry.last_wiki_successful_sync_at = DateTime.now
now = DateTime.now
registry.last_wiki_synced_at = now
registry.last_wiki_successful_sync_at = now
success = registry.save
log_info("#{success ? 'Successfully marked' : 'Failed to mark'} disabled wiki as synced", registry_id: registry.id, project_id: registry.project_id)
......
......@@ -12,33 +12,34 @@ module Geo
].freeze
def perform
Gitlab::Geo::ShardHealthCache.update(eligible_shards)
Gitlab::Geo::ShardHealthCache.update(eligible_shard_names)
eligible_shards.each do |shard_name|
schedule_job(shard_name)
eligible_shard_names.each { |shard_name| schedule_job(shard_name) }
end
def eligible_shard_names
healthy_shard_names
end
def schedule_job(shard_name)
raise NotImplementedError
end
def eligible_shards
healthy_shards
end
def healthy_shards
strong_memoize(:healthy_shards) do
def healthy_shard_names
strong_memoize(:healthy_shard_names) do
# For now, we need to perform both Gitaly and direct filesystem checks to ensure
# the shard is healthy. We take the intersection of the successful checks
# as the healthy shards.
healthy_ready_shards.map { |result| result.labels[:shard] }.compact.uniq
end
end
def ready_shards
HEALTHY_SHARD_CHECKS.map(&:readiness)
.map { |check_result| check_result.select(&:success) }
.inject(:&)
.map { |check_result| check_result.labels[:shard] }
.compact
.uniq
end
def healthy_ready_shards
ready_shards.map { |result| result.select(&:success) }.inject(:&)
end
end
end
......
......@@ -16,8 +16,8 @@ module Geo
super
end
def eligible_shards
selective_sync_filter(healthy_shards)
def eligible_shard_names
selective_sync_filter(healthy_shard_names)
end
def selective_sync_filter(shards)
......
......@@ -15,7 +15,9 @@ RSpec.describe Geo::ProjectSyncWorker do
end
context 'when project could not be found' do
it 'does not raise an error' do
it 'logs an error and returns' do
expect(subject).to receive(:log_error).with("Couldn't find project, skipping syncing", project_id: 999)
expect { subject.perform(999, Time.now) }.not_to raise_error
end
end
......
......@@ -9,7 +9,7 @@ describe Geo::RepositorySyncWorker, :geo, :clean_gitlab_redis_cache do
let!(:project_in_synced_group) { create(:project, group: synced_group) }
let!(:unsynced_project) { create(:project) }
let(:healthy_shard) { project_in_synced_group.repository.storage }
let(:healthy_shard_name) { project_in_synced_group.repository.storage }
subject { described_class.new }
......@@ -39,13 +39,13 @@ describe Geo::RepositorySyncWorker, :geo, :clean_gitlab_redis_cache do
end
it 'skips backfill for projects on shards excluded by selective sync' do
secondary.update!(selective_sync_type: 'shards', selective_sync_shards: [healthy_shard])
secondary.update!(selective_sync_type: 'shards', selective_sync_shards: [healthy_shard_name])
# Report both shards as healthy
expect(Gitlab::HealthChecks::FsShardsCheck).to receive(:readiness)
.and_return([result(true, healthy_shard), result(true, 'broken')])
.and_return([result(true, healthy_shard_name), result(true, 'broken')])
expect(Gitlab::HealthChecks::GitalyCheck).to receive(:readiness)
.and_return([result(true, healthy_shard), result(true, 'broken')])
.and_return([result(true, healthy_shard_name), result(true, 'broken')])
expect(Geo::RepositoryShardSyncWorker).to receive(:perform_async).with('default')
expect(Geo::RepositoryShardSyncWorker).not_to receive(:perform_async).with('broken')
......@@ -78,11 +78,11 @@ describe Geo::RepositorySyncWorker, :geo, :clean_gitlab_redis_cache do
# Report only one healthy shard
expect(Gitlab::HealthChecks::FsShardsCheck).to receive(:readiness)
.and_return([result(true, healthy_shard), result(true, 'broken')])
.and_return([result(true, healthy_shard_name), result(true, 'broken')])
expect(Gitlab::HealthChecks::GitalyCheck).to receive(:readiness)
.and_return([result(true, healthy_shard), result(false, 'broken')])
.and_return([result(true, healthy_shard_name), result(false, 'broken')])
expect(Geo::RepositoryShardSyncWorker).to receive(:perform_async).with(healthy_shard)
expect(Geo::RepositoryShardSyncWorker).to receive(:perform_async).with(healthy_shard_name)
expect(Geo::RepositoryShardSyncWorker).not_to receive(:perform_async).with('broken')
subject.perform
......
require 'spec_helper'
describe Geo::Scheduler::PerShardSchedulerWorker do
it 'includes ::Gitlab::Geo::LogHelpers' do
it 'includes ApplicationWorker' do
expect(described_class).to include_module(ApplicationWorker)
end
it 'includes CronjobQueue' do
expect(described_class).to include_module(CronjobQueue)
end
it 'includes Gitlab::Utils::StrongMemoize' do
expect(described_class).to include_module(::Gitlab::Utils::StrongMemoize)
end
it 'includes Gitlab::Geo::LogHelpers' do
expect(described_class).to include_module(::Gitlab::Geo::LogHelpers)
end
it 'needs many other specs'
it 'includes FsShards and Gitaly health checks' do
expect(described_class::HEALTHY_SHARD_CHECKS).to include(
Gitlab::HealthChecks::FsShardsCheck,
Gitlab::HealthChecks::GitalyCheck
)
end
describe 'instance methods' do
subject(:per_shard_scheduler_worker) { described_class.new }
let(:default_shard_name) { 'default' }
let(:other_shard_name) { 'other' }
let(:unhealthy_shard_name) { 'unhealthy' }
let(:default_shard) { Gitlab::HealthChecks::Result.new(true, nil, shard: default_shard_name) }
let(:other_shard) { Gitlab::HealthChecks::Result.new(true, nil, shard: other_shard_name) }
let(:unhealthy_shard) { Gitlab::HealthChecks::Result.new(false, '14:Connect Failed', shard: unhealthy_shard_name) }
before do
allow(Gitlab::HealthChecks::FsShardsCheck).to receive(:readiness).and_return([default_shard, other_shard, unhealthy_shard])
allow(Gitlab::HealthChecks::GitalyCheck).to receive(:readiness).and_return([default_shard, other_shard, unhealthy_shard])
end
describe '#schedule_job' do
it "raises a NotImplementedError exception" do
expect do
per_shard_scheduler_worker.schedule_job(default_shard_name)
end.to raise_exception(NotImplementedError)
end
end
describe '#ready_shards' do
let(:ready_shards) { [[default_shard, other_shard, unhealthy_shard], [default_shard, other_shard, unhealthy_shard]] }
it "returns an array of ready shards" do
expect(per_shard_scheduler_worker.ready_shards).to eq(ready_shards)
end
end
describe '#healthy_ready_shards' do
let(:healthy_ready_shards) { [default_shard, other_shard] }
it "returns an array of healthy shard names" do
expect(per_shard_scheduler_worker.healthy_ready_shards).to eq(healthy_ready_shards)
end
end
describe '#healthy_shard_names' do
it "returns an array of healthy shard names" do
expect(per_shard_scheduler_worker.healthy_shard_names).to eq([default_shard_name, other_shard_name])
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