Merge branch 'da-repository-sync-backoff-time' into 'master'

Geo - Respect backoff time when repository have never been synced successfully

See merge request gitlab-org/gitlab-ee!4949

ee/app/finders/geo/project_registry_finder.rb

@@ -236,17 +236,17 @@ module Geo
 
     # @return [ActiveRecord::Relation<Project>] list of projects updated recently
     def legacy_find_projects_updated_recently
-      registries = Geo::ProjectRegistry.dirty.retry_due.pluck(:project_id, :last_repository_successful_sync_at)
+      registries = Geo::ProjectRegistry.dirty.retry_due.pluck(:project_id, :last_repository_synced_at)
       return Project.none if registries.empty?
 
-      id_and_last_sync_values = registries.map do |id, last_repository_successful_sync_at|
-        "(#{id}, #{quote_value(last_repository_successful_sync_at)})"
+      id_and_last_sync_values = registries.map do |id, last_repository_synced_at|
+        "(#{id}, #{quote_value(last_repository_synced_at)})"
       end
 
       joined_relation = current_node.projects.joins(<<~SQL)
         INNER JOIN
         (VALUES #{id_and_last_sync_values.join(',')})
-        project_registry(id, last_repository_successful_sync_at)
+        project_registry(id, last_repository_synced_at)
         ON #{Project.table_name}.id = project_registry.id
       SQL
 

ee/app/models/geo/project_registry.rb

@@ -81,24 +81,24 @@ class Geo::ProjectRegistry < Geo::BaseRegistry
   private
 
   def never_synced_repository?
-    last_repository_successful_sync_at.nil?
+    last_repository_synced_at.nil?
   end
 
   def never_synced_wiki?
-    last_wiki_successful_sync_at.nil?
+    last_wiki_synced_at.nil?
   end
 
   def repository_sync_needed?(timestamp)
     return false unless resync_repository?
     return false if repository_retry_at && timestamp < repository_retry_at
 
-    last_repository_synced_at.nil? || timestamp > last_repository_synced_at
+    last_repository_synced_at && timestamp > last_repository_synced_at
   end
 
   def wiki_sync_needed?(timestamp)
     return false unless resync_wiki?
     return false if wiki_retry_at && timestamp < wiki_retry_at
 
-    last_wiki_synced_at.nil? || timestamp > last_wiki_synced_at
+    last_wiki_synced_at && timestamp > last_wiki_synced_at
   end
 end

ee/app/workers/geo/repository_shard_sync_worker.rb

@@ -65,7 +65,7 @@ module Geo
 
     def find_project_ids_updated_recently(batch_size:)
       shard_restriction(finder.find_projects_updated_recently(batch_size: batch_size))
-        .order('project_registry.last_repository_successful_sync_at ASC NULLS FIRST, projects.last_repository_updated_at ASC')
+        .order('project_registry.last_repository_synced_at ASC NULLS FIRST, projects.last_repository_updated_at ASC')
         .pluck(:id)
     end
 

ee/changelogs/unreleased/da-repository-sync-backoff-time.yml

+---
+title: Geo - Respect backoff time when repository have never been synced successfully
+merge_request:
+author:
+type: fixed

ee/spec/models/geo/project_registry_spec.rb

@@ -104,69 +104,73 @@ describe Geo::ProjectRegistry do
   end
 
   describe '#repository_sync_due?' do
-    where(:resync_repository, :last_successful_sync, :last_sync, :expected) do
+    where(:last_synced_at, :resync, :retry_at, :expected) do
       now = Time.now
       past = now - 1.year
       future = now + 1.year
 
-      true  | nil | nil | true
-      true  | now | nil | true
-      false | nil | nil | true
-      false | now | nil | false
+      nil    | false | nil    | true
+      nil    | true  | nil    | true
+      nil    | true  | past   | true
+      nil    | true  | future | true
 
-      true  | nil | past | true
-      true  | now | past | true
-      false | nil | past | true
-      false | now | past | false
+      past   | false | nil    | false
+      past   | true  | nil    | true
+      past   | true  | past   | true
+      past   | true  | future | false
 
-      true  | nil | future | true
-      true  | now | future | false
-      false | nil | future | true
-      false | now | future | false
+      future | false | nil    | false
+      future | true  | nil    | false
+      future | true  | past   | false
+      future | true  | future | false
     end
 
     with_them do
       before do
-        registry.update!(resync_repository: resync_repository, last_repository_successful_sync_at: last_successful_sync, last_repository_synced_at: last_sync)
+        registry.update!(
+          last_repository_synced_at: last_synced_at,
+          resync_repository: resync,
+          repository_retry_at: retry_at
+        )
       end
 
-      subject { registry.repository_sync_due?(Time.now) }
-
-      it { is_expected.to eq(expected) }
+      it { expect(registry.repository_sync_due?(Time.now)).to eq(expected) }
     end
   end
 
   describe '#wiki_sync_due?' do
-    where(:resync_wiki, :last_successful_sync, :last_sync, :expected) do
+    where(:last_synced_at, :resync, :retry_at, :expected) do
       now = Time.now
       past = now - 1.year
       future = now + 1.year
 
-      true  | nil | nil | true
-      true  | now | nil | true
-      false | nil | nil | true
-      false | now | nil | false
+      nil    | false | nil    | true
+      nil    | true  | nil    | true
+      nil    | true  | past   | true
+      nil    | true  | future | true
 
-      true  | nil | past | true
-      true  | now | past | true
-      false | nil | past | true
-      false | now | past | false
+      past   | false | nil    | false
+      past   | true  | nil    | true
+      past   | true  | past   | true
+      past   | true  | future | false
 
-      true  | nil | future | true
-      true  | now | future | false
-      false | nil | future | true
-      false | now | future | false
+      future | false | nil    | false
+      future | true  | nil    | false
+      future | true  | past   | false
+      future | true  | future | false
     end
 
     with_them do
       before do
-        registry.update!(resync_wiki: resync_wiki, last_wiki_successful_sync_at: last_successful_sync, last_wiki_synced_at: last_sync)
+        registry.update!(
+          last_wiki_synced_at: last_synced_at,
+          resync_wiki: resync,
+          wiki_retry_at: retry_at
+        )
       end
 
-      subject { registry.wiki_sync_due?(Time.now) }
-
       context 'wiki enabled' do
-        it { is_expected.to eq(expected) }
+        it { expect(registry.wiki_sync_due?(Time.now)).to eq(expected) }
       end
 
       context 'wiki disabled' do
@@ -174,7 +178,7 @@ describe Geo::ProjectRegistry do
           project.update!(wiki_enabled: false)
         end
 
-        it { is_expected.to be_falsy }
+        it { expect(registry.wiki_sync_due?(Time.now)).to be_falsy }
       end
     end
   end