From de8fd561bcdbee541dc4dcf2644adfc4abc238de Mon Sep 17 00:00:00 2001 From: David Fernandez <dfernandez@gitlab.com> Date: Fri, 1 Oct 2021 18:00:47 +0200 Subject: [PATCH] Move the cleanup policies cache object It's not really a service so it's better placed in the "lib" folder --- .../cache_tags_created_at_service.rb | 70 ------------------ .../cleanup_tags_service.rb | 2 +- lib/gitlab/container_repository/tags/cache.rb | 72 +++++++++++++++++++ .../container_repository/tags/cache_spec.rb} | 2 +- 4 files changed, 74 insertions(+), 72 deletions(-) delete mode 100644 app/services/projects/container_repository/cache_tags_created_at_service.rb create mode 100644 lib/gitlab/container_repository/tags/cache.rb rename spec/{services/projects/container_repository/cache_tags_created_at_service_spec.rb => lib/gitlab/container_repository/tags/cache_spec.rb} (96%) diff --git a/app/services/projects/container_repository/cache_tags_created_at_service.rb b/app/services/projects/container_repository/cache_tags_created_at_service.rb deleted file mode 100644 index 3a5346d7a2302..0000000000000 --- a/app/services/projects/container_repository/cache_tags_created_at_service.rb +++ /dev/null @@ -1,70 +0,0 @@ -# frozen_string_literal: true - -module Projects - module ContainerRepository - class CacheTagsCreatedAtService - def initialize(container_repository) - @container_repository = container_repository - @cached_tag_names = Set.new - end - - def populate(tags) - return if tags.empty? - - # This will load all tags in one Redis roundtrip - # the maximum number of tags is configurable and is set to 200 by default. - # https://gitlab.com/gitlab-org/gitlab/blob/master/doc/user/packages/container_registry/index.md#set-cleanup-limits-to-conserve-resources - keys = tags.map(&method(:cache_key)) - cached_tags_count = 0 - - ::Gitlab::Redis::Cache.with do |redis| - tags.zip(redis.mget(keys)).each do |tag, created_at| - next unless created_at - - tag.created_at = DateTime.rfc3339(created_at) - @cached_tag_names << tag.name - cached_tags_count += 1 - end - end - - cached_tags_count - end - - def insert(tags, max_ttl_in_seconds) - return unless max_ttl_in_seconds - return if tags.empty? - - # tags with nil created_at are not cacheable - # tags already cached don't need to be cached again - cacheable_tags = tags.select do |tag| - tag.created_at.present? && !tag.name.in?(@cached_tag_names) - end - - return if cacheable_tags.empty? - - now = Time.zone.now - - ::Gitlab::Redis::Cache.with do |redis| - # we use a pipeline instead of a MSET because each tag has - # a specific ttl - redis.pipelined do - cacheable_tags.each do |tag| - created_at = tag.created_at - # ttl is the max_ttl_in_seconds reduced by the number - # of seconds that the tag has already existed - ttl = max_ttl_in_seconds - (now - created_at).seconds - ttl = ttl.to_i - redis.set(cache_key(tag), created_at.rfc3339, ex: ttl) if ttl > 0 - end - end - end - end - - private - - def cache_key(tag) - "container_repository:{#{@container_repository.id}}:tag:#{tag.name}:created_at" - end - end - end -end diff --git a/app/services/projects/container_repository/cleanup_tags_service.rb b/app/services/projects/container_repository/cleanup_tags_service.rb index 3a60de0f1ee21..b25205d2894dc 100644 --- a/app/services/projects/container_repository/cleanup_tags_service.rb +++ b/app/services/projects/container_repository/cleanup_tags_service.rb @@ -140,7 +140,7 @@ def cache_tags(tags) def cache strong_memoize(:cache) do - ::Projects::ContainerRepository::CacheTagsCreatedAtService.new(@container_repository) + ::Gitlab::ContainerRepository::Tags::Cache.new(@container_repository) end end diff --git a/lib/gitlab/container_repository/tags/cache.rb b/lib/gitlab/container_repository/tags/cache.rb new file mode 100644 index 0000000000000..ff457fb9219fa --- /dev/null +++ b/lib/gitlab/container_repository/tags/cache.rb @@ -0,0 +1,72 @@ +# frozen_string_literal: true + +module Gitlab + module ContainerRepository + module Tags + class Cache + def initialize(container_repository) + @container_repository = container_repository + @cached_tag_names = Set.new + end + + def populate(tags) + return if tags.empty? + + # This will load all tags in one Redis roundtrip + # the maximum number of tags is configurable and is set to 200 by default. + # https://gitlab.com/gitlab-org/gitlab/blob/master/doc/user/packages/container_registry/index.md#set-cleanup-limits-to-conserve-resources + keys = tags.map(&method(:cache_key)) + cached_tags_count = 0 + + ::Gitlab::Redis::Cache.with do |redis| + tags.zip(redis.mget(keys)).each do |tag, created_at| + next unless created_at + + tag.created_at = DateTime.rfc3339(created_at) + @cached_tag_names << tag.name + cached_tags_count += 1 + end + end + + cached_tags_count + end + + def insert(tags, max_ttl_in_seconds) + return unless max_ttl_in_seconds + return if tags.empty? + + # tags with nil created_at are not cacheable + # tags already cached don't need to be cached again + cacheable_tags = tags.select do |tag| + tag.created_at.present? && !tag.name.in?(@cached_tag_names) + end + + return if cacheable_tags.empty? + + now = Time.zone.now + + ::Gitlab::Redis::Cache.with do |redis| + # we use a pipeline instead of a MSET because each tag has + # a specific ttl + redis.pipelined do + cacheable_tags.each do |tag| + created_at = tag.created_at + # ttl is the max_ttl_in_seconds reduced by the number + # of seconds that the tag has already existed + ttl = max_ttl_in_seconds - (now - created_at).seconds + ttl = ttl.to_i + redis.set(cache_key(tag), created_at.rfc3339, ex: ttl) if ttl > 0 + end + end + end + end + + private + + def cache_key(tag) + "container_repository:{#{@container_repository.id}}:tag:#{tag.name}:created_at" + end + end + end + end +end diff --git a/spec/services/projects/container_repository/cache_tags_created_at_service_spec.rb b/spec/lib/gitlab/container_repository/tags/cache_spec.rb similarity index 96% rename from spec/services/projects/container_repository/cache_tags_created_at_service_spec.rb rename to spec/lib/gitlab/container_repository/tags/cache_spec.rb index dfe2ff9e57cc6..f84c1ce173f75 100644 --- a/spec/services/projects/container_repository/cache_tags_created_at_service_spec.rb +++ b/spec/lib/gitlab/container_repository/tags/cache_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe ::Projects::ContainerRepository::CacheTagsCreatedAtService, :clean_gitlab_redis_cache do +RSpec.describe ::Gitlab::ContainerRepository::Tags::Cache, :clean_gitlab_redis_cache do let_it_be(:dummy_tag_class) { Struct.new(:name, :created_at) } let_it_be(:repository) { create(:container_repository) } -- GitLab