fixes merge request according to feedback

app/models/remote_mirror.rb

@@ -2,6 +2,10 @@ class RemoteMirror < ActiveRecord::Base
   include AfterCommitQueue
   include IgnorableColumn
 
+  ignore_column :sync_time
+
+  BACKOFF_DELAY = 5.minutes
+
   attr_encrypted :credentials,
                  key: Gitlab::Application.secrets.db_key_base,
                  marshal: true,
@@ -10,8 +14,6 @@ class RemoteMirror < ActiveRecord::Base
                  insecure_mode: true,
                  algorithm: 'aes-256-cbc'
 
-  ignore_column :sync_time
-
   belongs_to :project, inverse_of: :remote_mirrors
 
   validates :url, presence: true, url: { protocols: %w(ssh git http https), allow_blank: true }
@@ -27,8 +29,16 @@ class RemoteMirror < ActiveRecord::Base
   scope :stuck,   -> { started.where('last_update_at < ? OR (last_update_at IS NULL AND updated_at < ?)', 1.day.ago, 1.day.ago) }
 
   state_machine :update_status, initial: :none do
+    event :schedule do
+      transition [:none, :finished] => :scheduling
+    end
+
+    event :reschedule do
+      transition failed: :scheduling
+    end
+
     event :update_start do
-      transition [:none, :finished] => :started
+      transition scheduling: :started
     end
 
     event :update_finish do
@@ -39,11 +49,16 @@ class RemoteMirror < ActiveRecord::Base
       transition started: :failed
     end
 
+    state :scheduling
     state :started
     state :finished
     state :failed
 
-    after_transition any => :started, do: :schedule_update_job
+    after_transition any => :scheduling, do: :schedule_update_job
+
+    after_transition scheduling: :started do |remote_mirror, _|
+      remote_mirror.update(last_update_started_at: Time.now)
+    end
 
     after_transition started: :finished do |remote_mirror, _|
       timestamp = Time.now
@@ -73,7 +88,11 @@ class RemoteMirror < ActiveRecord::Base
     return unless project
     return if !enabled || update_in_progress?
 
-    update_start
+    update_failed? ? reschedule : schedule
+  end
+
+  def updated_since?(timestamp)
+    last_update_started_at && last_update_started_at > timestamp && !update_failed?
   end
 
   def mark_for_delete_if_blank_url
@@ -131,7 +150,7 @@ class RemoteMirror < ActiveRecord::Base
   end
 
   def add_update_job
-    RepositoryUpdateRemoteMirrorWorker.perform_async(self.id, Time.now) if project&.repository_exists?
+    RepositoryUpdateRemoteMirrorWorker.perform_in(BACKOFF_DELAY, self.id, Time.now) if project&.repository_exists?
   end
 
   def refresh_remote

app/views/projects/mirrors/_show.html.haml

@@ -52,8 +52,8 @@
       %h4.prepend-top-0
         Push to a remote repository
       %p.light
-        Set up the remote repository that you want to update with every push with the content
-        of the current repository.
+        Set up the remote repository that you want to update with the content of the current repository
+        every time someone pushes to it.
         = link_to 'Read more', help_page_path('workflow/repository_mirroring', anchor: 'pushing-to-a-remote-repository'), target: '_blank'
     .col-lg-9
       = render "shared/remote_mirror_update_button", remote_mirror: @remote_mirror
@@ -73,8 +73,7 @@
           .prepend-left-20
             = rm_form.label :enabled, "Remote mirror repository", class: "label-light append-bottom-0"
             %p.light.append-bottom-0
-              Automatically update the remote mirror's branches, tags, and commits from this repository five minutes after every push.
-              In case of failure the mirroring will be retried 5 more times each adding a longer backoff period.
+              Automatically update the remote mirror's branches, tags, and commits from this repository every time someone pushes to it.
         .form-group.has-feedback
           = rm_form.label :url, "Git repository URL", class: "label-light"
           = rm_form.text_field :url, class: "form-control", placeholder: 'https://username:password@gitlab.company.com/group/project.git'

app/workers/repository_update_remote_mirror_worker.rb

