Skip to content

Avoid cache key collision in Gitlab::Cache::Helpers

Patrick Cyiza requested to merge 383433-confidential-issue-cache-key into master

What does this MR do and why?

Some of our cache keys are generated in a way that can be prone to collisions, because the class name is being extracted incorrectly.

As part of the analysis in gitlab-com/gl-infra/scalability#2023 (closed), we captured traffic to redis-cache. We see calls that look like this:

mget
cache:gitlab:Class:tag:702f5f52d2242ba84d00c5c34b00650dc1b1e9ce:projects/17884550-20221121151723507958
cache:gitlab:Class:tag:8bc747cfa2b4c15b04558d0ae26d5977f19e1601:projects/17884550-20221121151723507958
cache:gitlab:Class:tag:af84cff6178ec5ea97fcdd863a1cd5c3624da212:projects/17884550-20221121151723507958
...

Notice the "Class" here.

I believe this "Class" comes from contextual_cache_key. This is sometimes (maybe always) getting called with a class as an argument as opposed to what it expects: an instance.

To avoid that Gitlab::Cache::Helpers#presenter_key_prefix checks if the passed presenter is a class or an instance and gives the right name according to this.

MR acceptance checklist

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

Related to #383433 (closed)

Edited by Patrick Cyiza

Merge request reports