"find_commit" Gitaly N+1 when fetching commit information on MR

We're currently having N+1 calls for Gitaly find_commit endpoint when presenting discussions:

Steps to reproduce

1. Create a MR
2. Comment on a diff file
3. Outdate this diff file (change it and push)
4. You'll be able to see a new find_commit call for each new outdated file

Seems like we could be batching these requests using Gitlab::Git::Commit.batch_by_oid with BatchLoader, similarly as we lazy load Blobs.

cc @smcgivern

mentioned in issue gitaly#1129 (closed)

Thanks @oswaldo, I'm glad the new performance bar was helpful! Shall we put this on 11.0 for now?

added backend label

changed milestone to %11.0

I'm glad the new performance bar was helpful!

Indeed it is! :)

Shall we put this on 11.0 for now?

I see you already did it, thanks.

added maintenancerefactor label

FYI @zj worked on other performance issues on this code in the past:

• https://gitlab.com/gitlab-org/gitlab-ce/issues/37432
• https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/16967

This would be an easy win too, to reduce this problem further: https://gitlab.com/gitlab-org/gitlab-ce/issues/43916

@oswaldo - @andrewn pointed to this MR elsewhere: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5554

If you look at that, we appear to be doing thousands of Gitaly requests for the serialiser, too. Is that still to do with the discussions? Should we have two issues for those, or just use this one to tackle both?

mentioned in issue gitaly#1055 (closed)

marked this issue as related to gitaly#1055 (closed)

@smcgivern Wow, I can see a few find_commit calls for a local MR on .json?serializer=widget endpoint, but not thousands. Do you mind sharing details on this MR you're looking at?

This seems pretty much related though, I can handle it in the same issue if that's fine.

I see it's from https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5554, but I can't see the profiler bar, is there something I can do to enable it?

@oswaldo do you see it on GitLab.com normally? p + b should work.

It worked, thanks. This seems to have the same root cause. I think we can tackle it here as well.

I was partially wrong on https://gitlab.com/gitlab-org/gitlab-ce/issues/45190#note_68864387. I'm not being able to reproduce the same N+1 of the Discussion tab (this issue) on the .json endpoint. The cause of https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5554 massive find_commit calls on json endpoint still needs further investigation.

changed the description

I've got the strong suspicion that this happens on the notes. The way I've tested this, is just requesting endpoints in a loop and meanwhile scraping with Prometheus. I'll try and find more data today and update this issue to why this happens