diff options
author | Stan Hu <stanhu@gmail.com> | 2019-07-06 00:58:13 -0700 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2019-07-06 01:07:49 -0700 |
commit | a28844ea5683d6ae7ae7e37065feee9baa59aeaf (patch) | |
tree | 3adeaf09fbef16a1296bd522ced0b0a467844ba2 | |
parent | 6e9f882007a18d594b4a53e417a87fdeec3bc29f (diff) | |
download | gitlab-ce-a28844ea5683d6ae7ae7e37065feee9baa59aeaf.tar.gz |
Prevent amplification of ReactiveCachingWorker jobs upon failuressh-disable-reactive-caching-automatic-retries
When `ReactiveCachingWorker` hits an SSL or other exception that occurs
quickly and reliably, automatically rescheduling a new worker could lead
to excessive number of jobs being scheduled. This happens because not
only does the failed job get rescheduled in a minute, but each Sidekiq
retry will also add even more rescheduled jobs.
In busy instances, this can become an issue because large numbers of
`ReactiveCachingWorker` running can cause high rates of `ExclusiveLease`
reads to occur and possibly saturate the Redis server with queries.
We now disable this automatic retry and rely on Sidekiq to perform its 3
retries with a backoff period.
Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/64176
-rw-r--r-- | app/models/concerns/reactive_caching.rb | 2 | ||||
-rw-r--r-- | changelogs/unreleased/sh-disable-reactive-caching-automatic-retries.yml | 5 | ||||
-rw-r--r-- | spec/models/concerns/reactive_caching_spec.rb | 5 |
3 files changed, 9 insertions, 3 deletions
diff --git a/app/models/concerns/reactive_caching.rb b/app/models/concerns/reactive_caching.rb index 6c3962b4c4f..5b3880f94ba 100644 --- a/app/models/concerns/reactive_caching.rb +++ b/app/models/concerns/reactive_caching.rb @@ -178,7 +178,7 @@ module ReactiveCaching def enqueuing_update(*args) yield - ensure + ReactiveCachingWorker.perform_in(self.class.reactive_cache_refresh_interval, self.class, id, *args) end end diff --git a/changelogs/unreleased/sh-disable-reactive-caching-automatic-retries.yml b/changelogs/unreleased/sh-disable-reactive-caching-automatic-retries.yml new file mode 100644 index 00000000000..a0db68adb78 --- /dev/null +++ b/changelogs/unreleased/sh-disable-reactive-caching-automatic-retries.yml @@ -0,0 +1,5 @@ +--- +title: Prevent amplification of ReactiveCachingWorker jobs upon failures +merge_request: 30432 +author: +type: performance diff --git a/spec/models/concerns/reactive_caching_spec.rb b/spec/models/concerns/reactive_caching_spec.rb index 7faa196623f..e2ab9ddd4a5 100644 --- a/spec/models/concerns/reactive_caching_spec.rb +++ b/spec/models/concerns/reactive_caching_spec.rb @@ -206,8 +206,9 @@ describe ReactiveCaching, :use_clean_rails_memory_store_caching do expect(read_reactive_cache(instance)).to eq("preexisting") end - it 'enqueues a repeat worker' do - expect_reactive_cache_update_queued(instance) + it 'does not enqueue a repeat worker' do + expect(ReactiveCachingWorker) + .not_to receive(:perform_in) expect { go! }.to raise_error("foo") end |