Commit a7f2f559 authored by Sean McGivern's avatar Sean McGivern

Don't allow a default for reactive_cache_work_type

If we force this to be set explicitly, it's more likely to be correct.
parent d81fd9e9
...@@ -11,6 +11,7 @@ module Clusters ...@@ -11,6 +11,7 @@ module Clusters
RESERVED_NAMESPACES = %w(gitlab-managed-apps).freeze RESERVED_NAMESPACES = %w(gitlab-managed-apps).freeze
self.table_name = 'cluster_platforms_kubernetes' self.table_name = 'cluster_platforms_kubernetes'
self.reactive_cache_work_type = :external_dependency
belongs_to :cluster, inverse_of: :platform_kubernetes, class_name: 'Clusters::Cluster' belongs_to :cluster, inverse_of: :platform_kubernetes, class_name: 'Clusters::Cluster'
......
...@@ -9,7 +9,7 @@ module ReactiveCaching ...@@ -9,7 +9,7 @@ module ReactiveCaching
ExceededReactiveCacheLimit = Class.new(StandardError) ExceededReactiveCacheLimit = Class.new(StandardError)
WORK_TYPE = { WORK_TYPE = {
default: ReactiveCachingWorker, no_dependency: ReactiveCachingWorker,
external_dependency: ExternalServiceReactiveCachingWorker external_dependency: ExternalServiceReactiveCachingWorker
}.freeze }.freeze
...@@ -30,7 +30,6 @@ module ReactiveCaching ...@@ -30,7 +30,6 @@ module ReactiveCaching
self.reactive_cache_refresh_interval = 1.minute self.reactive_cache_refresh_interval = 1.minute
self.reactive_cache_lifetime = 10.minutes self.reactive_cache_lifetime = 10.minutes
self.reactive_cache_hard_limit = nil # this value should be set in megabytes. E.g: 1.megabyte self.reactive_cache_hard_limit = nil # this value should be set in megabytes. E.g: 1.megabyte
self.reactive_cache_work_type = :default
self.reactive_cache_worker_finder = ->(id, *_args) do self.reactive_cache_worker_finder = ->(id, *_args) do
find_by(primary_key => id) find_by(primary_key => id)
end end
......
...@@ -8,5 +8,6 @@ module ReactiveService ...@@ -8,5 +8,6 @@ module ReactiveService
# Default cache key: class name + project_id # Default cache key: class name + project_id
self.reactive_cache_key = ->(service) { [service.class.model_name.singular, service.project_id] } self.reactive_cache_key = ->(service) { [service.class.model_name.singular, service.project_id] }
self.reactive_cache_work_type = :external_dependency
end end
end end
...@@ -30,6 +30,7 @@ class MergeRequest < ApplicationRecord ...@@ -30,6 +30,7 @@ class MergeRequest < ApplicationRecord
self.reactive_cache_key = ->(model) { [model.project.id, model.iid] } self.reactive_cache_key = ->(model) { [model.project.id, model.iid] }
self.reactive_cache_refresh_interval = 10.minutes self.reactive_cache_refresh_interval = 10.minutes
self.reactive_cache_lifetime = 10.minutes self.reactive_cache_lifetime = 10.minutes
self.reactive_cache_work_type = :no_dependency
SORTING_PREFERENCE_FIELD = :merge_requests_sort SORTING_PREFERENCE_FIELD = :merge_requests_sort
......
...@@ -10,6 +10,8 @@ module PodLogs ...@@ -10,6 +10,8 @@ module PodLogs
CACHE_KEY_GET_POD_LOG = 'get_pod_log' CACHE_KEY_GET_POD_LOG = 'get_pod_log'
K8S_NAME_MAX_LENGTH = 253 K8S_NAME_MAX_LENGTH = 253
self.reactive_cache_work_type = :external_dependency
def id def id
cluster.id cluster.id
end end
......
...@@ -11,7 +11,6 @@ module PodLogs ...@@ -11,7 +11,6 @@ module PodLogs
:pod_logs, :pod_logs,
:filter_return_keys :filter_return_keys
self.reactive_cache_work_type = :external_dependency
self.reactive_cache_worker_finder = ->(id, _cache_key, namespace, params) { new(::Clusters::Cluster.find(id), namespace, params: params) } self.reactive_cache_worker_finder = ->(id, _cache_key, namespace, params) { new(::Clusters::Cluster.find(id), namespace, params: params) }
private private
......
...@@ -17,7 +17,6 @@ module PodLogs ...@@ -17,7 +17,6 @@ module PodLogs
:split_logs, :split_logs,
:filter_return_keys :filter_return_keys
self.reactive_cache_work_type = :external_dependency
self.reactive_cache_worker_finder = ->(id, _cache_key, namespace, params) { new(::Clusters::Cluster.find(id), namespace, params: params) } self.reactive_cache_worker_finder = ->(id, _cache_key, namespace, params) { new(::Clusters::Cluster.find(id), namespace, params: params) }
private private
......
...@@ -85,9 +85,7 @@ The ReactiveCaching concern can be used in models as well as `project_services` ...@@ -85,9 +85,7 @@ The ReactiveCaching concern can be used in models as well as `project_services`
1. Implement the `calculate_reactive_cache` method in your model/service. 1. Implement the `calculate_reactive_cache` method in your model/service.
1. Call `with_reactive_cache` in your model/service where the cached value is needed. 1. Call `with_reactive_cache` in your model/service where the cached value is needed.
1. If the `calculate_reactive_cache` method above submits requests to external services 1. Set the [`reactive_cache_work_type` accordingly](#selfreactive_cache_work_type).
(e.g. Prometheus, K8s), make sure to change the
[`reactive_cache_work_type` accordingly](#selfreactive_cache_work_type).
### In controllers ### In controllers
...@@ -252,7 +250,7 @@ self.reactive_cache_hard_limit = 5.megabytes ...@@ -252,7 +250,7 @@ self.reactive_cache_hard_limit = 5.megabytes
- This is the type of work performed by the `calculate_reactive_cache` method. Based on this attribute, - This is the type of work performed by the `calculate_reactive_cache` method. Based on this attribute,
it's able to pick the right worker to process the caching job. Make sure to it's able to pick the right worker to process the caching job. Make sure to
set it as `:external_dependency` if the work performs any external request set it as `:external_dependency` if the work performs any external request
(e.g. Kubernetes, Sentry). (e.g. Kubernetes, Sentry); otherwise set it to `:no_dependency`.
#### `self.reactive_cache_worker_finder` #### `self.reactive_cache_worker_finder`
......
...@@ -14,6 +14,7 @@ RSpec.describe ReactiveCaching, :use_clean_rails_memory_store_caching do ...@@ -14,6 +14,7 @@ RSpec.describe ReactiveCaching, :use_clean_rails_memory_store_caching do
self.reactive_cache_lifetime = 5.minutes self.reactive_cache_lifetime = 5.minutes
self.reactive_cache_refresh_interval = 15.seconds self.reactive_cache_refresh_interval = 15.seconds
self.reactive_cache_work_type = :no_dependency
attr_reader :id attr_reader :id
...@@ -372,4 +373,14 @@ RSpec.describe ReactiveCaching, :use_clean_rails_memory_store_caching do ...@@ -372,4 +373,14 @@ RSpec.describe ReactiveCaching, :use_clean_rails_memory_store_caching do
it { expect(subject.reactive_cache_hard_limit).to be_nil } it { expect(subject.reactive_cache_hard_limit).to be_nil }
it { expect(subject.reactive_cache_worker_finder).to respond_to(:call) } it { expect(subject.reactive_cache_worker_finder).to respond_to(:call) }
end end
describe 'classes including this concern' do
it 'sets reactive_cache_work_type' do
classes = ObjectSpace.each_object(Class).select do |klass|
klass < described_class && klass.name
end
expect(classes).to all(have_attributes(reactive_cache_work_type: be_in(described_class::WORK_TYPE.keys)))
end
end
end end
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