Skip to content

CommitService#find_commit doesn't ever cache the value in a merge request

When loading a URL for a merge request widget (e.g. http://localhost:3000/TRYME/test-gitlab-bug2/merge_requests/2.json?serializer=widget), CommitService#find_commit never actually caches the value properly. If I add this debug:

--- a/lib/gitlab/gitaly_client/commit_service.rb
+++ b/lib/gitlab/gitaly_client/commit_service.rb
@@ -270,12 +270,16 @@ module Gitlab
             relative_path: @gitaly_repo.relative_path,
             commit_id: revision
           }
+
+          Rails.logger.info "=== looking up #{Thread.current[:correlation_id]} with key #{key}, exists? #{Gitlab::SafeRequestStore.exist?(key)}"
           return Gitlab::SafeRequestStore[key] if Gitlab::SafeRequestStore.exist?(key)
 
+          Rails.logger.info "=== calling find_commit #{Thread.current[:correlation_id]}: #{caller.join("\n")}"
           commit = call_find_commit(revision)
           return unless commit
 
           key[:commit_id] = commit.id
+          Rails.logger.info "=== saving find_commit #{Thread.current[:correlation_id]}: #{key}"
           Gitlab::SafeRequestStore[key] = commit
         else
           call_find_commit(revision)

I see this:

=== looking up ["268645c1-371a-4e9d-a1e3-3955ffba204f"] with key {:storage=>"default", :relative_path=>"TRYME/test-gitlab-bug2.git", :commit_id=>"refs/heads/10-7-stable"}, exists? false
=== calling find_commit ["268645c1-371a-4e9d-a1e3-3955ffba204f"]: /Users/stanhu/gdk/gitlab/lib/gitlab/git/commit.rb:66:in `block in find'
=== saving find_commit ["268645c1-371a-4e9d-a1e3-3955ffba204f"]: {:storage=>"default", :relative_path=>"TRYME/test-gitlab-bug2.git", :commit_id=>"1f0f86878c6729d9cf2cfbd40fde7514dbe21451"}
=== looking up ["268645c1-371a-4e9d-a1e3-3955ffba204f"] with key {:storage=>"default", :relative_path=>"TRYME/test-gitlab-bug2.git", :commit_id=>"refs/heads/10-6-stable"}, exists? false
=== calling find_commit ["268645c1-371a-4e9d-a1e3-3955ffba204f"]: /Users/stanhu/gdk/gitlab/lib/gitlab/git/commit.rb:66:in `block in find'
=== saving find_commit ["268645c1-371a-4e9d-a1e3-3955ffba204f"]: {:storage=>"default", :relative_path=>"TRYME/test-gitlab-bug2.git", :commit_id=>"16f9faa876df1edc1fa2a6519afa5fcebbdc1fc1"}

Notice that the requested commit_id was refs/heads/10-7-stable, but commit_id resolved to the actual OID. Gitaly then receives multiple FindCommit calls per request.

Edited by Stan Hu