Commit ad0f7577 authored by Nick Thomas's avatar Nick Thomas

Merge branch 'sh-geo-cap-retries' into 'master'

Cap the Geo retry_at value to prevent out of range timestamps

See merge request gitlab-org/gitlab-ee!3461
parents f6affc35 3811973a
...@@ -136,7 +136,7 @@ module Geo ...@@ -136,7 +136,7 @@ module Geo
if started_at if started_at
attrs["last_#{type}_synced_at"] = started_at attrs["last_#{type}_synced_at"] = started_at
attrs["#{type}_retry_count"] = retry_count + 1 attrs["#{type}_retry_count"] = retry_count + 1
attrs["#{type}_retry_at"] = Time.now + delay(attrs["#{type}_retry_count"]).seconds attrs["#{type}_retry_at"] = next_retry_time(attrs["#{type}_retry_count"])
end end
if finished_at if finished_at
...@@ -210,5 +210,14 @@ module Geo ...@@ -210,5 +210,14 @@ module Geo
raise Gitlab::Shell::Error, 'Can not move temporary repository' raise Gitlab::Shell::Error, 'Can not move temporary repository'
end end
end end
# To prevent the retry time from storing invalid dates in the database,
# cap the max time to a week plus some random jitter value.
def next_retry_time(retry_count)
proposed_time = Time.now + delay(retry_count).seconds
max_future_time = Time.now + 7.days + delay(1).seconds
[proposed_time, max_future_time].min
end
end end
end end
...@@ -189,6 +189,25 @@ describe Geo::RepositorySyncService do ...@@ -189,6 +189,25 @@ describe Geo::RepositorySyncService do
subject.execute subject.execute
end end
it 'successfully redownloads the file even if the retry time exceeds max value' do
timestamp = Time.now.utc
registry = create(
:geo_project_registry,
project: project,
repository_retry_count: Geo::BaseSyncService::RETRY_BEFORE_REDOWNLOAD + 2000,
repository_retry_at: timestamp,
force_to_redownload_repository: true
)
subject.execute
# The repository should be redownloaded and cleared without errors. If
# the timestamp were not capped, we would have seen a "timestamp out
# of range" in the first update to the project registry.
registry.reload
expect(registry.repository_retry_at).to be_nil
end
end end
context 'secondary replicates over SSH' do context 'secondary replicates over SSH' do
......
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