@@ -4,31 +4,31 @@ class RepositoryUpdateRemoteMirrorWorker
   include Sidekiq::Worker
   include Gitlab::ShellAdapter
 
-  BACKOFF_DELAY = 5.minutes.to_i
-  MAX_RETRIES = 5
+  sidekiq_options queue: :project_mirror, retry: 3
 
-  sidekiq_options queue: :project_mirror, retry: MAX_RETRIES
-  sidekiq_retry_in { |count| BACKOFF_DELAY**count }
+  sidekiq_retries_exhausted do |msg, e|
+    Sidekiq.logger.warn "Failed #{msg['class']} with #{msg['args']}: #{msg['error_message']}"
+  end
 
-  def perform(remote_mirror_id, current_time)
+  def perform(remote_mirror_id, scheduled_time)
     begin
       remote_mirror = RemoteMirror.find(remote_mirror_id)
-      return if remote_mirror&.last_update_at.to_i > current_time.to_i
+      return if remote_mirror.updated_since?(scheduled_time)
+
+      remote_mirror.update_start
 
       project = remote_mirror.project
       current_user = project.creator
-
       result = Projects::UpdateRemoteMirrorService.new(project, current_user).execute(remote_mirror)
 
-      if result[:status] == :error
-        remote_mirror.mark_as_failed(result[:message])
-      else
-        remote_mirror.update_finish
-      end
-    rescue => ex
-      remote_mirror.mark_as_failed("We're sorry, a temporary error occurred, please try again.")
+      raise UpdateRemoteMirrorError, result[:message] if result[:status] == :error
 
-      raise UpdateRemoteMirrorError, "#{ex.class}: #{Gitlab::UrlSanitizer.sanitize(ex.message)}"
+      remote_mirror.update_finish
+    rescue UpdateRemoteMirrorError => ex
+      remote_mirror.mark_as_failed(Gitlab::UrlSanitizer.sanitize(ex.message))
+      raise
+    rescue => ex
+      raise UpdateRemoteMirrorError, "#{ex.class}: #{ex.message}"
     end
   end
 end

db/migrate/20170427180205_add_last_update_started_at_column_to_remote_mirrors.rb

+class AddLastUpdateStartedAtColumnToRemoteMirrors < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :remote_mirrors, :last_update_started_at, :datetime
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170426175636) do
+ActiveRecord::Schema.define(version: 20170427180205) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -1241,6 +1241,7 @@ ActiveRecord::Schema.define(version: 20170426175636) do
     t.string "encrypted_credentials_salt"
     t.datetime "created_at", null: false
     t.datetime "updated_at", null: false
+    t.datetime "last_update_started_at"
   end
 
   add_index "remote_mirrors", ["last_successful_update_at"], name: "index_remote_mirrors_on_last_successful_update_at", using: :btree
@@ -1622,4 +1623,4 @@ ActiveRecord::Schema.define(version: 20170426175636) do
   add_foreign_key "timelogs", "merge_requests", name: "fk_timelogs_merge_requests_merge_request_id", on_delete: :cascade
   add_foreign_key "trending_projects", "projects", on_delete: :cascade
   add_foreign_key "u2f_registrations", "users"
-end
\ No newline at end of file
+end

doc/workflow/repository_mirroring.md

@@ -71,12 +71,10 @@ repository to push to. Hit **Save changes** for the changes to take effect.
 
 Similarly to the pull mirroring, since the upstream repository functions as a
 mirror to the repository in GitLab, you are advised not to push commits directly
-to the mirrored repository. Instead, any commits should be pushed to GitLab,
-and will end up in the mirrored repository automatically within the configured time,
-or when a [forced update](#forcing-an-update) is initiated.
+to the mirrored repository. Instead, all changes will end up in the mirrored repository
+whenever commits are be pushed to GitLab, or when a [forced update](#forcing-an-update) is initiated.
 
-Push mirroring unlike pull mirroring, does not have any synchronization options available,
-therefore triggering the update whenever a push happens to the respective GitLab repository.
+Pushes into GitLab are automatically pushed to the remote mirror 5 minutes after they come in.
 
 In case of a diverged branch, you will see an error indicated at the
 **Mirror repository** settings.