summaryrefslogtreecommitdiff
path: root/app/models/repository.rb
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2018-03-06 17:01:12 -0800
committerStan Hu <stanhu@gmail.com>2018-03-07 06:25:21 -0800
commit611929eb96c0ee9f07d98163d0610325d8a2a322 (patch)
tree6dda6341c6a44c1b94d0db4d5fd915035c590c2a /app/models/repository.rb
parentff00cfe45c744393de5bfcafdcfbd12108cc664f (diff)
downloadgitlab-ce-611929eb96c0ee9f07d98163d0610325d8a2a322.tar.gz
Memoize Repository#empty? instead of double caching the valuesh-remove-double-caching-repo-empty
We saw that in a customer instance, `empty?` was cached to be `true` even though `has_visible_content?` and `exists?` were `true`. This double caching can run into edge cases because there's no guarantee that the inner values will properly expire the outer one, especially if there is Redis replication lag. Consider this scenario: 1. `exists?` and `has_visible_content?` are false 2. `empty?` is expired 3. A subsequent call to `empty?` returns `true` because `exists?` is false even though `empty` is true 4. `exists?` and `has_visible_content?` are then expired 5. `exists?` and `has_visible_content?` are set to true 6. `empty?` is still stuck in the wrong value as `true` Closes #43882
Diffstat (limited to 'app/models/repository.rb')
-rw-r--r--app/models/repository.rb8
1 files changed, 5 insertions, 3 deletions
diff --git a/app/models/repository.rb b/app/models/repository.rb
index 1a14afb951a..e2e2f82ccea 100644
--- a/app/models/repository.rb
+++ b/app/models/repository.rb
@@ -35,7 +35,7 @@ class Repository
CACHED_METHODS = %i(size commit_count rendered_readme contribution_guide
changelog license_blob license_key gitignore koding_yml
gitlab_ci_yml branch_names tag_names branch_count
- tag_count avatar exists? empty? root_ref has_visible_content?
+ tag_count avatar exists? root_ref has_visible_content?
issue_template_names merge_request_template_names).freeze
# Methods that use cache_method but only memoize the value
@@ -360,7 +360,7 @@ class Repository
def expire_emptiness_caches
return unless empty?
- expire_method_caches(%i(empty? has_visible_content?))
+ expire_method_caches(%i(has_visible_content?))
end
def lookup_cache
@@ -506,12 +506,14 @@ class Repository
end
cache_method :exists?
+ # We don't need to cache the output of this method because both exists? and
+ # has_visible_content? are already memoized and cached. There's no guarantee
+ # that the values are expired and loaded atomically.
def empty?
return true unless exists?
!has_visible_content?
end
- cache_method :empty?
# The size of this repository in megabytes.
def size