From 15c8d29bd65164727893fbad9b0be0494ee06a8c Mon Sep 17 00:00:00 2001 From: Sarah Yasonik Date: Thu, 13 Jun 2019 16:45:36 +0000 Subject: [PATCH] Refactor for cleaner caching in dashboards Opts to cache a full list of cached dashboards to better manage removing items from the cache. This also allows dashboards to be stored in the cache that don't necessarily correspond to a single dashboard yml. --- .../projects/environments_controller.rb | 2 +- lib/gitlab/metrics/dashboard/base_service.rb | 20 +++------ lib/gitlab/metrics/dashboard/cache.rb | 44 +++++++++++++++++++ lib/gitlab/metrics/dashboard/finder.rb | 2 + .../dashboard/project_dashboard_service.rb | 12 +---- 5 files changed, 54 insertions(+), 26 deletions(-) create mode 100644 lib/gitlab/metrics/dashboard/cache.rb diff --git a/app/controllers/projects/environments_controller.rb b/app/controllers/projects/environments_controller.rb index 08a0647f6b2..ae46a234aa6 100644 --- a/app/controllers/projects/environments_controller.rb +++ b/app/controllers/projects/environments_controller.rb @@ -164,7 +164,7 @@ class Projects::EnvironmentsController < Projects::ApplicationController if Feature.enabled?(:environment_metrics_show_multiple_dashboards, project) result = dashboard_finder.find(project, current_user, environment, params[:dashboard]) - result[:all_dashboards] = project.repository.metrics_dashboard_paths + result[:all_dashboards] = dashboard_finder.find_all_paths(project) else result = dashboard_finder.find(project, current_user, environment) end diff --git a/lib/gitlab/metrics/dashboard/base_service.rb b/lib/gitlab/metrics/dashboard/base_service.rb index 4664aee71f6..090014aa597 100644 --- a/lib/gitlab/metrics/dashboard/base_service.rb +++ b/lib/gitlab/metrics/dashboard/base_service.rb @@ -7,18 +7,19 @@ module Gitlab module Dashboard class BaseService < ::BaseService PROCESSING_ERROR = Gitlab::Metrics::Dashboard::Stages::BaseStage::DashboardProcessingError + NOT_FOUND_ERROR = Gitlab::Template::Finders::RepoTemplateFinder::FileNotFoundError def get_dashboard - return error("#{dashboard_path} could not be found.", :not_found) unless path_available? - success(dashboard: process_dashboard) + rescue NOT_FOUND_ERROR + error("#{dashboard_path} could not be found.", :not_found) rescue PROCESSING_ERROR => e error(e.message, :unprocessable_entity) end # Summary of all known dashboards for the service. # @return [Array] ex) [{ path: String, default: Boolean }] - def all_dashboard_paths(_project) + def self.all_dashboard_paths(_project) raise NotImplementedError end @@ -38,7 +39,7 @@ module Gitlab # Returns an un-processed dashboard from the cache. def raw_dashboard - Rails.cache.fetch(cache_key) { get_raw_dashboard } + Gitlab::Metrics::Dashboard::Cache.fetch(cache_key) { get_raw_dashboard } end # @return [Hash] an unmodified dashboard @@ -56,17 +57,6 @@ module Gitlab def insert_project_metrics? false end - - # Checks if dashboard path exists or should be rejected - # as a result of file-changes to the project repository. - # @return [Boolean] - def path_available? - available_paths = Gitlab::Metrics::Dashboard::Finder.find_all_paths(project) - - available_paths.any? do |path_params| - path_params[:path] == dashboard_path - end - end end end end diff --git a/lib/gitlab/metrics/dashboard/cache.rb b/lib/gitlab/metrics/dashboard/cache.rb new file mode 100644 index 00000000000..a9ccf0fea9b --- /dev/null +++ b/lib/gitlab/metrics/dashboard/cache.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'set' + +module Gitlab + module Metrics + module Dashboard + class Cache + CACHE_KEYS = 'all_cached_metric_dashboards' + + class << self + # Stores a dashboard in the cache, documenting the key + # so the cached can be cleared in bulk at another time. + def fetch(key) + register_key(key) + + Rails.cache.fetch(key) { yield } + end + + # Resets all dashboard caches, such that all + # dashboard content will be loaded from source on + # subsequent dashboard calls. + def delete_all! + all_keys.each { |key| Rails.cache.delete(key) } + + Rails.cache.delete(CACHE_KEYS) + end + + private + + def register_key(key) + new_keys = all_keys.add(key).to_a.join('|') + + Rails.cache.write(CACHE_KEYS, new_keys) + end + + def all_keys + Set.new(Rails.cache.read(CACHE_KEYS)&.split('|')) + end + end + end + end + end +end diff --git a/lib/gitlab/metrics/dashboard/finder.rb b/lib/gitlab/metrics/dashboard/finder.rb index 4a41590f000..49ea5c7d4f2 100644 --- a/lib/gitlab/metrics/dashboard/finder.rb +++ b/lib/gitlab/metrics/dashboard/finder.rb @@ -27,6 +27,8 @@ module Gitlab # Summary of all known dashboards. Used to populate repo cache. # Prefer #find_all_paths. def find_all_paths_from_source(project) + Gitlab::Metrics::Dashboard::Cache.delete_all! + system_service.all_dashboard_paths(project) .+ project_service.all_dashboard_paths(project) end diff --git a/lib/gitlab/metrics/dashboard/project_dashboard_service.rb b/lib/gitlab/metrics/dashboard/project_dashboard_service.rb index fdffd067c93..e88658e4f9f 100644 --- a/lib/gitlab/metrics/dashboard/project_dashboard_service.rb +++ b/lib/gitlab/metrics/dashboard/project_dashboard_service.rb @@ -13,20 +13,12 @@ module Gitlab def all_dashboard_paths(project) file_finder(project) .list_files_for(DASHBOARD_ROOT) - .map do |filepath| - Rails.cache.delete(cache_key(project.id, filepath)) - - { path: filepath, default: false } - end + .map { |filepath| { path: filepath, default: false } } end def file_finder(project) Gitlab::Template::Finders::RepoTemplateFinder.new(project, DASHBOARD_ROOT, '.yml') end - - def cache_key(id, dashboard_path) - "project_#{id}_metrics_dashboard_#{dashboard_path}" - end end private @@ -39,7 +31,7 @@ module Gitlab end def cache_key - self.class.cache_key(project.id, dashboard_path) + "project_#{project.id}_metrics_dashboard_#{dashboard_path}" end end end