Skip to content

Use cache_method_asymmetrically with Repository#has_visible_content?

Dylan Griffith requested to merge cache-issues-with-has_visible_content into master

What does this MR do?

TL;DR I don't know if there is actually a bug here and I can't reproduce this anymore but I wanted to find out from someone more familiar with the code if there is something wrong with the current caching of has_visible_content? . I'm also not sure if this fix is right.

Not sure if this is right but my local GDK got in a situation where it was caching the wrong value for some time.

Thread in https://gitlab.slack.com/archives/C8HG8D9MY/p1569970733201700

I can't reproduce this problem anymore for some reason. From what I can tell there is no redis involved (it's just in memory caching I think) in this particular cache so I can't for the life of me understand why the behaviour just changed at one point even after resetting rails console.

Basically I was consistently seeing that project.has_visible_content? => false while project.raw_repository.has_visible_content? => true for some time:

[1] pry(main)> project = Project.find(5)
  Project Load (7.4ms)  SELECT  "projects".* FROM "projects" WHERE "projects"."id" = $1 LIMIT $2  [["id", 5], ["LIMIT", 1]]
  Route Load (3.7ms)  SELECT  "routes".* FROM "routes" WHERE "routes"."source_id" = $1 AND "routes"."source_type" = $2 LIMIT $3  [["source_id", 5], ["source_type", "Project"], ["LIMIT", 1]]
=> #<Project id:5 root/wiki-index-test>
[2] pry(main)> repository = project.wiki.repository
  Group Load (4.0ms)  SELECT  "namespaces".* FROM "namespaces" WHERE "namespaces"."type" IN ('Group') AND "namespaces"."id" = $1 AND "namespaces"."type" = $2 LIMIT $3  [["id", 1], ["type", "Group"], ["LIMIT", 1]]
  Namespace Load (0.3ms)  SELECT  "namespaces".* FROM "namespaces" WHERE "namespaces"."id" = $1 LIMIT $2  [["id", 1], ["LIMIT", 1]]
  User Load (5.2ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1 LIMIT $2  [["id", 1], ["LIMIT", 1]]
=> #<Repository:@hashed/ef/2d/ef2d127de37b942baad06145e54b0c619a1f22327b2ebbcfbec78f5564afe39d.wiki>
[3] pry(main)> repository.raw_repository.has_visible_content?
  ApplicationSetting Load (2.6ms)  SELECT  "application_settings".* FROM "application_settings" ORDER BY "application_settings"."id" DESC LIMIT $1  [["LIMIT", 1]]
   (1.2ms)  SELECT "schema_migrations"."version" FROM "schema_migrations" ORDER BY "schema_migrations"."version" ASC
  Feature::FlipperGate Load (1.3ms)  SELECT "feature_gates".* FROM "feature_gates" WHERE "feature_gates"."feature_key" = $1  [["feature_key", "gitaly_cache_invalidator"]]
  Feature::FlipperGate Load (0.3ms)  SELECT "feature_gates".* FROM "feature_gates" WHERE "feature_gates"."feature_key" = $1  [["feature_key", "gitaly_inforef_uploadpack_cache"]]
  Feature::FlipperGate Load (0.2ms)  SELECT "feature_gates".* FROM "feature_gates" WHERE "feature_gates"."feature_key" = $1  [["feature_key", "gitaly_get_all_lfs_pointers_go"]]
=> true
[4] pry(main)> repository.has_visible_content?
=> false

Then at some point (I believe after I invoked repository.expire_emptiness_caches or after I edited the fallback in the cache) it stopped doing this and started consistently returning the correct value:

[1] pry(main)> project = Project.find(5)
  Project Load (2.2ms)  SELECT  "projects".* FROM "projects" WHERE "projects"."id" = $1 LIMIT $2  [["id", 5], ["LIMIT", 1]]
  Route Load (0.5ms)  SELECT  "routes".* FROM "routes" WHERE "routes"."source_id" = $1 AND "routes"."source_type" = $2 LIMIT $3  [["source_id", 5], ["source_type", "Project"], ["LIMIT", 1]]
=> #<Project id:5 root/wiki-index-test>
[2] pry(main)>
[3] pry(main)> repository = project.wiki.repository
  Group Load (2.0ms)  SELECT  "namespaces".* FROM "namespaces" WHERE "namespaces"."type" IN ('Group') AND "namespaces"."id" = $1 AND "namespaces"."type" = $2 LIMIT $3  [["id", 1], ["type", "Group"], ["LIMIT", 1]]
  Namespace Load (1.0ms)  SELECT  "namespaces".* FROM "namespaces" WHERE "namespaces"."id" = $1 LIMIT $2  [["id", 1], ["LIMIT", 1]]
  User Load (1.7ms)  SELECT  "users".* FROM "users" WHERE "users"."id" = $1 LIMIT $2  [["id", 1], ["LIMIT", 1]]
=> #<Repository:@hashed/ef/2d/ef2d127de37b942baad06145e54b0c619a1f22327b2ebbcfbec78f5564afe39d.wiki>
[4] pry(main)>
[5] pry(main)> repository.raw_repository.has_visible_content?
=> true
[6] pry(main)>
[7] pry(main)> repository.has_visible_content?
=> true

But this keep behaving that way even after I changed the code back.

Possibly related issue with fallback and false

Additionally as was noted by Kras:

# Avoid unnecessary gRPC invocations
return fallback if fallback && fallback_early?(name)

This code is likely not valid for a fallback value of false.

Screenshots

Does this MR meet the acceptance criteria?

Conformity

Performance and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team
Edited by 🤖 GitLab Bot 🤖

Merge request reports