Skip to content

Fix pages internal cache to not use app settings in the cache key

What does this MR do and why?

During the rollout of cache_pages_domain_api feature flag, the gitlab pages QA tests on staging started to fail.

After some investigation, we found out that the application settings are different between rails nodes (where the cache is created) and sidekiq nodes (where the cache is invalidated).

Since we use the application settings is in the cache key, the sidekiq job never finds the cache key to invalidate it.

To fix that, we're now assuming that application settings might be different between any node, rails or sidekiq.

Assuming that the settings are different per node, besides caching the payload in a cache key that contains the domain type, id and application settings hash, like pages_domain_for_project_1_aaaa, the application settings hash is also cached in a key based only in the domain type and id, like: pages_domain_for_project_1.

The application settings cache is a Set which might contain different application settings hashes from different rails nodes.

This way, when invalidating the cache, we can retrieve the application settings hashes using only the domain type and id, reading the pages_domain_for_project_1 key and then building the payload cache key pages_domain_for_project_1_aaaa.

So, when the cache invalidation happens it'll won't need the current node application settings hash to invalidate the cache.

Related to: gitlab-pages#809 (closed)

How to set up and validate locally

/tmp/test.rb
def test
  load 'lib/gitlab/pages/cache_control.rb'

  puts "> Request 1"
  cc = Gitlab::Pages::CacheControl.for_namespace(1)
  puts "\tcache_key: #{cc.cache_key}"
  puts "\tCached settings: #{Rails.cache.read('pages_domain_for_namespace_1').join(',')}"

  puts "\n> change settings <\n\n"
  ApplicationSetting.update(force_pages_access_control: !ApplicationSetting.last.force_pages_access_control)

  puts "> Request 2"
  cc = Gitlab::Pages::CacheControl.for_namespace(1)
  puts "\tcache_key: #{cc.cache_key}"
  puts "\tCached settings: #{Rails.cache.read('pages_domain_for_namespace_1').join(',')}"

  puts "\n> clear_cache <"
  cc = Gitlab::Pages::CacheControl.for_namespace(1)
  cc.clear_cache
  puts "\tCached settings: #{Rails.cache.read('pages_domain_for_namespace_1')&.join(',') || []}\n\n"

  puts "> Request 3"
  cc = Gitlab::Pages::CacheControl.for_namespace(1)
  puts "\tcache_key: #{cc.cache_key}"
  puts "\tCached settings: #{Rails.cache.read('pages_domain_for_namespace_1').join(',')}"
end
[dev](main)> load '/tmp/test.rb'; test
> Request 1
	cache_key: pages_domain_for_namespace_1_7d8721ac282017ec8bdf4e46c80336dcc595bf6edb0d70f086b1144480142d52
	Cached settings: 7d8721ac282017ec8bdf4e46c80336dcc595bf6edb0d70f086b1144480142d52

> change settings <

> Request 2
	cache_key: pages_domain_for_namespace_1_7330c4eccde23243f80a09f4ee9416c74465dff0dd1bb66b1ce05b4864ecbf81
	Cached settings: 7d8721ac282017ec8bdf4e46c80336dcc595bf6edb0d70f086b1144480142d52,7330c4eccde23243f80a09f4ee9416c74465dff0dd1bb66b1ce05b4864ecbf81

> clear_cache <
	Cached settings: []

> Request 3
	cache_key: pages_domain_for_namespace_1_7330c4eccde23243f80a09f4ee9416c74465dff0dd1bb66b1ce05b4864ecbf81
	Cached settings: 7330c4eccde23243f80a09f4ee9416c74465dff0dd1bb66b1ce05b4864ecbf81

MR acceptance checklist

This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.

Edited by Kassio Borges

Merge request reports

Loading